SMT back-end: Extend trace value evaluation

[tautschnig]

We record the values of expressions by adding expr == expr as constraints in order to be able to fetch and display them in the GOTO trace (for declaration; we presently also introduce new symbols in other cases). Constructing a trace then requires the ability to evaluate all kinds of expressions to obtain the value for those expressions.

Fixes: #7308

• 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.

[codecov]

Codecov Report

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

Coverage data is based on head (f89af9d) compared to base (a295651).
Patch coverage: 83.58% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #7310   +/-   ##
========================================
  Coverage    78.28%   78.29%           
========================================
  Files         1642     1642           
  Lines       190004   190012    +8     
========================================
+ Hits        148744   148761   +17     
+ Misses       41260    41251    -9     

Impacted Files	Coverage Δ
src/goto-instrument/contracts/contracts.h	100.00% <ø> (ø)
src/goto-programs/graphml_witness.cpp	52.94% <0.00%> (-0.15%)	⬇️
src/goto-programs/interpreter.cpp	52.38% <0.00%> (+0.17%)	⬆️
src/util/expr_initializer.cpp	85.10% <80.00%> (+1.77%)	⬆️
src/solvers/smt2/smt2_conv.cpp	66.52% <92.85%> (+0.13%)	⬆️
jbmc/src/java_bytecode/expr2java.cpp	86.25% <93.75%> (ø)
src/ansi-c/c_typecheck_initializer.cpp	76.57% <100.00%> (+0.04%)	⬆️
src/ansi-c/expr2c.cpp	67.33% <100.00%> (-0.02%)	⬇️
src/goto-instrument/contracts/contracts.cpp	95.09% <100.00%> (+0.06%)	⬆️
src/goto-instrument/dump_c.cpp	79.97% <100.00%> (-0.03%)	⬇️
... and 4 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.

[TGWDB]

I don't quite understand why we return nil if any operand could be nil? The old code here (e.g.) for arrays would leave the nil in place and return the larger expr.

[tautschnig]

I'd fear that we fail to notice problems. I'm not sure returning nil ever is the right thing to do?

[kroening]

The question is whether the decision procedure or the user of the decision procedure ought to do the simplification.

[peterschrammel]

Reason: full_lhs_value in assignment must not be nil

How does simplification make it non-nil?

[tautschnig]

@kroening said:

The question is whether the decision procedure or the user of the decision procedure ought to do the simplification.

I accept that the changes introducing simplification to io_args could be factored out, they were a bit of a drive-by cleanup. We already do this in other branches of build_goto_trace, so it just seemed like a good idea to make code simpler. Let me know if you want me to remove those from the PR.

@peterschrammel said:

Reason: full_lhs_value in assignment must not be nil

How does simplification make it non-nil?

Would you mind elaborating for I'm not sure how this comment came about?

[peterschrammel]

Would you mind elaborating for I'm not sure how this comment came about?

I'm referring to the issue that is supposed to fixed here. I'm not entirely sure I understand how simplification fixes it.

[tautschnig]

Would you mind elaborating for I'm not sure how this comment came about?

I'm referring to the issue that is supposed to fixed here. I'm not entirely sure I understand how simplification fixes it.

Ah! The issue is fixed by the changes in this PR as https://github.com/diffblue/cbmc/pull/7310/files#diff-0eb7414a51aacc433e17eec049af7ed61dd62d3ee3c4b466f991c60a861c7fccR390-R391 will now receive a non-nil value. Not a matter of simplification at all.

[tautschnig]

The question is whether the decision procedure or the user of the decision procedure ought to do the simplification.

I have removed the simplification changes from this PR and moved them to #7323 instead.