[clang] c81bf94 - [analyzer] Handling non-POD multidimensional arrays in ArrayInitLoopExpr

via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 22 04:54:02 PDT 2022


Author: isuckatcs
Date: 2022-08-22T13:53:53+02:00
New Revision: c81bf940c77ee9439eb1f63011e7ecf0e07c56d9

URL: https://github.com/llvm/llvm-project/commit/c81bf940c77ee9439eb1f63011e7ecf0e07c56d9
DIFF: https://github.com/llvm/llvm-project/commit/c81bf940c77ee9439eb1f63011e7ecf0e07c56d9.diff

LOG: [analyzer] Handling non-POD multidimensional arrays in ArrayInitLoopExpr

This patch makes it possible for lambdas, implicit copy/move ctors
and structured bindings to handle non-POD multidimensional arrays.

Differential Revision: https://reviews.llvm.org/D131840

Added: 
    

Modified: 
    clang/include/clang/AST/ASTContext.h
    clang/include/clang/Analysis/CFG.h
    clang/lib/AST/ASTContext.cpp
    clang/lib/Analysis/CFG.cpp
    clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
    clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
    clang/test/Analysis/array-init-loop.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index a01a6aa4bd7e4..2d533b49114b6 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -2731,6 +2731,10 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// Return number of constant array elements.
   uint64_t getConstantArrayElementCount(const ConstantArrayType *CA) const;
 
+  /// Return number of elements initialized in an ArrayInitLoopExpr.
+  uint64_t
+  getArrayInitLoopExprElementCount(const ArrayInitLoopExpr *AILE) const;
+
   /// Perform adjustment on the parameter type of a function.
   ///
   /// This routine adjusts the given parameter type @p T to the actual

diff  --git a/clang/include/clang/Analysis/CFG.h b/clang/include/clang/Analysis/CFG.h
index 4f16a6361950e..9212276a3f691 100644
--- a/clang/include/clang/Analysis/CFG.h
+++ b/clang/include/clang/Analysis/CFG.h
@@ -1466,6 +1466,8 @@ class CFG {
   llvm::DenseMap<const DeclStmt *, const DeclStmt *> SyntheticDeclStmts;
 };
 
+Expr *extractElementInitializerFromNestedAILE(const ArrayInitLoopExpr *AILE);
+
 } // namespace clang
 
 //===----------------------------------------------------------------------===//

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index a4c6a6951279d..8cc70fdc75d15 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -6905,6 +6905,21 @@ ASTContext::getConstantArrayElementCount(const ConstantArrayType *CA)  const {
   return ElementCount;
 }
 
+uint64_t ASTContext::getArrayInitLoopExprElementCount(
+    const ArrayInitLoopExpr *AILE) const {
+  if (!AILE)
+    return 0;
+
+  uint64_t ElementCount = 1;
+
+  do {
+    ElementCount *= AILE->getArraySize().getZExtValue();
+    AILE = dyn_cast<ArrayInitLoopExpr>(AILE->getSubExpr());
+  } while (AILE);
+
+  return ElementCount;
+}
+
 /// getFloatingRank - Return a relative rank for floating point types.
 /// This routine will assert if passed a built-in type that isn't a float.
 static FloatingRank getFloatingRank(QualType T) {

diff  --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 31655e43e8993..6131256d4a0d0 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -1332,6 +1332,18 @@ class CFGBuilder {
 
 } // namespace
 
+Expr *
+clang::extractElementInitializerFromNestedAILE(const ArrayInitLoopExpr *AILE) {
+  if (!AILE)
+    return nullptr;
+
+  Expr *AILEInit = AILE->getSubExpr();
+  while (const auto *E = dyn_cast<ArrayInitLoopExpr>(AILEInit))
+    AILEInit = E->getSubExpr();
+
+  return AILEInit;
+}
+
 inline bool AddStmtChoice::alwaysAdd(CFGBuilder &builder,
                                      const Stmt *stmt) const {
   return builder.alwaysAdd(stmt) || kind == AlwaysAdd;
@@ -1706,11 +1718,12 @@ CFGBlock *CFGBuilder::addInitializer(CXXCtorInitializer *I) {
   if (Init) {
     // If the initializer is an ArrayInitLoopExpr, we want to extract the
     // initializer, that's used for each element.
-    const auto *AILE = dyn_cast_or_null<ArrayInitLoopExpr>(Init);
+    auto *AILEInit = extractElementInitializerFromNestedAILE(
+        dyn_cast<ArrayInitLoopExpr>(Init));
 
     findConstructionContexts(
         ConstructionContextLayer::create(cfg->getBumpVectorContext(), I),
-        AILE ? AILE->getSubExpr() : Init);
+        AILEInit ? AILEInit : Init);
 
     if (HasTemporaries) {
       // For expression with temporaries go directly to subexpression to omit
@@ -3415,11 +3428,12 @@ CFGBlock *CFGBuilder::VisitLambdaExpr(LambdaExpr *E, AddStmtChoice asc) {
     if (Expr *Init = *it) {
       // If the initializer is an ArrayInitLoopExpr, we want to extract the
       // initializer, that's used for each element.
-      const auto *AILE = dyn_cast_or_null<ArrayInitLoopExpr>(Init);
+      auto *AILEInit = extractElementInitializerFromNestedAILE(
+          dyn_cast<ArrayInitLoopExpr>(Init));
 
       findConstructionContexts(ConstructionContextLayer::create(
                                    cfg->getBumpVectorContext(), {E, Idx}),
-                               AILE ? AILE->getSubExpr() : Init);
+                               AILEInit ? AILEInit : Init);
 
       CFGBlock *Tmp = Visit(Init);
       if (Tmp)

diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 20b13975d054b..f9edcb3189e21 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -537,9 +537,10 @@ ExprEngine::addObjectUnderConstruction(ProgramStateRef State,
     Init = Item.getCXXCtorInitializer()->getInit();
 
   // In an ArrayInitLoopExpr the real initializer is returned by
-  // getSubExpr().
+  // getSubExpr(). Note that AILEs can be nested in case of
+  // multidimesnional arrays.
   if (const auto *AILE = dyn_cast_or_null<ArrayInitLoopExpr>(Init))
-    Init = AILE->getSubExpr();
+    Init = extractElementInitializerFromNestedAILE(AILE);
 
   // FIXME: Currently the state might already contain the marker due to
   // incorrect handling of temporaries bound to default parameters.

diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index c3f7719537789..ced1932a037f3 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -524,16 +524,32 @@ bindRequiredArrayElementToEnvironment(ProgramStateRef State,
   //       |     `-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();
+
   // 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 = cast<CXXConstructExpr>(AILE->getSubExpr());
-  const auto *OVESrc = AILE->getCommonExpr()->getSourceExpr();
+  const auto *CE =
+      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 = cast<DeclRefExpr>(OVESrc))
+  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");
@@ -596,8 +612,9 @@ void ExprEngine::handleConstructor(const Expr *E,
     if (AILE) {
       // Only set this once even though we loop through it multiple times.
       if (!getPendingInitLoop(State, CE, LCtx))
-        State = setPendingInitLoop(State, CE, LCtx,
-                                   AILE->getArraySize().getLimitedValue());
+        State = setPendingInitLoop(
+            State, CE, LCtx,
+            getContext().getArrayInitLoopExprElementCount(AILE));
 
       State = bindRequiredArrayElementToEnvironment(
           State, AILE, LCtx, svalBuilder.makeArrayIndex(Idx));

diff  --git a/clang/test/Analysis/array-init-loop.cpp b/clang/test/Analysis/array-init-loop.cpp
index 0b6198a1dcdc1..ca7c832e8ab13 100644
--- a/clang/test/Analysis/array-init-loop.cpp
+++ b/clang/test/Analysis/array-init-loop.cpp
@@ -223,3 +223,86 @@ void move_ctor_init_non_pod() {
   clang_analyzer_eval(moved.arr[2].i == 5); // expected-warning{{TRUE}}
   clang_analyzer_eval(moved.arr[3].i == 6); // expected-warning{{TRUE}}
 }
+
+//Note: This is the only solution I could find to check the values without 
+// crashing clang. For more details on the crash see Issue #57135.
+void lambda_capture_multi_array() {
+  S3 arr[2][2] = {1,2,3,4};
+
+  {
+    int x = [arr] { return arr[0][0].i; }();
+    clang_analyzer_eval(x == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    int x = [arr] { return arr[0][1].i; }();
+    clang_analyzer_eval(x == 2); // expected-warning{{TRUE}}
+  }
+
+  {
+    int x = [arr] { return arr[1][0].i; }();
+    clang_analyzer_eval(x == 3); // expected-warning{{TRUE}}
+  }
+
+  {
+    int x = [arr] { return arr[1][1].i; }();
+    clang_analyzer_eval(x == 4); // expected-warning{{TRUE}}
+  }
+}
+
+// This struct will force constructor inlining in MultiWrapper.
+struct UserDefinedCtor {
+  int i;
+  UserDefinedCtor() {}
+  UserDefinedCtor(const UserDefinedCtor &copy) {
+    int j = 1;
+    i = copy.i;
+  }
+};
+
+struct MultiWrapper {
+  UserDefinedCtor arr[2][2];
+};
+
+void copy_ctor_multi() {
+  MultiWrapper MW;
+
+  MW.arr[0][0].i = 0;
+  MW.arr[0][1].i = 1;
+  MW.arr[1][0].i = 2;
+  MW.arr[1][1].i = 3;
+
+  MultiWrapper MWCopy = MW;
+  
+  clang_analyzer_eval(MWCopy.arr[0][0].i == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(MWCopy.arr[0][1].i == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(MWCopy.arr[1][0].i == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(MWCopy.arr[1][1].i == 3); // expected-warning{{TRUE}}
+} 
+
+void move_ctor_multi() {
+  MultiWrapper MW;
+
+  MW.arr[0][0].i = 0;
+  MW.arr[0][1].i = 1;
+  MW.arr[1][0].i = 2;
+  MW.arr[1][1].i = 3;
+
+  MultiWrapper MWMove = (MultiWrapper &&) MW;
+  
+  clang_analyzer_eval(MWMove.arr[0][0].i == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(MWMove.arr[0][1].i == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(MWMove.arr[1][0].i == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(MWMove.arr[1][1].i == 3); // expected-warning{{TRUE}}
+} 
+
+void structured_binding_multi() {
+  S3 arr[2][2] = {1,2,3,4};
+
+  auto [a,b] = arr;
+
+  clang_analyzer_eval(a[0].i == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a[1].i == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(b[0].i == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(b[1].i == 4); // expected-warning{{TRUE}}
+}


        


More information about the cfe-commits mailing list