-
Notifications
You must be signed in to change notification settings - Fork 85
Simplify witness invariants for bitfield domain #1897
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
base: master
Are you sure you want to change the base?
Conversation
| line: 5 | ||
| column: 3 | ||
| function: main | ||
| value: (x & (_Bool)1) == (_Bool)0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong for a top _Bool, but I'm having hard time understanding why.
The representation of top int is 0b?...?, (zs:-1, os:-1), but the representation of top _Bool is 0b0...0?, (zs:-1, os:1). These are not consistent: for int it has an infinite sequence of unknown bits (including higher bits that fit into int), but for _Bool it's one unknown bit with infinitely many definite zero bits in front.
I suspect this causes something to go wrong because the invariant generation is made to believe that there are all these definite 0 bits, but they're outside of the type range.
What are the bits outside of type size supposed to be?
I'm not sure who supervised #1623, @michael-schwarz, @jerhard? Does anyone know
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it was @jerhard and @DrMichaelPetter who supervised it, I only sat in on some meetings, so this may be wrong:
Iirc, the idea is that it's mathematical integers and the two's complement just proceeds infinitely long (which is consistent with what happens on upcast iirc)? Not sure if this helps clarify?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For mathematical integers I don't see an issue: they have infinitely many bits and it makes sense to speak about all of them, whether they can be zero or one. And it's very convenient that Zarith has this neat representation with infinite leading ones.
The problem is that this doesn't define what the behavior is supposed to be on machine integers.
Can the 1000th bit of int be a zero or a one? (My comment implies says it may be either.)
What about the 1000th bit of _Bool? (My comment implies it must be zero.)
What does this mean? I don't know. Neither type actually has such bit, so it's not like the concrete semantics forces such behavior. One could even argue that all bits above the type size should be bot because they can't be either because they don't exist.
(In which case both the zeros and ones mask have leading 0s. So none of the Zarith two's complement trickery would even be needed.)
Here's what we currently do for top values:
analyzer/src/cdomain/value/cdomains/int/bitfieldDomain.ml
Lines 196 to 198 in 3988737
| let top_of ?bitfield ik = (* TODO: use bitfield *) | |
| if GoblintCil.isSigned ik then top () | |
| else (BArith.one_mask, Ints_t.of_bigint (snd (Size.range ik))) |
Signed types give an abstraction which allows infinitely many zeros and ones. For unsigned types, it may be infinitely may zeros, but only ones up to the type size (which means higher bits will definitely be zero).
If I change that function to return always top (), no test fails. So there's no clear reason for the distinction.
I tried looking at the cited Miné paper, but I didn't immediately see an answer: it defines abstract domains on mathematical integers and then just slaps a wrap operation around every operation or something.
However, we carry the ikind together with the abstract value, instead of building abstract domains on mathematical integers, so it's not exactly the same anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I change that function to return always
top (), no test fails.
I think this may just be because our tests are poor. I think the case where it would make a difference is if you do an upcast from one unsigned type to a bigger one. There, because of our definition hopefully the upper bound would survive?
But overall, I'm also not sure what the best / most helpful thing would be.
I just happened to look into a witness we now produce with the portfolio at a level which enables the bitfield domain and saw lots of trivial invariants like
(i & 0) == 0.The non-relational witness invariant simplification (#1517) happened before the bitfield domain was introduced. The latter wasn't added to witness invariant tests, so this went unnoticed.
This is a bit unfortunate because we now have a bunch of logic for simplifying non-relational and relational witness invariants, but then we also added these, so the SV-COMP 2026 witnesses might again be unnecessarily large...