================
@@ -2187,7 +2187,10 @@ std::optional<SVal>
RegionStoreManager::getBindingForDerivedDefaultValue(
// Lazy bindings are usually handled through getExistingLazyBinding().
// We should unify these two code paths at some point.
- if (isa<nonloc::LazyCompoundVal, nonloc::CompoundVal>(val))
+ // 'nonloc::ConcreteInt' values can arise from compound literals in
+ // designated initializers for bitfields in unions.
+ if (isa<nonloc::LazyCompoundVal, nonloc::CompoundVal, nonloc::ConcreteInt>(
+ val))
----------------
steakhal wrote:
Sorry for my very late reply.
>> Could you help me with a Store for which the crash happened without the fix?
> I'm not sure what do you mean by "a Store", minimal repro goes like this:
> https://godbolt.org/z/8z1vqf6Wv.
> We get the crush as soon as try to get default binding when encountered a
> copied struct.
When you have the crash, walk up a couple stack frames until you see the
ProgramState, frequently denoted by a variable named `State`. If you call
`State->dump()` on it, you will have a couple things printed, including the
Store and Environment (in other words, the memory and the registers of the
abstract machine, respectively) along with some other stuff like checker state
and the constraint system, but those are not relevant in this case.
I wanted to have a look at it when the crash happened.
> The fix may be a little "light-minded", I'll try to investigate more on the
> issue with other NonLocs that could get beside ConcreteInt. Also, can some
> deeper logic be broken, and we should never expect ConcreteInt right here?
By the looks of the code from where this unreachable crash refers to in the
godbolt link suggests to me that the code originally had no intention of
handling other ConcreteInts than zero.
I think I managed to dump it, like [this](https://godbolt.org/z/eTd11and3).
```json
"store": { "pointer": "0x11a3f940", "items": [
{ "cluster": "SymRegion{conj_$0{int &, LC1, S945, #0}}", "pointer":
"0x11930820", "items": [
{ "kind": "Direct", "offset": 0, "value": "0 S32b" }
]},
{ "cluster": "tmp", "pointer": "0x11a3fa50", "items": [
{ "kind": "Default", "offset": 0, "value": "1 S32b" }
]}
]}
```
After thinking about this for a while, we should not have here a ConcreteInt of
1.
I bisected this crash to 9b2ec87f5bce57cc900cf52a99f805d999716053 (#116840),
thanks to manyclangs. And it starts to make sense now. Basically, in this
particular case we want to copy a `CompoundVal{CompoundVal{1}}`, and I should
have only elided one level of `CompoundVal` I think. And now, it collapses down
to `1` thus we bind that as a default binding instead.
What I should have done in that patch was to check if the value is either of
`Symbol, Zero, Unknown, LazyCompoundVal, CompoundVal`, to preserve the
invariant that a default binding should only ever refer to one of these.
https://github.com/llvm/llvm-project/pull/146418
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
