[analyzer] Ignore single element arrays in getStaticSize() conditionally

Quoting https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html:
> In the absence of the zero-length array extension, in ISO C90 the contents
> array in the example above would typically be declared to have a single
> element.

We should not assume that the size of the //flexible array member// field has
a single element, because in some cases they use it as a fallback for not
having the //zero-length array// language extension.
In this case, the analyzer should return `Unknown` as the extent of the field
instead.

Reviewed By: martong

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

diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
index e97e0a6..16ca7a8 100644 (file)
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -320,6 +320,14 @@ ANALYZER_OPTION(bool, ShouldDisplayCheckerNameForText, "display-checker-name",
                 "Display the checker name for textual outputs",
                 true)
 
+ANALYZER_OPTION(
+    bool, ShouldConsiderSingleElementArraysAsFlexibleArrayMembers,
+    "consider-single-element-arrays-as-flexible-array-members",
+    "Consider single element arrays as flexible array member candidates. "
+    "This will prevent the analyzer from assuming that a single element array "
+    "holds a single element.",
+    false)
+
 //===----------------------------------------------------------------------===//
 // Unsigned analyzer options.
 //===----------------------------------------------------------------------===//

diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index 1a614d4..0358430 100644 (file)
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -28,7 +28,9 @@
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
@@ -770,9 +772,16 @@ DefinedOrUnknownSVal MemRegionManager::getStaticSize(const MemRegion *MR,
     QualType Ty = cast<TypedValueRegion>(SR)->getDesugaredValueType(Ctx);
     const DefinedOrUnknownSVal Size = getElementExtent(Ty, SVB);
 
-    // A zero-length array at the end of a struct often stands for dynamically
-    // allocated extra memory.
-    const auto isFlexibleArrayMemberCandidate = [this](QualType Ty) -> bool {
+    // We currently don't model flexible array members (FAMs), which are:
+    //  - int array[]; of IncompleteArrayType
+    //  - int array[0]; of ConstantArrayType with size 0
+    //  - int array[1]; of ConstantArrayType with size 1 (*)
+    // (*): Consider single element array object members as FAM candidates only
+    //      if the consider-single-element-arrays-as-flexible-array-members
+    //      analyzer option is true.
+    // https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
+    const auto isFlexibleArrayMemberCandidate = [this,
+                                                 &SVB](QualType Ty) -> bool {
       const ArrayType *AT = Ctx.getAsArrayType(Ty);
       if (!AT)
         return false;
@@ -783,6 +792,15 @@ DefinedOrUnknownSVal MemRegionManager::getStaticSize(const MemRegion *MR,
         const llvm::APInt &Size = CAT->getSize();
         if (Size.isNullValue())
           return true;
+
+        // FIXME: Acquire the AnalyzerOptions in a simpler way.
+        const AnalyzerOptions &Opts = SVB.getStateManager()
+                                          .getOwningEngine()
+                                          .getAnalysisManager()
+                                          .getAnalyzerOptions();
+        if (Opts.ShouldConsiderSingleElementArraysAsFlexibleArrayMembers &&
+            Size.isOneValue())
+          return true;
       }
       return false;
     };

diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c
index c9f9b43..bf3335c 100644 (file)
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -31,6 +31,7 @@
 // CHECK-NEXT: cfg-rich-constructors = true
 // CHECK-NEXT: cfg-scopes = false
 // CHECK-NEXT: cfg-temporary-dtors = true
+// CHECK-NEXT: consider-single-element-arrays-as-flexible-array-members = false
 // CHECK-NEXT: core.CallAndMessage:ArgInitializedness = true
 // CHECK-NEXT: core.CallAndMessage:ArgPointeeInitializedness = false
 // CHECK-NEXT: core.CallAndMessage:CXXDeallocationArg = true

diff --git a/clang/test/Analysis/flexible-array-members.c b/clang/test/Analysis/flexible-array-members.c
index 6200ed6..2c11910 100644 (file)
--- a/clang/test/Analysis/flexible-array-members.c
+++ b/clang/test/Analysis/flexible-array-members.c
@@ -9,6 +9,11 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++14 -x c++
 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++17 -x c++
 
+// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c17 \
+// RUN:    -analyzer-config consider-single-element-arrays-as-flexible-array-members=true -DSINGLE_ELEMENT_FAMS
+// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++17 -x c++ \
+// RUN:    -analyzer-config consider-single-element-arrays-as-flexible-array-members=true -DSINGLE_ELEMENT_FAMS
+
 typedef __typeof(sizeof(int)) size_t;
 size_t clang_analyzer_getExtent(void *);
 void clang_analyzer_dump(size_t);
@@ -75,6 +80,26 @@ void test_single_element_array_possible_fam() {
     int data[1];
   } FAM;
 
+#ifdef SINGLE_ELEMENT_FAMS
+  FAM likely_fam;
+  clang_analyzer_dump(clang_analyzer_getExtent(&likely_fam));
+  clang_analyzer_dump(clang_analyzer_getExtent(likely_fam.data));
+  // expected-warning@-2 {{8 S64b}}
+  // expected-warning@-2 {{Unknown}}
+
+  FAM *p = (FAM *)alloca(sizeof(FAM));
+  clang_analyzer_dump(clang_analyzer_getExtent(p));
+  clang_analyzer_dump(clang_analyzer_getExtent(p->data));
+  // expected-warning@-2 {{8 U64b}}
+  // expected-warning@-2 {{Unknown}}
+
+  FAM *q = (FAM *)malloc(sizeof(FAM));
+  clang_analyzer_dump(clang_analyzer_getExtent(q));
+  clang_analyzer_dump(clang_analyzer_getExtent(q->data));
+  // expected-warning@-2 {{8 U64b}}
+  // expected-warning@-2 {{Unknown}}
+  free(q);
+#else
   FAM likely_fam;
   clang_analyzer_dump(clang_analyzer_getExtent(&likely_fam));
   clang_analyzer_dump(clang_analyzer_getExtent(likely_fam.data));
@@ -93,4 +118,5 @@ void test_single_element_array_possible_fam() {
   // expected-warning@-2 {{8 U64b}}
   // expected-warning@-2 {{4 S64b}}
   free(q);
+#endif
 }