https://github.com/vbvictor updated https://github.com/llvm/llvm-project/pull/146418
>From c1083bbdd281a686a6d38585ac201735f244090f Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Mon, 30 Jun 2025 23:34:33 +0300 Subject: [PATCH 1/3] [analyzer] Fix crash on compound literals with bitfields in unions --- clang/docs/ReleaseNotes.rst | 3 + clang/lib/StaticAnalyzer/Core/RegionStore.cpp | 5 +- clang/test/Analysis/fields.c | 96 +++++++++++++++++++ 3 files changed, 103 insertions(+), 1 deletion(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index ec51ffddce1af..760f86b8e8bf2 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -227,6 +227,9 @@ New features Crash and bug fixes ^^^^^^^^^^^^^^^^^^^ +- Fixed a crash when analyzing default bindings as compound literals in + designated initializers for bitfields in unions. (#GH146050) + Improvements ^^^^^^^^^^^^ diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index 388034b087789..288c53b375f28 100644 --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -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)) return val; llvm_unreachable("Unknown default value"); diff --git a/clang/test/Analysis/fields.c b/clang/test/Analysis/fields.c index 203c30c5960a1..66c882cdba1f3 100644 --- a/clang/test/Analysis/fields.c +++ b/clang/test/Analysis/fields.c @@ -113,6 +113,102 @@ void testBitfields(void) { } +struct BitfieldUnion { + union { + struct { + unsigned int addr : 22; + unsigned int vf : 1; + }; + unsigned int raw; + }; +}; + +struct BitfieldUnion processBitfieldUnion(struct BitfieldUnion r) { + struct BitfieldUnion result = r; + result.addr += 1; + return result; +} + +void testBitfieldUnionCompoundLiteral(void) { + struct BitfieldUnion r1 = processBitfieldUnion((struct BitfieldUnion){.addr = 100, .vf = 1}); + clang_analyzer_eval(r1.addr == 101); // expected-warning{{TRUE}} + clang_analyzer_eval(r1.vf == 1); // expected-warning{{UNKNOWN}} + + struct BitfieldUnion r2 = processBitfieldUnion((struct BitfieldUnion){.addr = 1}); + clang_analyzer_eval(r2.addr == 2); // expected-warning{{TRUE}} + clang_analyzer_eval(r2.vf); // expected-warning{{UNKNOWN}} +} + +struct NestedBitfields { + struct { + unsigned x : 16; + unsigned y : 16; + } inner; +}; + +struct NestedBitfields processNestedBitfields(struct NestedBitfields n) { + struct NestedBitfields tmp = n; + tmp.inner.x += 1; + return tmp; +} + +void testNestedBitfields(void) { + struct NestedBitfields n1 = processNestedBitfields((struct NestedBitfields){.inner.x = 1}); + clang_analyzer_eval(n1.inner.x == 2); // expected-warning{{TRUE}} + clang_analyzer_eval(n1.inner.y == 0); // expected-warning{{TRUE}} + + struct NestedBitfields n2 = processNestedBitfields((struct NestedBitfields){{200, 300}}); + clang_analyzer_eval(n2.inner.x == 201); // expected-warning{{TRUE}} + clang_analyzer_eval(n2.inner.y == 300); // expected-warning{{TRUE}} +} + +struct UnionContainerBitfields { + union { + unsigned val; + struct { + unsigned x : 16; + unsigned y : 16; + }; + } u; +}; + +struct UnionContainerBitfields processUnionContainer(struct UnionContainerBitfields c) { + struct UnionContainerBitfields tmp = c; + tmp.u.x += 1; + return tmp; +} + +void testUnionContainerBitfields(void) { + struct UnionContainerBitfields c1 = processUnionContainer((struct UnionContainerBitfields){.u.val = 100}); + clang_analyzer_eval(c1.u.x == 101); // expected-warning{{FALSE}} // expected-warning{{TRUE}} + + struct UnionContainerBitfields c2 = processUnionContainer((struct UnionContainerBitfields){.u.x = 100}); + clang_analyzer_eval(c2.u.x == 101); // expected-warning{{TRUE}} +} + +struct MixedBitfields { + unsigned char x; + unsigned y : 12; + unsigned z : 20; +}; + +struct MixedBitfields processMixedBitfields(struct MixedBitfields m) { + struct MixedBitfields tmp = m; + tmp.y += 1; + return tmp; +} + +void testMixedBitfields(void) { + struct MixedBitfields m1 = processMixedBitfields((struct MixedBitfields){.x = 100, .y = 100}); + clang_analyzer_eval(m1.x == 100); // expected-warning{{TRUE}} + clang_analyzer_eval(m1.y == 101); // expected-warning{{TRUE}} + + struct MixedBitfields m2 = processMixedBitfields((struct MixedBitfields){.z = 100}); + clang_analyzer_eval(m2.y == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(m2.z == 100); // expected-warning{{TRUE}} +} + + //----------------------------------------------------------------------------- // Incorrect behavior //----------------------------------------------------------------------------- >From f3fdef02b6409e27b895a1b3c46e1aec5595574a Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Fri, 4 Jul 2025 02:41:58 +0300 Subject: [PATCH 2/3] separate ConcreteInt case from CompoundVal case. --- clang/lib/StaticAnalyzer/Core/RegionStore.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index 288c53b375f28..390fdb2c457b8 100644 --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -2187,10 +2187,12 @@ std::optional<SVal> RegionStoreManager::getBindingForDerivedDefaultValue( // Lazy bindings are usually handled through getExistingLazyBinding(). // We should unify these two code paths at some point. - // 'nonloc::ConcreteInt' values can arise from compound literals in + if (isa<nonloc::LazyCompoundVal, nonloc::CompoundVal>(val)) + return val; + + // 'nonloc::ConcreteInt' values can arise from e.g. compound literals in // designated initializers for bitfields in unions. - if (isa<nonloc::LazyCompoundVal, nonloc::CompoundVal, nonloc::ConcreteInt>( - val)) + if (isa<nonloc::ConcreteInt>(val)) return val; llvm_unreachable("Unknown default value"); >From 66593be87a9c5eb52a7eaf81d8d9dd849130ac6e Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Fri, 25 Jul 2025 17:16:51 +0300 Subject: [PATCH 3/3] fix rootcause problem --- clang/lib/StaticAnalyzer/Core/RegionStore.cpp | 19 +++++++++++++------ clang/test/Analysis/fields.c | 2 +- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index 390fdb2c457b8..fcd2c03f5beb3 100644 --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -2190,11 +2190,6 @@ std::optional<SVal> RegionStoreManager::getBindingForDerivedDefaultValue( if (isa<nonloc::LazyCompoundVal, nonloc::CompoundVal>(val)) return val; - // 'nonloc::ConcreteInt' values can arise from e.g. compound literals in - // designated initializers for bitfields in unions. - if (isa<nonloc::ConcreteInt>(val)) - return val; - llvm_unreachable("Unknown default value"); } @@ -2763,7 +2758,19 @@ RegionStoreManager::getUniqueDefaultBinding(RegionBindingsConstRef B, return std::nullopt; const auto [Key, Value] = *Cluster->begin(); - return Key.isDirect() ? std::optional<SVal>{} : Value; + if (Key.isDirect()) + return std::nullopt; + + // Preserve the invariant that default bindings should only be one of: + // Symbol, Zero, Unknown, LazyCompoundVal, CompoundVal + // This prevents over-collapsing of CompoundVals e.g. + // CompoundVal{CompoundVal{1}} -> 1 + if (Value.isUnknownOrUndef() || Value.isZeroConstant() || + isa<nonloc::SymbolVal>(Value) || + isa<nonloc::LazyCompoundVal, nonloc::CompoundVal>(Value)) + return Value; + + return std::nullopt; } std::optional<SVal> diff --git a/clang/test/Analysis/fields.c b/clang/test/Analysis/fields.c index 66c882cdba1f3..d3174cd6cc26d 100644 --- a/clang/test/Analysis/fields.c +++ b/clang/test/Analysis/fields.c @@ -180,7 +180,7 @@ struct UnionContainerBitfields processUnionContainer(struct UnionContainerBitfie void testUnionContainerBitfields(void) { struct UnionContainerBitfields c1 = processUnionContainer((struct UnionContainerBitfields){.u.val = 100}); - clang_analyzer_eval(c1.u.x == 101); // expected-warning{{FALSE}} // expected-warning{{TRUE}} + clang_analyzer_eval(c1.u.x == 101); // expected-warning{{TRUE}} struct UnionContainerBitfields c2 = processUnionContainer((struct UnionContainerBitfields){.u.x = 100}); clang_analyzer_eval(c2.u.x == 101); // expected-warning{{TRUE}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits