https://github.com/T-Gruber updated https://github.com/llvm/llvm-project/pull/133381
>From 241c95a956a440b9a118654baad55fb253d2413c Mon Sep 17 00:00:00 2001 From: T-Gruber <tobi.gru...@gmx.de> Date: Fri, 28 Mar 2025 07:37:59 +0100 Subject: [PATCH 1/7] Remove early return for UnknownVal --- clang/lib/StaticAnalyzer/Core/Store.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/Store.cpp b/clang/lib/StaticAnalyzer/Core/Store.cpp index 5f30fae4b7047..96e139143bc90 100644 --- a/clang/lib/StaticAnalyzer/Core/Store.cpp +++ b/clang/lib/StaticAnalyzer/Core/Store.cpp @@ -511,13 +511,10 @@ SVal StoreManager::getLValueElement(QualType elementType, NonLoc Offset, // Only allow non-integer offsets if the base region has no offset itself. // FIXME: This is a somewhat arbitrary restriction. We should be using // SValBuilder here to add the two offsets without checking their types. - if (!isa<nonloc::ConcreteInt>(Offset)) { - if (isa<ElementRegion>(BaseRegion->StripCasts())) - return UnknownVal(); - + if (!isa<nonloc::ConcreteInt>(Offset)) return loc::MemRegionVal(MRMgr.getElementRegion( elementType, Offset, cast<SubRegion>(ElemR->getSuperRegion()), Ctx)); - } + const llvm::APSInt& OffI = Offset.castAs<nonloc::ConcreteInt>().getValue(); assert(BaseIdxI.isSigned()); >From d993a7f96aebff6f558c1185d1ea6bd7834ede3f Mon Sep 17 00:00:00 2001 From: T-Gruber <tobi.gru...@gmx.de> Date: Fri, 28 Mar 2025 07:38:46 +0100 Subject: [PATCH 2/7] Unittests for StoreManager::getLValueElement --- clang/unittests/StaticAnalyzer/CMakeLists.txt | 1 + .../StoreManagerLValueElement.cpp | 148 ++++++++++++++++++ 2 files changed, 149 insertions(+) create mode 100644 clang/unittests/StaticAnalyzer/StoreManagerLValueElement.cpp diff --git a/clang/unittests/StaticAnalyzer/CMakeLists.txt b/clang/unittests/StaticAnalyzer/CMakeLists.txt index 3b01a4e9e5327..c95dc39c0c001 100644 --- a/clang/unittests/StaticAnalyzer/CMakeLists.txt +++ b/clang/unittests/StaticAnalyzer/CMakeLists.txt @@ -19,6 +19,7 @@ add_clang_unittest(StaticAnalysisTests ParamRegionTest.cpp RangeSetTest.cpp RegisterCustomCheckersTest.cpp + StoreManagerLValueElement.cpp StoreTest.cpp SymbolReaperTest.cpp SValSimplifyerTest.cpp diff --git a/clang/unittests/StaticAnalyzer/StoreManagerLValueElement.cpp b/clang/unittests/StaticAnalyzer/StoreManagerLValueElement.cpp new file mode 100644 index 0000000000000..3949a168c9284 --- /dev/null +++ b/clang/unittests/StaticAnalyzer/StoreManagerLValueElement.cpp @@ -0,0 +1,148 @@ +//===- LValueElementTest.cpp -----------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "CheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h" +#include "llvm/Support/raw_ostream.h" +#include "gtest/gtest.h" +#include <fstream> + +using namespace clang; +using namespace ento; + +namespace { + +class LValueElementChecker + : public Checker<check::PreStmt<ArraySubscriptExpr>> { +public: + void checkPreStmt(const ArraySubscriptExpr *ASE, CheckerContext &CC) const { + const Expr *BaseEx = ASE->getBase()->IgnoreParens(); + const Expr *IdxEx = ASE->getIdx()->IgnoreParens(); + + SVal BaseVal = CC.getSVal(BaseEx); + SVal IdxVal = CC.getSVal(IdxEx); + + auto IdxNonLoc = IdxVal.getAs<NonLoc>(); + ASSERT_TRUE(IdxNonLoc) << "Expect NonLoc as index SVal\n"; + + QualType ArrayT = ASE->getType(); + SVal LValue = + CC.getStoreManager().getLValueElement(ArrayT, *IdxNonLoc, BaseVal); + + if (ExplodedNode *Node = CC.generateNonFatalErrorNode(CC.getState())) { + std::string TmpStr; + llvm::raw_string_ostream TmpStream{TmpStr}; + LValue.dumpToStream(TmpStream); + auto Report = std::make_unique<PathSensitiveBugReport>(Bug, TmpStr, Node); + CC.emitReport(std::move(Report)); + } + } + +private: + const BugType Bug{this, "LValueElementBug"}; +}; + +void addLValueElementChecker(AnalysisASTConsumer &AnalysisConsumer, + AnalyzerOptions &AnOpts) { + AnOpts.CheckersAndPackages = {{"LValueElementChecker", true}}; + AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) { + Registry.addChecker<LValueElementChecker>("LValueElementChecker", "Desc", + "DocsURI"); + }); +} + +bool runLValueElementChecker(StringRef Code, std::string &Output) { + return runCheckerOnCode<addLValueElementChecker>(Code.str(), Output, + /*OnlyEmitWarnings=*/true); +} + +TEST(LValueElementTest, IdxConInt) { + StringRef Code = R"cpp( +const int index = 1; +extern int array[3]; + +void top() { + array[index]; +})cpp"; + + std::string Output; + ASSERT_TRUE(runLValueElementChecker(Code, Output)); + EXPECT_EQ(Output, "LValueElementChecker: &Element{array,1 S64b,int}\n"); +} + +TEST(LValueElementTest, IdxSymVal) { + StringRef Code = R"cpp( +extern int un_index; +extern int array[3]; + +void top() { + array[un_index]; +})cpp"; + + std::string Output; + ASSERT_TRUE(runLValueElementChecker(Code, Output)); + EXPECT_EQ(Output, + "LValueElementChecker: &Element{array,reg_$0<int un_index>,int}\n"); +} + +TEST(LValueElementTest, IdxConIntSymVal) { + StringRef Code = R"cpp( +extern int un_index; +extern int matrix[3][3]; + +void top() { + matrix[1][un_index]; +})cpp"; + + std::string Output; + ASSERT_TRUE(runLValueElementChecker(Code, Output)); + EXPECT_EQ(Output, "LValueElementChecker: &Element{Element{matrix,1 " + "S64b,int[3]},reg_$0<int un_index>,int}\n" + "LValueElementChecker: &Element{matrix,1 S64b,int[3]}\n"); +} + +TEST(LValueElementTest, IdxSymValConInt) { + StringRef Code = R"cpp( +extern int un_index; +extern int matrix[3][3]; + +void top() { + matrix[un_index][1]; +})cpp"; + + std::string Output; + ASSERT_TRUE(runLValueElementChecker(Code, Output)); + EXPECT_EQ( + Output, + "LValueElementChecker: &Element{Element{matrix,reg_$0<int " + "un_index>,int[3]},1 S64b,int}\n" + "LValueElementChecker: &Element{matrix,reg_$0<int un_index>,int[3]}\n"); +} + +TEST(LValueElementTest, IdxSymValSymVal) { + StringRef Code = R"cpp( +extern int un_index; +extern int matrix[3][3]; + +void top() { + matrix[un_index][un_index]; +})cpp"; + + std::string Output; + ASSERT_TRUE(runLValueElementChecker(Code, Output)); + EXPECT_EQ( + Output, + "LValueElementChecker: &Element{Element{matrix,reg_$0<int " + "un_index>,int[3]},reg_$0<int un_index>,int}\n" + "LValueElementChecker: &Element{matrix,reg_$0<int un_index>,int[3]}\n"); +} + +} // namespace >From 0fefdd7448836a8ff517ec877ace3e8027796ab5 Mon Sep 17 00:00:00 2001 From: T-Gruber <tobi.gru...@gmx.de> Date: Fri, 28 Mar 2025 07:45:20 +0100 Subject: [PATCH 3/7] Run clang-format --- clang/lib/StaticAnalyzer/Core/Store.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/Store.cpp b/clang/lib/StaticAnalyzer/Core/Store.cpp index 96e139143bc90..da6885ecd0ec5 100644 --- a/clang/lib/StaticAnalyzer/Core/Store.cpp +++ b/clang/lib/StaticAnalyzer/Core/Store.cpp @@ -511,10 +511,9 @@ SVal StoreManager::getLValueElement(QualType elementType, NonLoc Offset, // Only allow non-integer offsets if the base region has no offset itself. // FIXME: This is a somewhat arbitrary restriction. We should be using // SValBuilder here to add the two offsets without checking their types. - if (!isa<nonloc::ConcreteInt>(Offset)) + if (!isa<nonloc::ConcreteInt>(Offset)) return loc::MemRegionVal(MRMgr.getElementRegion( elementType, Offset, cast<SubRegion>(ElemR->getSuperRegion()), Ctx)); - const llvm::APSInt& OffI = Offset.castAs<nonloc::ConcreteInt>().getValue(); assert(BaseIdxI.isSigned()); >From 975bad2a21e050bc11af57cdfe2db419d4d941df Mon Sep 17 00:00:00 2001 From: T-Gruber <tobi.gru...@gmx.de> Date: Fri, 28 Mar 2025 07:48:38 +0100 Subject: [PATCH 4/7] Remove unneeded includes --- clang/unittests/StaticAnalyzer/StoreManagerLValueElement.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/clang/unittests/StaticAnalyzer/StoreManagerLValueElement.cpp b/clang/unittests/StaticAnalyzer/StoreManagerLValueElement.cpp index 3949a168c9284..c00e431d53515 100644 --- a/clang/unittests/StaticAnalyzer/StoreManagerLValueElement.cpp +++ b/clang/unittests/StaticAnalyzer/StoreManagerLValueElement.cpp @@ -8,12 +8,10 @@ #include "CheckerRegistration.h" #include "clang/StaticAnalyzer/Core/Checker.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h" #include "llvm/Support/raw_ostream.h" #include "gtest/gtest.h" -#include <fstream> using namespace clang; using namespace ento; >From f55db172ae9199efa4362f61a11fe22db945b3c9 Mon Sep 17 00:00:00 2001 From: T-Gruber <tobi.gru...@gmx.de> Date: Fri, 28 Mar 2025 09:33:54 +0100 Subject: [PATCH 5/7] Adapt assumption report for ArrayBound --- clang/test/Analysis/ArrayBound/assumption-reporting.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/clang/test/Analysis/ArrayBound/assumption-reporting.c b/clang/test/Analysis/ArrayBound/assumption-reporting.c index d687886ada1ae..535e623baa815 100644 --- a/clang/test/Analysis/ArrayBound/assumption-reporting.c +++ b/clang/test/Analysis/ArrayBound/assumption-reporting.c @@ -39,14 +39,9 @@ int assumingBothPointerToMiddle(int arg) { // will speak about the "byte offset" measured from the beginning of the TenElements. int *p = TenElements + 2; int a = p[arg]; - // FIXME: The following note does not appear: - // {{Assuming byte offset is non-negative and less than 40, the extent of 'TenElements'}} - // It seems that the analyzer "gives up" modeling this pointer arithmetics - // and says that `p[arg]` is just an UnknownVal (instead of calculating that - // it's equivalent to `TenElements[2+arg]`). + // expected-note@-1 {{Assuming byte offset is non-negative and less than 40, the extent of 'TenElements'}} int b = TenElements[arg]; // This is normal access, and only the lower bound is new. - // expected-note@-1 {{Assuming index is non-negative}} int c = TenElements[arg + 10]; // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} >From 4ed4661755f5e20845fd2a7479b1581731d3f1b6 Mon Sep 17 00:00:00 2001 From: T-Gruber <tobi.gru...@gmx.de> Date: Mon, 31 Mar 2025 07:19:51 +0200 Subject: [PATCH 6/7] Remove unittests --- clang/unittests/StaticAnalyzer/CMakeLists.txt | 1 - .../StoreManagerLValueElement.cpp | 146 ------------------ 2 files changed, 147 deletions(-) delete mode 100644 clang/unittests/StaticAnalyzer/StoreManagerLValueElement.cpp diff --git a/clang/unittests/StaticAnalyzer/CMakeLists.txt b/clang/unittests/StaticAnalyzer/CMakeLists.txt index c95dc39c0c001..3b01a4e9e5327 100644 --- a/clang/unittests/StaticAnalyzer/CMakeLists.txt +++ b/clang/unittests/StaticAnalyzer/CMakeLists.txt @@ -19,7 +19,6 @@ add_clang_unittest(StaticAnalysisTests ParamRegionTest.cpp RangeSetTest.cpp RegisterCustomCheckersTest.cpp - StoreManagerLValueElement.cpp StoreTest.cpp SymbolReaperTest.cpp SValSimplifyerTest.cpp diff --git a/clang/unittests/StaticAnalyzer/StoreManagerLValueElement.cpp b/clang/unittests/StaticAnalyzer/StoreManagerLValueElement.cpp deleted file mode 100644 index c00e431d53515..0000000000000 --- a/clang/unittests/StaticAnalyzer/StoreManagerLValueElement.cpp +++ /dev/null @@ -1,146 +0,0 @@ -//===- LValueElementTest.cpp -----------------------------------===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#include "CheckerRegistration.h" -#include "clang/StaticAnalyzer/Core/Checker.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h" -#include "llvm/Support/raw_ostream.h" -#include "gtest/gtest.h" - -using namespace clang; -using namespace ento; - -namespace { - -class LValueElementChecker - : public Checker<check::PreStmt<ArraySubscriptExpr>> { -public: - void checkPreStmt(const ArraySubscriptExpr *ASE, CheckerContext &CC) const { - const Expr *BaseEx = ASE->getBase()->IgnoreParens(); - const Expr *IdxEx = ASE->getIdx()->IgnoreParens(); - - SVal BaseVal = CC.getSVal(BaseEx); - SVal IdxVal = CC.getSVal(IdxEx); - - auto IdxNonLoc = IdxVal.getAs<NonLoc>(); - ASSERT_TRUE(IdxNonLoc) << "Expect NonLoc as index SVal\n"; - - QualType ArrayT = ASE->getType(); - SVal LValue = - CC.getStoreManager().getLValueElement(ArrayT, *IdxNonLoc, BaseVal); - - if (ExplodedNode *Node = CC.generateNonFatalErrorNode(CC.getState())) { - std::string TmpStr; - llvm::raw_string_ostream TmpStream{TmpStr}; - LValue.dumpToStream(TmpStream); - auto Report = std::make_unique<PathSensitiveBugReport>(Bug, TmpStr, Node); - CC.emitReport(std::move(Report)); - } - } - -private: - const BugType Bug{this, "LValueElementBug"}; -}; - -void addLValueElementChecker(AnalysisASTConsumer &AnalysisConsumer, - AnalyzerOptions &AnOpts) { - AnOpts.CheckersAndPackages = {{"LValueElementChecker", true}}; - AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) { - Registry.addChecker<LValueElementChecker>("LValueElementChecker", "Desc", - "DocsURI"); - }); -} - -bool runLValueElementChecker(StringRef Code, std::string &Output) { - return runCheckerOnCode<addLValueElementChecker>(Code.str(), Output, - /*OnlyEmitWarnings=*/true); -} - -TEST(LValueElementTest, IdxConInt) { - StringRef Code = R"cpp( -const int index = 1; -extern int array[3]; - -void top() { - array[index]; -})cpp"; - - std::string Output; - ASSERT_TRUE(runLValueElementChecker(Code, Output)); - EXPECT_EQ(Output, "LValueElementChecker: &Element{array,1 S64b,int}\n"); -} - -TEST(LValueElementTest, IdxSymVal) { - StringRef Code = R"cpp( -extern int un_index; -extern int array[3]; - -void top() { - array[un_index]; -})cpp"; - - std::string Output; - ASSERT_TRUE(runLValueElementChecker(Code, Output)); - EXPECT_EQ(Output, - "LValueElementChecker: &Element{array,reg_$0<int un_index>,int}\n"); -} - -TEST(LValueElementTest, IdxConIntSymVal) { - StringRef Code = R"cpp( -extern int un_index; -extern int matrix[3][3]; - -void top() { - matrix[1][un_index]; -})cpp"; - - std::string Output; - ASSERT_TRUE(runLValueElementChecker(Code, Output)); - EXPECT_EQ(Output, "LValueElementChecker: &Element{Element{matrix,1 " - "S64b,int[3]},reg_$0<int un_index>,int}\n" - "LValueElementChecker: &Element{matrix,1 S64b,int[3]}\n"); -} - -TEST(LValueElementTest, IdxSymValConInt) { - StringRef Code = R"cpp( -extern int un_index; -extern int matrix[3][3]; - -void top() { - matrix[un_index][1]; -})cpp"; - - std::string Output; - ASSERT_TRUE(runLValueElementChecker(Code, Output)); - EXPECT_EQ( - Output, - "LValueElementChecker: &Element{Element{matrix,reg_$0<int " - "un_index>,int[3]},1 S64b,int}\n" - "LValueElementChecker: &Element{matrix,reg_$0<int un_index>,int[3]}\n"); -} - -TEST(LValueElementTest, IdxSymValSymVal) { - StringRef Code = R"cpp( -extern int un_index; -extern int matrix[3][3]; - -void top() { - matrix[un_index][un_index]; -})cpp"; - - std::string Output; - ASSERT_TRUE(runLValueElementChecker(Code, Output)); - EXPECT_EQ( - Output, - "LValueElementChecker: &Element{Element{matrix,reg_$0<int " - "un_index>,int[3]},reg_$0<int un_index>,int}\n" - "LValueElementChecker: &Element{matrix,reg_$0<int un_index>,int[3]}\n"); -} - -} // namespace >From bdeffce2721b8bda5e91256af53340c2bdd7985e Mon Sep 17 00:00:00 2001 From: T-Gruber <tobi.gru...@gmx.de> Date: Mon, 31 Mar 2025 07:20:18 +0200 Subject: [PATCH 7/7] Implement lit tests --- clang/test/Analysis/lvalue_elements.c | 31 +++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 clang/test/Analysis/lvalue_elements.c diff --git a/clang/test/Analysis/lvalue_elements.c b/clang/test/Analysis/lvalue_elements.c new file mode 100644 index 0000000000000..73b9c037d80d2 --- /dev/null +++ b/clang/test/Analysis/lvalue_elements.c @@ -0,0 +1,31 @@ +// RUN: %clang_analyze_cc1 -std=c11 -analyzer-checker=debug.ExprInspection -verify %s + +void clang_analyzer_dump(int*); + +const int const_index = 1; +extern int unknown_index; +extern int array[3]; +extern int matrix[3][3]; + +int main(){ + + // expected-warning@+1 {{&Element{array,1 S64b,int}}} + clang_analyzer_dump(&array[const_index]); + + // expected-warning@+1 {{&Element{array,reg_$1<int unknown_index>,int}}} + clang_analyzer_dump(&array[unknown_index]); + + // expected-warning@+1 {{&Element{Element{matrix,1 S64b,int[3]},1 S64b,int}}} + clang_analyzer_dump(&matrix[const_index][const_index]); + + // expected-warning@+1 {{&Element{Element{matrix,reg_$1<int unknown_index>,int[3]},1 S64b,int}}} + clang_analyzer_dump(&matrix[unknown_index][const_index]); + + // expected-warning@+1 {{&Element{Element{matrix,1 S64b,int[3]},reg_$1<int unknown_index>,int}}} + clang_analyzer_dump(&matrix[const_index][unknown_index]); + + // expected-warning@+1 {{&Element{Element{matrix,reg_$1<int unknown_index>,int[3]},reg_$1<int unknown_index>,int}}} + clang_analyzer_dump(&matrix[unknown_index][unknown_index]); + + return 0; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits