Skip to content

Fix trace invariant violation on smt2 incremental backend #7404

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

Conversation

esteffin
Copy link
Contributor

@esteffin esteffin commented Dec 1, 2022

Fix invariant violation in incremental SMT solver trace generator.

An invariant was generated when the trace node was not a symbol_exprt, while now the expression is simplified by replacing its operands with the value got from the solver (as in the other backends).

This PR also fixes an inconsistency with the bit-vector shift operator when the left-hand-side operands has more bits than the left-hand.

  • 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.
  • 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).
  • My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@esteffin esteffin force-pushed the esteffin/fix-trace-invariant-violation branch from b657adf to 0f8fe68 Compare December 1, 2022 14:46
@thomasspriggs thomasspriggs force-pushed the esteffin/fix-trace-invariant-violation branch from 9aedb44 to a6059fd Compare December 1, 2022 17:10
@codecov
Copy link

codecov bot commented Dec 2, 2022

Codecov Report

Base: 78.38% // Head: 78.36% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (50a8b01) compared to base (0841040).
Patch coverage: 97.50% of modified lines in pull request are covered.

❗ Current head 50a8b01 differs from pull request most recent head 10806ee. Consider uploading reports for the commit 10806ee to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #7404      +/-   ##
===========================================
- Coverage    78.38%   78.36%   -0.03%     
===========================================
  Files         1647     1647              
  Lines       190362   189969     -393     
===========================================
- Hits        149213   148865     -348     
+ Misses       41149    41104      -45     
Impacted Files Coverage Δ
...ncremental/smt2_incremental_decision_procedure.cpp 96.67% <94.11%> (-0.20%) ⬇️
src/solvers/smt2/smt2_conv.cpp 68.69% <100.00%> (+0.02%) ⬆️
...c/solvers/smt2_incremental/convert_expr_to_smt.cpp 83.48% <100.00%> (+0.10%) ⬆️
...t/solvers/smt2_incremental/convert_expr_to_smt.cpp 99.60% <100.00%> (+<0.01%) ⬆️
...ncremental/smt2_incremental_decision_procedure.cpp 98.59% <100.00%> (-0.17%) ⬇️
src/cpp/cpp_item.h 64.15% <0.00%> (-7.55%) ⬇️
src/cpp/cpp_typecheck_namespace.cpp 53.48% <0.00%> (-3.96%) ⬇️
src/goto-programs/goto_convert_exceptions.cpp 51.78% <0.00%> (-1.67%) ⬇️
src/cpp/cpp_typecheck_enum_type.cpp 77.19% <0.00%> (-1.32%) ⬇️
src/statement-list/statement_list_entry_point.cpp 83.87% <0.00%> (-1.28%) ⬇️
... and 146 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.

@esteffin esteffin force-pushed the esteffin/fix-trace-invariant-violation branch from 50a8b01 to 656c9e7 Compare December 5, 2022 17:32
@esteffin esteffin marked this pull request as ready for review December 5, 2022 17:36
KNOWNBUG
byte_update_issue.c
CORE
byte_update.c
--trace
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to have some regexes in these tests, to confirm that the generated traces are as we expect them to be.

@esteffin esteffin force-pushed the esteffin/fix-trace-invariant-violation branch 4 times, most recently from 30efdfe to 241b916 Compare December 7, 2022 13:14
Enrico Steffinlongo and others added 3 commits December 7, 2022 14:05
This improves maintainability by reducing the level of nesting and
moving the descriptor building code together.
In order to avoid increasing the size of the ::get function and so that
we can explain what this piece of code is doing.
@esteffin esteffin force-pushed the esteffin/fix-trace-invariant-violation branch from 241b916 to 10806ee Compare December 7, 2022 14:05
@esteffin esteffin changed the title Fix trace invariant violation Fix trace invariant violation on smt2 incremental backend Dec 7, 2022
@esteffin esteffin merged commit 8d6f87b into diffblue:develop Dec 7, 2022
@esteffin esteffin deleted the esteffin/fix-trace-invariant-violation branch December 7, 2022 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants