[analyzer] Fix for incorrect handling of 0 length non-POD array construction

Prior to this patch when the analyzer encountered a non-POD 0 length array,
it still invoked the constructor for 1 element, which lead to false positives.
This patch makes sure that we no longer construct any elements when we see a
0 length array.

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

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

@@ -606,6 +606,27 @@ void ExprEngine::handleConstructor(const Expr *E,
 
     unsigned Idx = 0;
     if (CE->getType()->isArrayType() || AILE) {
+
+      auto isZeroSizeArray = [&] {
+        uint64_t Size = 1;
+
+        if (const auto *CAT = dyn_cast<ConstantArrayType>(CE->getType()))
+          Size = getContext().getConstantArrayElementCount(CAT);
+        else if (AILE)
+          Size = getContext().getArrayInitLoopExprElementCount(AILE);
+
+        return Size == 0;
+      };
+
+      // No element construction will happen in a 0 size array.
+      if (isZeroSizeArray()) {
+        StmtNodeBuilder Bldr(Pred, destNodes, *currBldrCtx);
+        static SimpleProgramPointTag T{"ExprEngine",
+                                       "Skipping 0 size array construction"};
+        Bldr.generateNode(CE, Pred, State, &T);
+        return;
+      }
+
       Idx = getIndexOfElementToConstruct(State, CE, LCtx).value_or(0u);
       State = setIndexOfElementToConstruct(State, CE, LCtx, Idx + 1);
     }
@@ -1165,6 +1186,16 @@ void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE, ExplodedNode *Pred,
 
       assert(InitExpr && "Capture missing initialization expression");
 
+      // Capturing a 0 length array is a no-op, so we ignore it to get a more
+      // accurate analysis. If it's not ignored, it would set the default
+      // binding of the lambda to 'Unknown', which can lead to falsely detecting
+      // 'Uninitialized' values as 'Unknown' and not reporting a warning.
+      const auto FTy = FieldForCapture->getType();
+      if (FTy->isConstantArrayType() &&
+          getContext().getConstantArrayElementCount(
+              getContext().getAsConstantArrayType(FTy)) == 0)
+        continue;
+
       // With C++17 copy elision the InitExpr can be anything, so instead of
       // pattern matching all cases, we simple check if the current field is
       // under construction or not, regardless what it's InitExpr is.

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

@@ -2594,6 +2594,12 @@ RegionStoreManager::tryBindSmallStruct(RegionBindingsConstRef B,
       return None;
 
     QualType Ty = FD->getType();
+
+    // Zero length arrays are basically no-ops, so we also ignore them here.
+    if (Ty->isConstantArrayType() &&
+        Ctx.getConstantArrayElementCount(Ctx.getAsConstantArrayType(Ty)) == 0)
+      continue;
+
     if (!(Ty->isScalarType() || Ty->isReferenceType()))
       return None;
 

clang/test/Analysis/array-init-loop.cpp

@@ -306,3 +306,27 @@ void structured_binding_multi() {
   clang_analyzer_eval(b[0].i == 3); // expected-warning{{TRUE}}
   clang_analyzer_eval(b[1].i == 4); // expected-warning{{TRUE}}
 }
+
+// This snippet used to crash
+namespace crash {
+
+struct S
+{
+  int x;
+  S() { x = 1; }
+};
+
+void no_crash() {
+  S arr[0];
+  int n = 1;
+
+  auto l = [arr, n] { return n; };
+
+  int x = l();
+  clang_analyzer_eval(x == 1); // expected-warning{{TRUE}}
+
+  // FIXME: This should be 'Undefined'.
+  clang_analyzer_eval(arr[0].x); // expected-warning{{UNKNOWN}}
+}
+
+} // namespace crash

clang/test/Analysis/dtor-array.cpp

@@ -258,8 +258,7 @@ void zeroLength(){
   auto *arr4 = new InlineDtor[2][2][0];
   delete[] arr4;
 
-  // FIXME: Should be TRUE once the constructors are handled properly.
-  clang_analyzer_eval(InlineDtor::dtorCalled == 0); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+  clang_analyzer_eval(InlineDtor::dtorCalled == 0); // expected-warning {{TRUE}}
 }
 
 

clang/test/Analysis/flexible-array-member.cpp

@@ -0,0 +1,58 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++17 -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+void clang_analyzer_eval(bool);
+
+struct S
+{
+    static int c;
+    static int d;
+    int x;
+    S() { x = c++; }
+    ~S() { d++; }
+};
+
+int S::c = 0;
+int S::d = 0;
+
+struct Flex
+{
+    int length;
+    S contents[0];
+};
+
+void flexibleArrayMember()
+{
+    S::c = 0;
+    S::d = 0;
+
+    const int size = 4;
+
+    Flex *arr =
+        (Flex *)::operator new(__builtin_offsetof(Flex, contents) + sizeof(S) * size);
+
+    clang_analyzer_eval(S::c == 0); // expected-warning{{TRUE}}
+
+    new (&arr->contents[0]) S;
+    new (&arr->contents[1]) S;
+    new (&arr->contents[2]) S;
+    new (&arr->contents[3]) S;
+
+    clang_analyzer_eval(S::c == size); // expected-warning{{TRUE}}
+
+    clang_analyzer_eval(arr->contents[0].x == 0); // expected-warning{{TRUE}}
+    clang_analyzer_eval(arr->contents[1].x == 1); // expected-warning{{TRUE}}
+    clang_analyzer_eval(arr->contents[2].x == 2); // expected-warning{{TRUE}}
+    clang_analyzer_eval(arr->contents[3].x == 3); // expected-warning{{TRUE}}
+
+    arr->contents[0].~S();
+    arr->contents[1].~S();
+    arr->contents[2].~S();
+    arr->contents[3].~S();
+
+    ::operator delete(arr);
+
+    clang_analyzer_eval(S::d == size); // expected-warning{{TRUE}}
+}

clang/test/Analysis/zero-size-non-pod-array.cpp

@@ -0,0 +1,189 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++17 -verify %s
+
+void clang_analyzer_eval(bool);
+
+struct S{
+    static int CtorInvocationCount;
+    static int DtorInvocationCount;
+
+    S(){CtorInvocationCount++;}
+    ~S(){DtorInvocationCount++;}
+};
+
+int S::CtorInvocationCount = 0;
+int S::DtorInvocationCount = 0;
+
+void zeroSizeArrayStack() {
+    S::CtorInvocationCount = 0;
+
+    S arr[0];
+
+    clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}}
+}
+
+void zeroSizeMultidimensionalArrayStack() {
+    S::CtorInvocationCount = 0;
+    S::DtorInvocationCount = 0;
+
+    {
+        S arr[2][0];
+        S arr2[0][2];
+
+        S arr3[0][2][2];
+        S arr4[2][2][0];
+        S arr5[2][0][2];
+    }
+
+    clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}}
+    clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}}
+}
+
+void zeroSizeArrayStackInLambda() {
+    S::CtorInvocationCount = 0;
+    S::DtorInvocationCount = 0;
+
+    []{
+        S arr[0];
+    }();    
+    
+    clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}}
+    clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}}
+}
+
+void zeroSizeArrayHeap() {
+    S::CtorInvocationCount = 0;
+    S::DtorInvocationCount = 0;
+
+    auto *arr = new S[0];
+    delete[] arr;
+    
+    clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}}
+    clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}}
+}
+
+void zeroSizeMultidimensionalArrayHeap() {
+    S::CtorInvocationCount = 0;
+    S::DtorInvocationCount = 0;
+
+    auto *arr = new S[2][0];
+    delete[] arr;
+        
+    auto *arr2 = new S[0][2];
+    delete[] arr2;
+
+    auto *arr3 = new S[0][2][2];
+    delete[] arr3;
+
+    auto *arr4 = new S[2][2][0];
+    delete[] arr4;
+
+    auto *arr5 = new S[2][0][2];
+    delete[] arr5;
+
+    clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}}
+    clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}}
+}
+
+#if __cplusplus >= 201703L
+
+void zeroSizeArrayBinding() {
+    S::CtorInvocationCount = 0;
+
+    S arr[0];
+
+    // Note: This is an error in gcc but a warning in clang.
+    // In MSVC the declaration of 'S arr[0]' is already an error
+    // and it doesn't recognize this syntax as a structured binding.
+    auto [] = arr; //expected-warning{{ISO C++17 does not allow a decomposition group to be empty}}
+
+    clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}}
+}
+
+#endif
+
+void zeroSizeArrayLambdaCapture() {
+    S::CtorInvocationCount = 0;
+    S::DtorInvocationCount = 0;
+        
+    S arr[0];
+
+    auto l = [arr]{};
+    [arr]{}();    
+    
+    //FIXME: These should be TRUE. We should avoid calling the destructor 
+    // of the temporary that is materialized as the lambda.
+    clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}} expected-warning{{FALSE}}
+    clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}} expected-warning{{FALSE}}
+}
+
+// FIXME: Report a warning if the standard is at least C++17.
+#if __cplusplus < 201703L
+void zeroSizeArrayLambdaCaptureUndefined1() {
+    S arr[0];
+    int n;
+
+    auto l = [arr, n]{
+        int x = n; //expected-warning{{Assigned value is garbage or undefined}}
+        (void) x;
+    };
+
+    l();
+}
+#endif
+
+void zeroSizeArrayLambdaCaptureUndefined2() {
+    S arr[0];
+    int n;
+
+    [arr, n]{
+        int x = n; //expected-warning{{Assigned value is garbage or undefined}}
+        (void) x;
+    }();
+}
+
+struct Wrapper{
+    S arr[0];
+};
+
+void zeroSizeArrayMember() {
+    S::CtorInvocationCount = 0;
+    S::DtorInvocationCount = 0;
+
+    {
+        Wrapper W;
+    }
+
+    clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}}
+    clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}}
+}
+
+void zeroSizeArrayMemberCopyMove() {
+    S::CtorInvocationCount = 0;
+    S::DtorInvocationCount = 0;
+    
+    {
+        Wrapper W;
+        Wrapper W2 = W;
+        Wrapper W3 = (Wrapper&&) W2;
+    }
+
+    clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}}
+    clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}}
+}
+
+struct MultiWrapper{
+    S arr[2][0];
+};
+
+void zeroSizeMultidimensionalArrayMember() {
+    S::CtorInvocationCount = 0;
+    S::DtorInvocationCount = 0;
+
+    {
+        MultiWrapper MW;
+    }
+
+    clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}}
+    clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}}
+}