Skip to content

array index off by one #926

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

Closed
mgudemann opened this issue May 15, 2017 · 6 comments
Closed

array index off by one #926

mgudemann opened this issue May 15, 2017 · 6 comments

Comments

@mgudemann
Copy link
Contributor

This program is verified by CBMC

class incr
{
  int[] nums=new int[]{1,2,3};
  void f(int n)
  {
    if(n==0)
      assert(nums[n++]==2);
  }
  public static void main(String[] args)
  {
    new incr().f(0);
  }
}    

as

[java::incr.f:(I)V.assertion.1] assertion at file incr.java line 7 function java::incr.f:(I)V bytecode_index 13: SUCCESS

but java -ea incr correctly fails

Exception in thread "main" java.lang.AssertionError
	at incr.f(incr.java:7)
	at incr.main(incr.java:13)

the problem is that the array access index in the GOTO program is incremented before the access is done

@mgudemann
Copy link
Contributor Author

found by @reuk in java/util/ArrayList.java

@mgudemann
Copy link
Contributor Author

There seems to be a problem with the ?load implementation. In the example the bytecodes in question are

        14: iload_1
        15: iinc          1, 1
        18: iaload

where local variable 1 corresponds to n. This is supposed to 1) put the content of local variable 1 onto the stack 2) increase the local variable 1, and 3) access an array at the index on the stack (in this case the old value of n).

The ?load implementation puts the variable itself in the result (and therefore on the stack) and therefore the increment applies to the access index of the array, too.

@mgudemann
Copy link
Contributor Author

So this seems to be a slightly more complex problem. In short, on the stack are values of local variables or fields. When the local variable or field is changes, the value of the stack must not change at the same time. The current implementation of ?load pushes the variable instead of its value and getfield / getstatic push a reference to the field.
This can be solved by introducing a temporary variable for each such case, where the temporary variable holds the value of the variable or field at the time of the push.

@mgudemann
Copy link
Contributor Author

cf. #931

@tautschnig
Copy link
Collaborator

This will be addressed by #951.

@tautschnig
Copy link
Collaborator

#951 has been merged several months ago.

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

No branches or pull requests

2 participants