-
Notifications
You must be signed in to change notification settings - Fork 274
Fix cbmc crash on pointer checks of void pointer dereferences #5427
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
int main() | ||
{ | ||
void *p; | ||
*p; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
CORE | ||
main.c | ||
--pointer-check | ||
^EXIT=10$ | ||
^SIGNAL=0$ | ||
-- | ||
^warning: ignoring | ||
-- | ||
Checks that cbmc does not crash with --pointer-check on dereference of an | ||
invalid void pointer |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
int main() | ||
{ | ||
int a = 0; | ||
void *p = &a; | ||
*p; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
CORE | ||
main.c | ||
--pointer-check | ||
^EXIT=0$ | ||
^SIGNAL=0$ | ||
^VERIFICATION SUCCESSFUL$ | ||
-- | ||
^warning: ignoring | ||
-- | ||
Checks that cbmc does not crash with --pointer-check on dereference of a valid | ||
void pointer |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1164,10 +1164,24 @@ void goto_checkt::pointer_validity_check( | |
|
||
const exprt &pointer=expr.pointer(); | ||
|
||
auto size_of_expr_opt = size_of_expr(expr.type(), ns); | ||
CHECK_RETURN(size_of_expr_opt.has_value()); | ||
exprt size; | ||
|
||
auto conditions = address_check(pointer, size_of_expr_opt.value()); | ||
if(expr.type().id() == ID_empty) | ||
{ | ||
// a dereference *p (with p being a pointer to void) is valid if p points to | ||
// valid memory (of any size). the smallest possible size of the memory | ||
// segment p could be pointing to is 1, hence we use this size for the | ||
// address check | ||
size = from_integer(1, size_type()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we change There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure. There are quite a number of uses in the codebase that check that the return value of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the purpose of this to create an address check that fails? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No the address check should succeed. Basically There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please explain the magic |
||
} | ||
else | ||
{ | ||
auto size_of_expr_opt = size_of_expr(expr.type(), ns); | ||
CHECK_RETURN(size_of_expr_opt.has_value()); | ||
size = size_of_expr_opt.value(); | ||
} | ||
|
||
auto conditions = address_check(pointer, size); | ||
|
||
for(const auto &c : conditions) | ||
{ | ||
|
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.
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.
Not necessary I think as
EXIT=10
already indicates that cbmc exited without error and verification failed.