https://github.com/T-Gruber created https://github.com/llvm/llvm-project/pull/133381
Remove the early return for BaseRegions of type ElementRegion. Return meaningful MemRegionVal for these cases as well. Previous discussion: https://discourse.llvm.org/t/lvalueelement-returns-unknownval-for-multi-dimensional-arrays/85476 >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/4] 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/4] 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/4] 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/4] 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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
