-
Notifications
You must be signed in to change notification settings - Fork 77
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
Make meet in AddressDomain more precise #1468
Merged
Merged
+96
−3
Conversation
This file contains 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
michael-schwarz
changed the title
Make meet in AddressDomain more Precise
Make meet in AddressDomain more precise
May 15, 2024
7 tasks
sim642
reviewed
May 20, 2024
Sorry for being MIA, I will take this up and hopefully get it mergeable before the holidays. |
@sim642 anything blocking the merge here? After your comments are integrated, it now is quite lightweight. |
sim642
reviewed
Jan 17, 2025
sim642
approved these changes
Jan 17, 2025
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.
Currently, in the address domain inside the buckets, for the meets we use
ProjectiveSetPairwiseMeet
analyzer/src/domain/disjointDomain.ml
Lines 190 to 205 in bffc5e3
where
B.may_be_equal
delegates toanalyzer/src/cdomain/value/cdomains/addressDomain.ml
Line 180 in bffc5e3
Addr.semantic_equal
given byanalyzer/src/cdomain/value/cdomains/addressDomain.ml
Lines 102 to 111 in bffc5e3
which calls
SD.sematic_equal
analyzer/src/cdomain/value/cdomains/stringDomain.ml
Lines 77 to 81 in bffc5e3
this is needed to handle all sorts of different offsets that may be semantically equal, i.e., evaluate to the same physical address. In this case it keeps both elements.
After some discussion with @jerhard and @hseidl, we reached the following insights:
This behavior is overly conservative, as the mess of the address lattice contains some sub-lattices where the
meet
operation is semantically correct.This now checks via a new predicate
amenable_to_meet
whether for the two addressesa
andb
this relationship between values and the meet holds. If this is the case, a meet is performed and the element added to the resulting set only if it is distinct frombot
.Currently, we answer
amenable_to_meet
by true whenever both arguments are strings, or when the both are addresses that differ in the numeric(!) offsets only.This PR also adds two tests where this yields additional precision.
Closes #1467