-
Notifications
You must be signed in to change notification settings - Fork 79
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
Unsoundness with Datatype Axioms #1366
Comments
I'm guessing this has to do with the way Height interacts with Poly. We have an axiom that says |
I think you might be onto something, one of the intermediary steps in Vampire is
If we consider the sub-terms an their SMT Sorts:
So Vampire is somehow able to derive |
It seems that one follows from:
this is clearly nonsense, it's taking an 'Empty', making it into Poly, and then un-polying it but reinterpreting that as a Tuple. However I can't quite make sense of how 110 comes about. |
I think it might come down to this axiom:
|
Note that in line 91 from the proof trace:
It's calling 'Some/0' on a 'None' value. |
Yeah something is borked about the way we handle None here. The encoding has 1 Option datatype to cover every type, so only one 'None' value. Thus 'Poly(None)' needs to have every type ( But we also have a function I think the fundamental problem is that these SMT datatype constructors don't have type parameters. |
Very interesting. Technically, SMT datatypes can be polymorphic with respect to On the other hand, I wonder what is the current benefit of using solver's builtin theory for datatypes? I have been diagnosing various performance issues, and the theory instantiations (datatypes, ariths) have been difficult to deal with. |
The encoding of datatypes was recently changed to use SMT-native types. However, I think the soundness issue was probably present in the original encoding as well. Unfortunately, it probably would have much easier to fix the issue using the old encoding ... I don't know if SMT-sort polymorphism would help here. That wouldn't help with the way we do polymorphism via our |
I think the following would work:
|
Just a quick note that this is in flight, and may interact with fixes to this issue: #1359 |
@yizhou7 Thanks for finding this and generating a minimized example! I've been on vacation, so it took me a while to take a look at this. I just went through a quick attempt to prove all the axioms in the SMT file you posted. Indeed, one of the axioms is missing a precondition. Specifically, for the following:
the Below are some definitions and proofs to justify the corrected axiom, along with the other axioms, and to demonstrate that the uncorrected axiom is false. (Of course, we might want to check these in an independent tool, like F* or Dafny or Coq.)
|
I came to a similar conclusion earlier, but then I wasn't so sure. If we add the hypothesis "unbox_opt(x) is Some" to this axiom, won't it violate Verus's maxim that all functions be total? i.e., this would no longer be total:
This is why I suggested the other change about the type parameters. (Though I haven't formally validated that it's correct.) |
@tjhance Ah, I see what you mean. My quick attempt above was too quick and wouldn't cover the
|
One thing that may help with this is that, to avoid matching loops, field getters are already uninterpreted functions that wrap the raw SMT datatype getters:
So implementing this plan could be as simple as adding the type arguments to the existing field getter wrappers and updating the existing |
I think I found an unsoundness issue using empty structs. I have reduced the example to the following:
Since it doesn't really prove anything, the SMT file contains axioms only, which is not expected to create any conflicts.
I initially fond a conflict using vampire (version 4.8):
Then I manually reduced the SMT file (by removing certain axioms and all the options),
I was able to get Z3 (version 4.13.0) to reach unsat as well.
However, it is worth noting that Z3 does not reach unsat under the default options Verus uses, which means that the unsoundess is not easily reachable using pattern-based QI only.
2024-12-18-11-49-35.zip
The text was updated successfully, but these errors were encountered: