Bounds context: update after assignments (tests)#402
Conversation
| int tmp1 = 0; | ||
| arg4 = &tmp1; | ||
| arg4 = &*arg4; | ||
| arg4 = &*arg4; // expected-error {{expression has unknown bounds}} |
There was a problem hiding this comment.
This could be confusing to the user to get an error message here, since this is effectively the same as the assignment arg4 = arg4. In the assignment arg4 = arg4, the original value of arg4 is arg4, so there is no error. However, in this assignment arg4 = &*arg4, arg4 has no original value, so the observed bounds for the source of the assignment are bounds(unknown).
A potential fix would be to modify the IsInvertible and Inverse methods so that the original value of &*p with respect to the variable p is p. Then, an assignment p = &*p would have the same behavior as the assignment p = p. @dtarditi would this be the correct behavior?
There was a problem hiding this comment.
This issue will be addressed once the checkedc-clang PR 839 is merged. &*arg4 will have an inverse of arg4.
There was a problem hiding this comment.
This is now addressed after merging checkedc-clang 839.
dtarditi
left a comment
There was a problem hiding this comment.
I have a few suggestions for improving this change.
| array_ptr<int> r : count(3) = 0; | ||
| ptr<int> q = 0; | ||
| r = _Assume_bounds_cast<array_ptr<int>>(h4(), bounds(r, r + 4) rel_align(int)); | ||
| r = _Assume_bounds_cast<array_ptr<int>>(h4(), bounds(r, r + 4) rel_align(int)); // expected-error {{expression has unknown bounds}} |
There was a problem hiding this comment.
It is worth adding a comment explaining what is broken here. We declare the result of h4 to have bounds based on r, but overwrite r, invaliding the bounds of the entire _Assume_bounds_cast expression (since r has now changed).
Could you add an example of how to property initialize r? Does
r = _Assume_bounds_cast<array_ptr<int>>(h4(), count(4) )
work?
There was a problem hiding this comment.
r = _Assume_bounds_cast<array_ptr<int>>(h4(), count(4)) works. I've added it to the test.
This PR updates checked-c tests to add expected warnings and errors since the checkedc-clang PR 836 updates the observed bounds context after an assignment to a variable.