[clang] Unknown array lvalue element (PR #133381)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: None (T-Gruber)

<details>
<summary>Changes</summary>

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


---
Full diff: https://github.com/llvm/llvm-project/pull/133381.diff


3 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Core/Store.cpp (+1-5) 
- (modified) clang/unittests/StaticAnalyzer/CMakeLists.txt (+1) 
- (added) clang/unittests/StaticAnalyzer/StoreManagerLValueElement.cpp (+146) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Core/Store.cpp b/clang/lib/StaticAnalyzer/Core/Store.cpp
index 5f30fae4b7047..da6885ecd0ec5 100644
--- a/clang/lib/StaticAnalyzer/Core/Store.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -511,13 +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<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());
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..c00e431d53515
--- /dev/null
+++ b/clang/unittests/StaticAnalyzer/StoreManagerLValueElement.cpp
@@ -0,0 +1,146 @@
+//===- 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

``````````

</details>


https://github.com/llvm/llvm-project/pull/133381