https://github.com/marco-antognini-sonarsource updated https://github.com/llvm/llvm-project/pull/153177
>From fdbe7367059478fde53a5cc5d8dc8342cfa09f67 Mon Sep 17 00:00:00 2001 From: Marco Borgeaud <[email protected]> Date: Tue, 12 Aug 2025 11:40:25 +0200 Subject: [PATCH 1/6] [analyzer] Harden RegionStoreManager::bindArray Fixes https://github.com/llvm/llvm-project/issues/147686 by completing handling of literals and handling symbolic values similarly to bindStruct. CPP-6688 --- clang/lib/StaticAnalyzer/Core/RegionStore.cpp | 9 +++- clang/test/Analysis/initializer.cpp | 41 +++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index 02375b0c3469a..ebe1a264e40f8 100644 --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -2654,14 +2654,19 @@ RegionStoreManager::bindArray(LimitedRegionBindingsConstRef B, SVal V = getBinding(B.asStore(), *MRV, R->getValueType()); return bindAggregate(B, R, V); } + if (auto const *Value = Init.getAsInteger()) { + auto SafeValue = StateMgr.getBasicVals().getValue(*Value); + return bindAggregate(B, R, nonloc::ConcreteInt(SafeValue)); + } - // Handle lazy compound values. + // Handle lazy compound values and symbolic values. if (std::optional LCV = Init.getAs<nonloc::LazyCompoundVal>()) { if (std::optional NewB = tryBindSmallArray(B, R, AT, *LCV)) return *NewB; - return bindAggregate(B, R, Init); } + if (isa<nonloc::SymbolVal>(Init)) + return bindAggregate(B, R, Init); if (Init.isUnknown()) return bindAggregate(B, R, UnknownVal()); diff --git a/clang/test/Analysis/initializer.cpp b/clang/test/Analysis/initializer.cpp index 713e121168571..ee90705ac3d28 100644 --- a/clang/test/Analysis/initializer.cpp +++ b/clang/test/Analysis/initializer.cpp @@ -610,3 +610,44 @@ void top() { consume(parseMatchComponent()); } } // namespace elementwise_copy_small_array_from_post_initializer_of_cctor + +namespace gh147686 { +// The problem reported in https://github.com/llvm/llvm-project/issues/147686 +// is sensitive to the initializer form: using parenthesis to initialize m_ptr +// resulted in crashes when analyzing *m_ptr = '\0'; but using braces is fine. + +struct A { + A() : m_ptr(m_buf) { *m_ptr = '\0'; } // no-crash + A(int overload) : m_ptr{m_buf} { *m_ptr = '\0'; } + A(char src) : m_ptr(m_buf) { *m_ptr = src; } // no-crash + A(char src, int overload) : m_ptr{m_buf} { *m_ptr = src; } + char m_buf[64] = {0}; + char * m_ptr; +}; + +void test1() { + A a; + clang_analyzer_eval(a.m_buf[0] == 0); // expected-warning{{TRUE}} + // FIXME The next eval should result in TRUE. + clang_analyzer_eval(*a.m_ptr == 0); // expected-warning{{UNKNOWN}} +} + +void test2() { + A a(314); + clang_analyzer_eval(a.m_buf[0] == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(*a.m_ptr == 0); // expected-warning{{TRUE}} +} + +void test3() { + A a(0); + clang_analyzer_eval(a.m_buf[0] == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(*a.m_ptr == 0); // expected-warning{{TRUE}} +} + +void test4() { + A a(0, 314); + clang_analyzer_eval(a.m_buf[0] == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(*a.m_ptr == 0); // expected-warning{{TRUE}} +} + +} // namespace gh147686 >From d22fc9437042ba3b2ee48bf994e55ab7073ea17a Mon Sep 17 00:00:00 2001 From: Marco Borgeaud <[email protected]> Date: Wed, 1 Oct 2025 16:59:27 +0200 Subject: [PATCH 2/6] Address feedback: spell out type --- clang/lib/StaticAnalyzer/Core/RegionStore.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index 916c2f9022884..96325b06159e7 100644 --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -2657,7 +2657,7 @@ RegionStoreManager::bindArray(LimitedRegionBindingsConstRef B, SVal V = getBinding(B.asStore(), *MRV, R->getValueType()); return bindAggregate(B, R, V); } - if (auto const *Value = Init.getAsInteger()) { + if (llvm::APSInt const *Value = Init.getAsInteger()) { auto SafeValue = StateMgr.getBasicVals().getValue(*Value); return bindAggregate(B, R, nonloc::ConcreteInt(SafeValue)); } >From 472de428485fbc7ea79b2f0f45e25de8b6a20af0 Mon Sep 17 00:00:00 2001 From: Marco Borgeaud <[email protected]> Date: Wed, 1 Oct 2025 17:00:20 +0200 Subject: [PATCH 3/6] Address feedback: remove dubious comment --- clang/lib/StaticAnalyzer/Core/RegionStore.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index 96325b06159e7..4d952ab6b4427 100644 --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -2662,7 +2662,6 @@ RegionStoreManager::bindArray(LimitedRegionBindingsConstRef B, return bindAggregate(B, R, nonloc::ConcreteInt(SafeValue)); } - // Handle lazy compound values and symbolic values. if (std::optional LCV = Init.getAs<nonloc::LazyCompoundVal>()) { if (std::optional NewB = tryBindSmallArray(B, R, AT, *LCV)) return *NewB; >From 82a7fc6957ab4f6b985a178ee8d268c656b9d185 Mon Sep 17 00:00:00 2001 From: Marco Borgeaud <[email protected]> Date: Wed, 1 Oct 2025 17:11:21 +0200 Subject: [PATCH 4/6] Address feedback: use proper symbolic value for test4 --- clang/test/Analysis/initializer.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/test/Analysis/initializer.cpp b/clang/test/Analysis/initializer.cpp index ee90705ac3d28..962431a66c7b3 100644 --- a/clang/test/Analysis/initializer.cpp +++ b/clang/test/Analysis/initializer.cpp @@ -644,10 +644,10 @@ void test3() { clang_analyzer_eval(*a.m_ptr == 0); // expected-warning{{TRUE}} } -void test4() { - A a(0, 314); - clang_analyzer_eval(a.m_buf[0] == 0); // expected-warning{{TRUE}} - clang_analyzer_eval(*a.m_ptr == 0); // expected-warning{{TRUE}} +void test4(char arg) { + A a(arg, 314); + clang_analyzer_eval(a.m_buf[0] == arg); // expected-warning{{TRUE}} + clang_analyzer_eval(*a.m_ptr == arg); // expected-warning{{TRUE}} } } // namespace gh147686 >From f5761ec97ab15c5cf5ea00f5d6ab28b3ef77714d Mon Sep 17 00:00:00 2001 From: Marco Borgeaud <[email protected]> Date: Wed, 1 Oct 2025 17:18:29 +0200 Subject: [PATCH 5/6] Address feedback: document current inconsistency --- clang/test/Analysis/initializer.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/clang/test/Analysis/initializer.cpp b/clang/test/Analysis/initializer.cpp index 962431a66c7b3..88758f7c3ac1d 100644 --- a/clang/test/Analysis/initializer.cpp +++ b/clang/test/Analysis/initializer.cpp @@ -644,6 +644,13 @@ void test3() { clang_analyzer_eval(*a.m_ptr == 0); // expected-warning{{TRUE}} } +void test3Bis(char arg) { + A a(arg); + // FIXME This test should behave like test3. + clang_analyzer_eval(a.m_buf[0] == arg); // expected-warning{{FALSE}} // expected-warning{{TRUE}} + clang_analyzer_eval(*a.m_ptr == arg); // expected-warning{{UNKNOWN}} +} + void test4(char arg) { A a(arg, 314); clang_analyzer_eval(a.m_buf[0] == arg); // expected-warning{{TRUE}} >From 9392138785970483f749d14eababeb8cddfb2059 Mon Sep 17 00:00:00 2001 From: Marco Borgeaud <[email protected]> Date: Wed, 1 Oct 2025 17:29:13 +0200 Subject: [PATCH 6/6] Address feedback: simplify bindArray & add FIXME No need to unwrap constants. This is a workaround to not crash -- not a proper fix. --- clang/lib/StaticAnalyzer/Core/RegionStore.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index 4d952ab6b4427..af0ef52334bd7 100644 --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -2657,16 +2657,18 @@ RegionStoreManager::bindArray(LimitedRegionBindingsConstRef B, SVal V = getBinding(B.asStore(), *MRV, R->getValueType()); return bindAggregate(B, R, V); } - if (llvm::APSInt const *Value = Init.getAsInteger()) { - auto SafeValue = StateMgr.getBasicVals().getValue(*Value); - return bindAggregate(B, R, nonloc::ConcreteInt(SafeValue)); - } + + // FIXME Single value constant should have been handled before this call to + // bindArray. This is only a hotfix to not crash. + if (Init.isConstant()) + return bindAggregate(B, R, Init); if (std::optional LCV = Init.getAs<nonloc::LazyCompoundVal>()) { if (std::optional NewB = tryBindSmallArray(B, R, AT, *LCV)) return *NewB; return bindAggregate(B, R, Init); } + if (isa<nonloc::SymbolVal>(Init)) return bindAggregate(B, R, Init); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
