[clang] [analyzer] Unknown array lvalue element in Store (PR #133381)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Mar 30 22:20:49 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/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.gruber at 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.gruber at 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.gruber at 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.gruber at 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 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}}
>From 4ed4661755f5e20845fd2a7479b1581731d3f1b6 Mon Sep 17 00:00:00 2001
From: T-Gruber <tobi.gruber at 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.gruber at 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 at +1 {{&Element{array,1 S64b,int}}}
+ clang_analyzer_dump(&array[const_index]);
+
+ // expected-warning at +1 {{&Element{array,reg_$1<int unknown_index>,int}}}
+ clang_analyzer_dump(&array[unknown_index]);
+
+ // expected-warning at +1 {{&Element{Element{matrix,1 S64b,int[3]},1 S64b,int}}}
+ clang_analyzer_dump(&matrix[const_index][const_index]);
+
+ // expected-warning at +1 {{&Element{Element{matrix,reg_$1<int unknown_index>,int[3]},1 S64b,int}}}
+ clang_analyzer_dump(&matrix[unknown_index][const_index]);
+
+ // expected-warning at +1 {{&Element{Element{matrix,1 S64b,int[3]},reg_$1<int unknown_index>,int}}}
+ clang_analyzer_dump(&matrix[const_index][unknown_index]);
+
+ // expected-warning at +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;
+}
More information about the cfe-commits
mailing list