Quantifier instantiation via simplistic E-matching#8224
Conversation
|
Despite this being a "draft" PR I would like to ask for feedback on the following aspects:
|
e08d98e to
24806af
Compare
24806af to
69f8720
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8224 +/- ##
===========================================
- Coverage 80.00% 80.00% -0.01%
===========================================
Files 1700 1700
Lines 188252 188470 +218
Branches 73 73
===========================================
+ Hits 150613 150783 +170
- Misses 37639 37687 +48 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
6e48e44 to
49b3909
Compare
7bb221f to
64252a0
Compare
d821caa to
090c84a
Compare
This is now done, comment explaining the (expected) verification failure is now included. |
090c84a to
72550cf
Compare
|
This needs serious consideration: failing quantifier instantiation is the leading source of proof brittleness, and we are opening the door to that here. |
|
I would much prefer a more deterministic, predictable and robust solution to this. |
|
Hard maybe? So, it would be good but there is going to be a limit on how much we can do like this. Unless we put a refinement loop around the solver and do something like MBQI. Getting that stable and reliable is tricky. Given the amount of work that has gone into E-matching in Z3 and Andy's decade+ of work on quantifiers in cvc5, I wonder whether it would be more efficient (in terms of dev-hours / things proved) to improve our SMT integration. To actually answer the question you asked, things that might help:
HTH |
72550cf to
5e379d6
Compare
5e379d6 to
e076dab
Compare
2a85427 to
9be48ee
Compare
9be48ee to
9e896a5
Compare
There was a problem hiding this comment.
Pull request overview
This PR implements quantifier instantiation via E-matching for cases where eager (enumerative) instantiation cannot be applied. The implementation uses pattern matching on array index expressions to find suitable instantiation values from the solver's cache.
Key changes:
- Added E-matching based quantifier instantiation as a fallback when eager instantiation fails
- Removed the
lb > ubcheck in eager instantiation (correctly handles empty ranges with vacuous truth/false) - Updated test expectations to reflect that previously unsupported quantifier cases now work
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/solvers/flattening/boolbv_quantifier.cpp | Implements E-matching algorithm via instantiate_one_quantifier function and modifies finish_eager_conversion_quantifiers to apply it |
| regression/cbmc/Quantifiers-unbounded-array/test.desc | Test descriptor for new unbounded array quantifier test case |
| regression/cbmc/Quantifiers-unbounded-array/main.c | Test case demonstrating quantifier instantiation with unbounded arrays and loop invariants |
| regression/cbmc/Quantifiers-unbounded-array-overflow/test.desc | Test descriptor for overflow detection with quantifiers |
| regression/cbmc/Quantifiers-unbounded-array-overflow/main.c | Test case that validates overflow detection in loop invariants with quantifiers |
| regression/cbmc/Quantifiers-two-dimension-array/no-upper-bound.desc | Test descriptor for 2D array quantifier handling |
| regression/cbmc/Quantifiers-two-dimension-array/no-upper-bound.c | Test case for nested quantifiers over 2D arrays |
| regression/cbmc/Quantifiers-type/test.desc | Updated test expectations to reflect working quantifier support |
| regression/cbmc/Quantifiers-invalid-var-range/test.desc | Updated test expectations from KNOWNBUG to CORE status |
| doc/cprover-manual/cbmc-assertions.md | Updated documentation from "future releases will support" to "CPROVER supports" for quantifiers |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else | ||
| return conjunction(instantiations); |
There was a problem hiding this comment.
Missing defensive check for quantifier type. Add an assertion or UNREACHABLE statement after the if-else chain to handle the case where q_expr.id() is neither ID_exists nor ID_forall, similar to the pattern used in eager_quantifier_instantiation at line 265.
| else | |
| return conjunction(instantiations); | |
| else if(q_expr.id() == ID_forall) | |
| return conjunction(instantiations); | |
| UNREACHABLE; | |
| return {}; |
9e896a5 to
d7eba68
Compare
conjunction(...) and disjunction(...) helper functions produce the appropriate result for empty operand sequences.
Use E-matching on index expressions to instantiate quantifiers when eager instantiation (aka enumerative instantiation) cannot be applied.
Replace the simplistic E-matching in instantiate_one_quantifier with a
complete instantiation approach based on:
Yeting Ge and Leonardo de Moura, "Complete instantiation for quantified
formulas in Satisfiability Modulo Theories", CAV 2009.
The paper shows that for essentially uninterpreted formulas (where
quantified variables only appear as arguments of uninterpreted functions),
the quantifier can be eliminated by instantiating with a finite set of
ground terms computed as the least fixed point of a system of set
constraints (ΔF). In CBMC's context, array accesses (index_exprt) play
the role of uninterpreted functions.
Key changes:
- find_index_contexts: Properly decomposes array index expressions into
bound_var + offset form using pre-order traversal. Handles patterns
var, cast(var), var+r, r+var, var-r. This implements the offset
extension from Section 4 of the paper, where for f(x_i + r) we
generate constraints S_{k,i} + r ⊆ A_{f,j} and A_{f,j} - r ⊆ S_{k,i}.
- collect_ground_indices: Collects ground index terms from both
index_exprt (array reads) and with_exprt (array writes) in the
bitvector cache, seeding the sets A_{f,j} from the paper.
- compute_instantiation_set: Computes the least fixed point of the set
constraint system ΔF. Uses integer arithmetic when all offsets and
ground indices are numeric constants (avoiding expression growth), with
a symbolic fallback using simplify_expr. Bounded by max_iterations and
max_terms to handle non-stratified (non-FEU) cases.
- arrays_match: SSA-aware array comparison using L1 object identifiers
to match across SSA versions. For nested array accesses (e.g.,
multi-dimensional arrays like a[i][j]), recursively matches the base
array and compares simplified indices, since the pattern may contain
unsimplified arithmetic (e.g., cast(0 % 2)) that is semantically
equal to a constant in the cache.
Instantiation terms are sorted (via irept::operator<) before generating
the conjunction/disjunction to ensure deterministic SAT clause ordering.
Without this, MiniSat performance varies from sub-second to minutes on
the same UNSAT instance depending on hash-dependent iteration order,
which changes with file paths embedded in source locations.
This enables verification of properties that depend on relationships
between adjacent array elements, such as:
forall j: arr[j+1] == arr[j] + 1
which the previous E-matching could not handle because it did not
generate the ground terms needed for the offset positions.
Add two regression tests exercising the complete instantiation approach:
- Quantifiers-offset-instantiation: Tests the offset pattern arr[j+1]
from Section 4 of Ge & de Moura (2009). The quantifier
forall j < n-1: arr[j+1] == arr[j] + 1
requires the set constraint fixed-point to generate ground terms for
both arr[j] and arr[j+1] positions. Without the offset handling, the
instantiation misses the terms needed to chain the equalities.
- Quantifiers-fixpoint-instantiation: Tests that the fixed-point
computation correctly propagates through multiple iterations. The
property arr[2] > 0 requires two steps of reasoning:
j=0: arr[0]>0 => arr[1]>0
j=1: arr[1]>0 => arr[2]>0
This exercises the iterative nature of the ΔF fixed-point from
Section 3 of the paper, where instantiating with one set of terms
generates new ground terms that feed back into the next iteration.
d7eba68 to
a8283e2
Compare
Add TIMEOUT 1200 to the set_tests_properties call in both copies of the add_test_pl_profile macro (regression/CMakeLists.txt and regression/libcprover-cpp/CMakeLists.txt). This ensures CTest will kill and report as failed any regression test that exceeds 20 minutes, preventing CI jobs from hanging indefinitely on tests that time out.
Add a -t <secs> option (and TESTPL_TIMEOUT environment variable) to test.pl for enforcing a wall-clock timeout on each individual test. The implementation replaces the system() call in run() with fork/exec, placing the child in its own process group (setpgrp). When the timeout fires via Perl's alarm(), the entire process group is killed with SIGKILL, ensuring all child processes (bash, cbmc, solvers) are cleaned up. On timeout the test is marked as failed and a synthetic output file is written so that the pattern-matching logic does not crash. The timeout defaults to 0 (disabled), preserving existing behaviour unless -t is explicitly passed or TESTPL_TIMEOUT is set.
eac08f6 to
0ae286e
Compare
Use E-matching on index expressions to instantiate quantifiers when eager instantiation (aka enumerative instantiation) cannot be applied.