Pointer overflow checks should detect overflow in offset multiplication #6633
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tautschnig
force-pushed
the
bugfixes/pointer-arith-overflow
branch
from
5980b0e to
81276c2
Compare
tautschnig
requested review from
chris-ryder,
martin-cs and
peterschrammel
as code owners
tautschnig
force-pushed
the
bugfixes/pointer-arith-overflow
branch
2 times, most recently
from
5204303 to
2a0230e
Compare
3 tasks
Codecov Report
@@ Coverage Diff @@
## develop #6633 +/- ##
========================================
Coverage 76.72% 76.72%
========================================
Files 1579 1579
Lines 181938 181953 +15
========================================
+ Hits 139587 139602 +15
Misses 42351 42351
Continue to review full report at Codecov.
|
tautschnig
force-pushed
the
bugfixes/pointer-arith-overflow
branch
6 times, most recently
from
35d46f3 to
6d9219d
Compare
chris-ryder
approved these changes
Feb 5, 2022
src/analyses/goto_check_c.cpp
Outdated
| /// | ||
| /// A flag's initial value (before any `set_flag` or `disable_flag`) is restored | ||
| /// when the entire object goes out of scope. | ||
| class flag_resett |
There was a problem hiding this comment.
I realise this is just code you've moved, but just wondering about the naming of this class (sorry...) Conceptually what this class is really about is less about "resetting" a flag, and more about introducing a "scope" for the flag? So maybe call it something like flag_scopet ? or flag_overridet ? One for a later PR though.
There was a problem hiding this comment.
Good call, I think flag_overridet is the best match for what it does. I've inserted a commit to do the renaming.
No changes in behaviour, just code motion. Upcoming commits will use this class in `check_rec`.
There isn't any use of fields other than the instruction's source location, so take just that. Also use `as_string()` instead of `pretty()` for more nicely formatted error reporting.
This class no longer is confined to resetting flags, but now has additional override capabilities. Co-authored-by: Chris Ryder <[email protected]>
Pointer arithmetic requires multiplication of the offset by the size of the base type (for any base type larger than 1 byte). Such a multiplication isn't introduced until the back-end, where no opportunity for adding properties exists anymore. Therefore, synthesize the multiplication to generate arithmetic overflow checks at the GOTO level. Fixes: diffblue#6631
tautschnig
force-pushed
the
bugfixes/pointer-arith-overflow
branch
from
6d9219d to
419837e
Compare
chris-ryder
approved these changes
Feb 6, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pointer arithmetic requires multiplication of the offset by the size of
the base type (for any base type larger than 1 byte). Such a
multiplication isn't introduced until the back-end, where no opportunity
for adding properties exists anymore. Therefore, synthesize the
multiplication to generate arithmetic overflow checks at the GOTO level.
Fixes: #6631