Skip to content
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

Value-set based dereferencing: fix simplified handling of *(p + i) #8578

Merged

Conversation

tautschnig
Copy link
Collaborator

Value-set based dereferencing must not take an access path through an object that precludes a subsequent index expression from accessing a different part of the object. Such a situation can arise when the value set has a known (constant) offset for the pointer that would identify one particular element in an array (within that object). The code using value-set based dereferencing, however, may be trying to resolve a subexpression of an index expression, where said index expression would lead to a different element that may itself be part of a different array within the same overall object.

Fixes: #8570

  • Each commit message has a non-empty body, explaining why the change was made.
  • Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • n/a The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • n/a My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

Copy link

codecov bot commented Jan 31, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.65%. Comparing base (66004dc) to head (164407f).
Report is 15 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #8578      +/-   ##
===========================================
- Coverage    78.71%   78.65%   -0.06%     
===========================================
  Files         1732     1732              
  Lines       199536   199673     +137     
  Branches     18281    18244      -37     
===========================================
- Hits        157057   157056       -1     
- Misses       42479    42617     +138     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@remi-delmas-3000
Copy link
Collaborator

The commit adds a new argument recurse_into_arrays to the get_subexpression_at_offset to force fallback to using byte-extracts but I could not really understand what condition triggers the fallback ? Is the idea to try to resolve a lookup expression as a cell/field access if enough info is known about the index and default to a flat/unstructured by-extract otherwise ?

@tautschnig tautschnig force-pushed the bugfixes/8570-pointer-and-array branch from d6de37e to f6fdf26 Compare February 9, 2025 22:47
@tautschnig tautschnig changed the title Value-set based dereferencing must consider possible index expressions Value-set based dereferencing: fix simplified handling of *(p + i) Feb 9, 2025
@tautschnig
Copy link
Collaborator Author

@kroening Reworked as discussed.

Value-set based dereferencing must not take an access path through an
object that precludes a subsequent index expression from accessing a
different part of the object. Such a situation can arise when the value
set has a known (constant) offset for the pointer that would identify
one particular element in an array (within that object). The code using
value-set based dereferencing, however, may be trying to resolve a
subexpression of an index expression, where said index expression would
lead to a different element that may itself be part of a different array
within the same overall object.

Fixes: diffblue#8570
The outer decision already ensures that types match, so replacing types
cannot be necessary.
@tautschnig tautschnig force-pushed the bugfixes/8570-pointer-and-array branch from f6fdf26 to 164407f Compare February 10, 2025 06:34
@tautschnig tautschnig assigned kroening and unassigned tautschnig Feb 11, 2025
@tautschnig tautschnig merged commit 159af34 into diffblue:develop Feb 20, 2025
39 of 40 checks passed
@tautschnig tautschnig deleted the bugfixes/8570-pointer-and-array branch February 20, 2025 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

__CPROVER_forall fails when dealing with flattening of nested structure
4 participants