Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsound interprocedural Array_bounds analysis when array is modified #9

Open
julbinb opened this issue Jan 31, 2024 · 1 comment
Open

Comments

@julbinb
Copy link

julbinb commented Jan 31, 2024

solved_ArraySwap.dsg.zip
Here is a Java version of bucket_swap.js from DAI's benchmarks:

public class ArraySwap {
    static boolean swap(int[] array, int i, int j) {
        int temp;
        if (i < 0 || i >= array.length || j < 0 || j >= array.length) {
            return false;
        }
        temp = array[i];
        array[i] = array[j];
        array[j] = temp;
        return true;
    }
    
    public static void main(String[] args) {
        int[] numberArray = {1, 8, 10};//;1, 8, 8, 8, 10, 10};
        boolean mustTrue = numberArray[1] == 8;
        
        // test cases "swap only accepts valid positions"
        boolean test1 = swap(numberArray, 0, 2);
        boolean test2 = swap(numberArray, 0, 1);
        
        // the analysis is unsound: it reports true here
        boolean mustFalse = numberArray[1] == 8;
        
        // test cases "swap accepts bad positions"
        boolean test3 = swap(numberArray, 42, 1);
    }
}

Interprocedural analysis with Array_bounds computes mustFalse to be true.

Looking at the DAIG of swap, I see that elements of the input array are removed from the abstract memory when the array is modified. And then in the main function, abstract memory remains the same, "forgetting" the fact the array has changed.

[UPD] The DSG is attached.

@bennostein
Copy link
Member

I can repro this -- thanks for the test case! That domain should be smarter in how it applies procedure summaries at return sites. It's able to deal with mutation on fields of the receiver but isn't correctly handling mutations on arguments. I'll try to get that ironed out soon when I get the time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants