Z3Prover / z3

The Z3 Theorem Prover
Other
10.3k stars 1.48k forks source link

infinite loop making model for recursive datatype in presence of quantifiers #578

Open smcpeak opened 8 years ago

smcpeak commented 8 years ago

If I give Z3 a recursive data type and a condition that is unsatisfiable for any finite instance, Z3 will go into an infinite loop during model creation, eventually timing out. The loop happens because it is trying all possible finite values, of which there are an infinite number of course. I would like Z3 to instead quickly stop and say "unknown".

I have a candidate fix for this, but it causes three tests in z3test to fail, so I know it isn't completely right.

My reduced input example is attached as rdp-min.smt2.txt:

rdp-min.smt2.txt

When I run Z3 version 4.4.1 like so:

  $ z3 -t:2000 rdp-min.smt2
  unknown

it runs for about 2 seconds and reports "unknown".

When I add some tracing:

  $ z3 -t:2000 -v:100 -tr:datatype rdp-min.smt2
  [...]
  (smt.final-check "datatype")
  (smt.stats :time 2.01 :before-memory 1.72 :after-memory 2.00)
  (tactic-exception "smt tactic failed to show goal to be sat/unsat")
  unknown

the timeout is clear, and near the end of .z3-trace, there is:

  -------- [datatype] internalize_term ../src/smt/theory_datatype.cpp:231 ---------
  internalizing term:
  (let ((a!1 (restOf (restOf (restOf (restOf theList))))))
  (let ((a!2 (restOf (restOf (restOf (restOf a!1))))))
  (let ((a!3 (restOf (restOf (restOf (restOf a!2))))))
  (let ((a!4 (restOf (restOf (restOf (restOf a!3))))))
  (let ((a!5 (restOf (restOf (restOf (restOf a!4))))))
  (let ((a!6 (restOf (restOf (restOf (restOf a!5))))))
  (let ((a!7 (restOf (restOf (restOf (restOf a!6))))))
  (let ((a!8 (restOf (restOf (restOf (restOf a!7))))))
  (let ((a!9 (restOf (restOf (restOf (restOf a!8))))))
  (let ((a!10 (restOf (restOf (restOf (restOf a!9))))))
    (is-End (restOf (restOf (restOf a!10))))))))))))))
  ------------------------------------------------

The above term has 43 "restOf" applications; if I let it run for 20s instead, it times out after building a list with length of about 140.

Stepping through the code in gdb, what's happening seems fairly straightforward. The prover core first thinks the formula is satisfiable, and asks each theory to help build a model. The "datatype" theory sees a term with indeterminate constructor, "(restOf theList)", and forces a case split on its constructor. The "End" case is quickly rejected, but the "Elem" case creates "(restOf (restOf theList))", which again has indeterminate constructor. When the attempt to build a model resumes, the process repeats, building longer and longer lists, until Z3 times out.

I have a candidate fix, attached as z3-rdp-changes.txt:

z3-rdp-changes.txt

With these changes, my reduced test case works:

  $ z3 -t:2000 -v:100 -tr:datatype rdp-min.smt2
  [...]
  (smt.stats :time 0.00 :before-memory 1.72 :after-memory 1.72)
  (tactic-exception "smt tactic failed to show goal to be sat/unsat")
  unknown

However, these tests in z3test fail:

  regressions/smt2/z3.17.smt2
  regressions/smt2/mut_rec4sdk_output1.smt2
    expected "sat", got "unknown"

  regressions/smt2/z3.16.smt2
    expected "sat, unsat", got "unknown, unknown"

It would seem that I need to allow Z3 to do bounded unfolding of recursive data types, but I don't know how, so I'd appreciate feedback on whether this is a good approach, and if so, suggestions on how to add an unfolding limit.

NikolajBjorner commented 8 years ago

