-
Notifications
You must be signed in to change notification settings - Fork 274
Include pointer offset in counterexample output #3135
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well this seems like an unpleasant omission. Are you sure that pointer_expr() is only used in output? If it is used elsewhere we might want to see if this fixes other things.
I guess if we really wanted to show off we could check if the base object was an array and turn it into &(a[i]). Would this be more human friendly than a + i ?
int A[2]; | ||
int *ip = &A[1]; | ||
uuu.structbb.intb = 1; | ||
// assert(*ptr == 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passed Diffblue compatibility checks (cbmc commit: 8652d9c).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/87494837
return typecast_exprt::conditional_cast( | ||
plus_exprt( | ||
base, from_integer(pointer.offset % *object_size, pointer_diff_type())), | ||
type); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You probably meant pointer.offset / *object_size
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooops - that clearly shows that my test case is not good enough. Thanks for catching that! I'll extend my test.
@martin-cs said:
I need to review #2161 as that has modifications in the same area of code. |
Marking do-not-merge as this needs to be built on top of the fixes in #2161. |
While the bit-level pointer output would convey the information, the human-readable expression previously did not consider non-zero offsets except for null pointers.
8652d9c
to
e140d16
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✔️
Passed Diffblue compatibility checks (cbmc commit: e140d16).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/89854653
While the bit-level pointer output would convey the information, the
human-readable expression previously did not consider non-zero offsets except
for null pointers.