[clang] Unknown array lvalue element (PR #133381)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 28 01:34:22 PDT 2025
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.gruber at gmx.de>
Date: Fri, 28 Mar 2025 07:37:59 +0100
Subject: [PATCH 1/5] 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.gruber at gmx.de>
Date: Fri, 28 Mar 2025 07:38:46 +0100
Subject: [PATCH 2/5] 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.gruber at gmx.de>
Date: Fri, 28 Mar 2025 07:45:20 +0100
Subject: [PATCH 3/5] 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.gruber at gmx.de>
Date: Fri, 28 Mar 2025 07:48:38 +0100
Subject: [PATCH 4/5] 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.gruber at gmx.de>
Date: Fri, 28 Mar 2025 09:33:54 +0100
Subject: [PATCH 5/5] 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 at -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 at -1 {{Assuming index is non-negative}}
int c = TenElements[arg + 10];
// expected-warning at -1 {{Out of bound access to memory after the end of 'TenElements'}}
// expected-note at -2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}}
More information about the cfe-commits
mailing list