Skip to content

SMT2 back-end: do not flatten flattened arrays #7144

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

Merged
merged 2 commits into from
Oct 26, 2022

Conversation

tautschnig
Copy link
Collaborator

convert_expr may flatten an array anyway, in which case flatten_array would generate select statements over bitvectors instead of arrays (as flatten_array itself invokes convert_expr to supposedly construct the array expression).

  • Each commit message has a non-empty body, explaining why the change was made.
  • n/a 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.

@tautschnig
Copy link
Collaborator Author

tautschnig commented Sep 20, 2022

The change thus far fixes syntax problems, but the verification outcome still isn't correct. Will continue to work on this.

@tautschnig tautschnig force-pushed the bugfixes/smt-array-flatten branch from 88c669e to ed855b9 Compare September 21, 2022 16:02
@tautschnig tautschnig marked this pull request as ready for review September 21, 2022 16:03
@tautschnig
Copy link
Collaborator Author

This PR now includes two (in some ways: unrelated) bugfixes. In order for the one test that is flipped from broken-smt-backend to works-with-Z3 both of these bugfixes are required. The second commit (which is about byte operator lowering) touches code that is most heavily exercised when using the SMT back-end, where all byte operators are rewritten via this code path.

@codecov
Copy link

codecov bot commented Sep 21, 2022

Codecov Report

Base: 78.00% // Head: 78.00% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (e5d94c8) compared to base (062bc0c).
Patch coverage: 66.66% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #7144   +/-   ##
========================================
  Coverage    78.00%   78.00%           
========================================
  Files         1621     1621           
  Lines       187279   187264   -15     
========================================
- Hits        146088   146080    -8     
+ Misses       41191    41184    -7     
Impacted Files Coverage Δ
src/cprover/axioms.cpp 92.11% <ø> (+0.54%) ⬆️
src/cprover/counterexample_found.cpp 93.33% <ø> (+11.19%) ⬆️
src/cprover/solver_progress.cpp 60.00% <0.00%> (-20.00%) ⬇️
src/cprover/state_encoding.cpp 80.57% <ø> (+0.22%) ⬆️
src/cprover/solver.cpp 81.53% <40.90%> (-1.62%) ⬇️
src/cprover/propagate.cpp 73.52% <44.44%> (-1.48%) ⬇️
src/solvers/smt2/smt2_conv.cpp 65.49% <85.71%> (+0.08%) ⬆️
src/solvers/lowering/byte_operators.cpp 92.65% <100.00%> (-0.11%) ⬇️
src/solvers/smt2/smt2_format.cpp 85.39% <100.00%> (ø)
src/cprover/solver_types.h 88.88% <0.00%> (-0.59%) ⬇️
... and 7 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tautschnig tautschnig force-pushed the bugfixes/smt-array-flatten branch 2 times, most recently from d396182 to 7997315 Compare October 19, 2022 14:04
@tautschnig tautschnig self-assigned this Oct 19, 2022
convert_expr may flatten an array anyway, in which case flatten_array
would generate select statements over bitvectors instead of arrays (as
flatten_array itself invokes convert_expr to supposedly construct the
array expression). Also, make sure the same flattening logic applies to
1-element structs.
When updating an array/vector at a non-constant byte offset the update
value may affect multiple array/vector elements. The last such element
needs to be updated with however many bytes have not yet been used from
the update value. With a non-constant byte offset the size of remaining
bytes may be zero. Therefore, make the tail update use symbolic offsets
rather than constants.

When fixing this, it also became apparent that we must not use byte
extracts of non-constant extraction size to build update elements.
Instead, the update offset should be non-constant, and will actually
need to be negative on such occasions.
@tautschnig tautschnig force-pushed the bugfixes/smt-array-flatten branch from 7997315 to e5d94c8 Compare October 20, 2022 07:50
@tautschnig tautschnig removed their assignment Oct 20, 2022
@kroening kroening merged commit 0a09814 into diffblue:develop Oct 26, 2022
@tautschnig tautschnig deleted the bugfixes/smt-array-flatten branch October 27, 2022 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants