-
Notifications
You must be signed in to change notification settings - Fork 274
A predicate to validate all pointer derefs in an expression AST #6365
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
f97776b
to
a5ec09d
Compare
a5ec09d
to
c211860
Compare
Codecov Report
@@ Coverage Diff @@
## develop #6365 +/- ##
===========================================
+ Coverage 75.90% 75.91% +0.01%
===========================================
Files 1515 1517 +2
Lines 164005 164024 +19
===========================================
+ Hits 124484 124515 +31
+ Misses 39521 39509 -12
Continue to review full report at Codecov.
|
This should not go into |
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.
Only minor comments.
{ | ||
binary_relation_exprt component_i_equality{lhs[i], ID_equal, rhs[i]}; | ||
equality_conjunctions[i] = | ||
and_exprt(equality_conjunctions[i - 1], component_i_equality); |
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.
The lack of test coverage reported for this loop body seems concerning.
src/goto-instrument/havoc_utils.cpp
Outdated
append_safe_havoc_code_for_expr(source_location, ns, expr, dest, [&]() { | ||
append_object_havoc_code_for_expr(source_location, expr, dest); | ||
}); | ||
continue; |
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.
Lack of test coverage on this conditional is a concern
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.
Good point. We do have a test case for multi-dimensional decreases clauses, and it seems to be working as expected. I can take a closer look at it, but I will probably split any fixes (if needed) into a separate PR, and keep this one only about validity checking.
In this change, we move two of the "static" functions within contracts.cpp to utils.cpp (a new file in contracts module). We also fix the #include dependence -- assigns should not #include contracts.h, it should be the other way round.
76babcb
to
59bfc73
Compare
59bfc73
to
10dc391
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.
LGTM.
+ checking containment in assigns clause verification + havocing only valid memory locations in assigns clause replacement + havocing only valid memory locations in loop contract application + creating history snapshots only for valid `old` expressions
10dc391
to
fcca0e5
Compare
This is useful for:
old
expressionsThe feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/My commit message includes data points confirming performance improvements (if claimed).