Thanks for the useful example. The way the datatype decision procedure performs splitting on recursive data-types is a decision procedure when the formulas are quantifier-free. For formulas with quantifiers, no termination guarantees are provided. The example exhibits a case where the procedure that builds a finite model for quantifier-free recursive data-types fails to terminate. The quantified formula keeps getting instantiated by freshly generated terms, and the instantiation causes the decision procedure to create fresh terms that cause additional instantiation. Regarding the code: you can get the sort of a theory variable without using m_find because the types are all the same within an equivalence class (assuming Z3 isn't buggy here). The more crucial part of your change is to quickly give up building models for the quantifier-free formula. This causes the discrepancies in the unit tests. It is possible that there are ways to have both termination in more such quantifier cases and also retain the ground decision procedure, but I can't say without digging in what it may be like.

smcpeak commented 8 years ago

Ok, thanks for the explanation and acknowledgment this behavior is not unexpected.

Since the novel part of my example evidently is the interaction of quantifiers and the datatype module, I'm thinking of a fix like this:

Based on your explanation above, this should never interfere with cases where the DP in 'datatype' is complete (namely, quantifier-free formulae), since the counter for quantifier instantiation would remain at zero. But in my example, after about 1s, the threshold for both would be hit, and hence Z3 would give up.

Thoughts?

NikolajBjorner commented 8 years ago

Maybe possible, but note that you can control the maximal instantiations of quantifiers. For example,

        z3.exe rdp-min.smt2 smt.qi.max_instances=100

Having cross-cutting parameters becomes a bit trickier to get right and maintain.

smcpeak commented 8 years ago

Thanks for the suggestion. I think I've got a satisfactory patch now:

changes.txt

This adds a limit on datatype case splits for recursive types only if quantifiers are present. datatype already does something related in apply_sort_cnstr(). The new parameter thus adds no new cross-cutting dependencies.

On rdp-min.smt2, Z3 now terminates fairly quickly:

$ z3 -t:2000 -v:100 -tr:datatype rdp-min.smt2
[...]
(smt.final-check "datatype")
(smt.stats :time 0.17 :before-memory 1.72 :after-memory 1.81)
(tactic-exception "smt tactic failed to show goal to be sat/unsat")
unknown

This change (which I have made on top of z3-4.4.1, approximately) also passes all of Z3's tests.

The default value of the new parameter is 10, causing Z3 to terminate on rdp-min.smt2 after about 0.2 seconds, which seems like a reasonable amount of time to me on that example, and is the reason I chose 10. However, Z3 will pass all of its current tests even with the parameter at 0, which inhibits all recursive type case splits in the presence of quantifiers.

If this change seems good to you, I'll go ahead and make pull requests for z3 and z3test. (The latter to add rdp-min.smt2. Is there a preferred naming convention? The existing names don't have any obvious structure.)

Also, I know I need the signed contributor agreement. Synopsys Legal has been sitting on it for two weeks. I'll wait on any pull request until that's done.

smcpeak commented 8 years ago

There are some problems with my most recent candidate fix.

First, I misunderstood what the code means by an "infinite" sort, thinking it was equivalent to "recursive", but a trivial tuple of Int is considered infinite but not recursive. So, the two places I used is_infinite() should have been m_util.is_recursive().

More importantly, there are additional cases where model creation fails assertions that aren't handled by the change I made to theory_datatype::mk_value(). I don't really understand why, other than that there is some apparent periodicity to model construction that causes other cases to arise. Therefore, my latest iteration changes context::search() to bail out of model creation whenever I return FC_GIVEUP from theory_datatype:

  status = bounded_search();
  TRACE("search_bug", tout << "status: " << status << ", inconsistent: " << inconsistent() << "\n";);
  TRACE("assigned_literals_per_lvl", display_num_assigned_literals_per_lvl(tout);
        tout << ", num_assigned: " << m_assigned_literals.size() << "\n";);

  if (m_last_search_failure != OK) {
      if (status != l_false) {
          // ***** ADDED THE FOLLOWING CONDITION *****
          if (m_last_search_failure == THEORY && status == l_undef) {
              // smcpeak: This happens for my datatype fix.
              // Attempting to build a model just leads to
              // more failed assertions, so bail entirely.
              break;
          }

But that repair simply exposes a new problem, which is that case splitting isn't the only way that datatype makes new terms, so I'm still getting infinte loops on bigger examples. In particular, assert_is_constructor_axiom(), assert_accessor_axioms(), assert_update_field_axioms(), and propagate_recognizer() all create new terms (via mk_app), and hence also need limits in order to not cause a matching loop in the presence of certain quantifiers and patterns. It's not obvious how to modify these methods to refrain from term creation without breaking invariants, nor whether the end result will still be useful as a decision tactic after all the hacks.

At this point I'm inclined to simply abandon recursive types altogether as incompatible with quantifiers (and instead just emulate the parts I need with more primitive mechanisms), but if someone has suggestions about how to make further fixes, I'll give them a shot.

NikolajBjorner commented 8 years ago

Sounds like material for nice social brainstorming.