[clang] [analyzer] Fix a crash from element region construction during `ArrayInitLoopExpr` analysis (PR #113570)

via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 24 07:08:19 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: None (isuckatcs)

<details>
<summary>Changes</summary>

This patch generalizes the way element regions are constructed when an `ArrayInitLoopExpr` is being analyzed. Previously the base region of the `ElementRegion` was determined with pattern matching, which led to crashes, when an unhandled pattern was encountered.

Fixes #<!-- -->112813

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


2 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (+12-57) 
- (modified) clang/test/Analysis/array-init-loop.cpp (+38) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index c50db1e0e2f863..6ddb9b44ddcf91 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -513,70 +513,25 @@ ProgramStateRef ExprEngine::updateObjectsUnderConstruction(
 static ProgramStateRef
 bindRequiredArrayElementToEnvironment(ProgramStateRef State,
                                       const ArrayInitLoopExpr *AILE,
-                                      const LocationContext *LCtx, SVal Idx) {
-  // The ctor in this case is guaranteed to be a copy ctor, otherwise we hit a
-  // compile time error.
-  //
-  //  -ArrayInitLoopExpr                <-- we're here
-  //   |-OpaqueValueExpr
-  //   | `-DeclRefExpr                  <-- match this
-  //   `-CXXConstructExpr
-  //     `-ImplicitCastExpr
-  //       `-ArraySubscriptExpr
-  //         |-ImplicitCastExpr
-  //         | `-OpaqueValueExpr
-  //         |   `-DeclRefExpr
-  //         `-ArrayInitIndexExpr
-  //
-  // The resulting expression might look like the one below in an implicit
-  // copy/move ctor.
-  //
-  //   ArrayInitLoopExpr                <-- we're here
-  //   |-OpaqueValueExpr
-  //   | `-MemberExpr                   <-- match this
-  //   |  (`-CXXStaticCastExpr)         <-- move ctor only
-  //   |     `-DeclRefExpr
-  //   `-CXXConstructExpr
-  //     `-ArraySubscriptExpr
-  //       |-ImplicitCastExpr
-  //       | `-OpaqueValueExpr
-  //       |   `-MemberExpr
-  //       |     `-DeclRefExpr
-  //       `-ArrayInitIndexExpr
-  //
-  // The resulting expression for a multidimensional array.
-  // ArrayInitLoopExpr                  <-- we're here
-  // |-OpaqueValueExpr
-  // | `-DeclRefExpr                    <-- match this
-  // `-ArrayInitLoopExpr
-  //   |-OpaqueValueExpr
-  //   | `-ArraySubscriptExpr
-  //   |   |-ImplicitCastExpr
-  //   |   | `-OpaqueValueExpr
-  //   |   |   `-DeclRefExpr
-  //   |   `-ArrayInitIndexExpr
-  //   `-CXXConstructExpr             <-- extract this
-  //     ` ...
-
-  const auto *OVESrc = AILE->getCommonExpr()->getSourceExpr();
+                                      const LocationContext *LCtx, NonLoc Idx) {
+  SValBuilder &SVB = State->getStateManager().getSValBuilder();
+  MemRegionManager &MRMgr = SVB.getRegionManager();
+  ASTContext &Ctx = SVB.getContext();
 
   // HACK: There is no way we can put the index of the array element into the
   // CFG unless we unroll the loop, so we manually select and bind the required
   // parameter to the environment.
-  const auto *CE =
+  const Expr *SourceArray = AILE->getCommonExpr()->getSourceExpr();
+  const auto *Ctor =
       cast<CXXConstructExpr>(extractElementInitializerFromNestedAILE(AILE));
 
-  SVal Base = UnknownVal();
-  if (const auto *ME = dyn_cast<MemberExpr>(OVESrc))
-    Base = State->getSVal(ME, LCtx);
-  else if (const auto *DRE = dyn_cast<DeclRefExpr>(OVESrc))
-    Base = State->getLValue(cast<VarDecl>(DRE->getDecl()), LCtx);
-  else
-    llvm_unreachable("ArrayInitLoopExpr contains unexpected source expression");
-
-  SVal NthElem = State->getLValue(CE->getType(), Idx, Base);
+  const SubRegion *SourceArrayRegion =
+      cast<SubRegion>(State->getSVal(SourceArray, LCtx).getAsRegion());
+  const ElementRegion *ElementRegion =
+      MRMgr.getElementRegion(Ctor->getType(), Idx, SourceArrayRegion, Ctx);
 
-  return State->BindExpr(CE->getArg(0), LCtx, NthElem);
+  return State->BindExpr(Ctor->getArg(0), LCtx,
+                         loc::MemRegionVal(ElementRegion));
 }
 
 void ExprEngine::handleConstructor(const Expr *E,
diff --git a/clang/test/Analysis/array-init-loop.cpp b/clang/test/Analysis/array-init-loop.cpp
index 4ab4489fc882f3..64b0aec1daf9d2 100644
--- a/clang/test/Analysis/array-init-loop.cpp
+++ b/clang/test/Analysis/array-init-loop.cpp
@@ -330,3 +330,41 @@ void no_crash() {
 }
 
 } // namespace crash
+
+namespace array_subscript_initializer {
+
+struct S
+{
+  int x;
+};
+
+void no_crash() {
+  S arr[][2] = {{1, 2}};
+
+  const auto [a, b] = arr[0];
+
+  clang_analyzer_eval(a.x == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(b.x == 2); // expected-warning{{TRUE}}
+}
+
+} // namespace array_subscript_initializer
+
+namespace iterator_initializer {
+
+struct S
+{
+  int x;
+};
+
+void no_crash() {
+  S arr[][2] = {{1, 2}, {3, 4}};
+
+  int i = 0;
+  for (const auto [a, b] : arr) {
+    clang_analyzer_eval(a.x == (i == 0 ? 1 : 3)); // expected-warning{{TRUE}}
+    clang_analyzer_eval(b.x == (i == 0 ? 2 : 4)); // expected-warning{{TRUE}}
+    ++i;
+  }
+}
+
+} // namespace iterator_initializer
\ No newline at end of file

``````````

</details>


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


More information about the cfe-commits mailing list