[clang] 360ced3 - [analyzer] Ignore IncompleteArrayTypes in getStaticSize() for FAMs

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2021-08-25T16:12:17+02:00
New Revision: 360ced3b8fd2cfb9f2a26deb739e6c381e98b9a5

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

LOG: [analyzer] Ignore IncompleteArrayTypes in getStaticSize() for FAMs

Currently only `ConstantArrayType` is considered for flexible array
members (FAMs) in `getStaticSize()`.
However, `IncompleteArrayType` also shows up in practice as FAMs.

This patch will ignore the `IncompleteArrayType` and return Unknown
for that case as well. This way it will be at least consistent with
the current behavior until we start modeling them accurately.

I'm expecting that this will resolve a bunch of false-positives
internally, caused by the `ArrayBoundV2`.

Reviewed By: ASDenysPetrov

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

Added: 
    clang/test/Analysis/flexible-array-members.c

Modified: 
    clang/lib/StaticAnalyzer/Core/MemRegion.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index bd725ee9eaa3..1a614d4d2bcd 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -768,14 +768,27 @@ DefinedOrUnknownSVal MemRegionManager::getStaticSize(const MemRegion *MR,
       return UnknownVal();
 
     QualType Ty = cast<TypedValueRegion>(SR)->getDesugaredValueType(Ctx);
-    DefinedOrUnknownSVal Size = getElementExtent(Ty, SVB);
+    const DefinedOrUnknownSVal Size = getElementExtent(Ty, SVB);
 
     // A zero-length array at the end of a struct often stands for dynamically
     // allocated extra memory.
-    if (Size.isZeroConstant()) {
-      if (isa<ConstantArrayType>(Ty))
-        return UnknownVal();
-    }
+    const auto isFlexibleArrayMemberCandidate = [this](QualType Ty) -> bool {
+      const ArrayType *AT = Ctx.getAsArrayType(Ty);
+      if (!AT)
+        return false;
+      if (isa<IncompleteArrayType>(AT))
+        return true;
+
+      if (const auto *CAT = dyn_cast<ConstantArrayType>(AT)) {
+        const llvm::APInt &Size = CAT->getSize();
+        if (Size.isNullValue())
+          return true;
+      }
+      return false;
+    };
+
+    if (isFlexibleArrayMemberCandidate(Ty))
+      return UnknownVal();
 
     return Size;
   }

diff  --git a/clang/test/Analysis/flexible-array-members.c b/clang/test/Analysis/flexible-array-members.c
new file mode 100644
index 000000000000..23a8d1fde0d9
--- /dev/null
+++ b/clang/test/Analysis/flexible-array-members.c
@@ -0,0 +1,96 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c90
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c99
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c11
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c17
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++98 -x c++
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++03 -x c++
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++11 -x c++
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++14 -x c++
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++17 -x c++
+
+typedef __typeof(sizeof(int)) size_t;
+size_t clang_analyzer_getExtent(void *);
+void clang_analyzer_dump(size_t);
+
+void *alloca(size_t size);
+void *malloc(size_t size);
+void free(void *ptr);
+
+void test_incomplete_array_fam() {
+  typedef struct FAM {
+    char c;
+    int data[];
+  } FAM;
+
+  FAM fam;
+  clang_analyzer_dump(clang_analyzer_getExtent(&fam));
+  clang_analyzer_dump(clang_analyzer_getExtent(fam.data));
+  // expected-warning at -2 {{4 S64b}}
+  // expected-warning at -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 at -2 {{4 U64b}}
+  // expected-warning at -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 at -2 {{4 U64b}}
+  // expected-warning at -2 {{Unknown}}
+  free(q);
+}
+
+void test_zero_length_array_fam() {
+  typedef struct FAM {
+    char c;
+    int data[0];
+  } FAM;
+
+  FAM fam;
+  clang_analyzer_dump(clang_analyzer_getExtent(&fam));
+  clang_analyzer_dump(clang_analyzer_getExtent(fam.data));
+  // expected-warning at -2 {{4 S64b}}
+  // expected-warning at -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 at -2 {{4 U64b}}
+  // expected-warning at -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 at -2 {{4 U64b}}
+  // expected-warning at -2 {{Unknown}}
+  free(q);
+}
+
+void test_single_element_array_possible_fam() {
+  typedef struct FAM {
+    char c;
+    int data[1];
+  } FAM;
+
+  FAM likely_fam;
+  clang_analyzer_dump(clang_analyzer_getExtent(&likely_fam));
+  clang_analyzer_dump(clang_analyzer_getExtent(likely_fam.data));
+  // expected-warning at -2 {{8 S64b}}
+  // expected-warning at -2 {{4 S64b}}
+
+  FAM *p = (FAM *)alloca(sizeof(FAM));
+  clang_analyzer_dump(clang_analyzer_getExtent(p));
+  clang_analyzer_dump(clang_analyzer_getExtent(p->data));
+  // expected-warning at -2 {{8 U64b}}
+  // expected-warning at -2 {{4 S64b}}
+
+  FAM *q = (FAM *)malloc(sizeof(FAM));
+  clang_analyzer_dump(clang_analyzer_getExtent(q));
+  clang_analyzer_dump(clang_analyzer_getExtent(q->data));
+  // expected-warning at -2 {{8 U64b}}
+  // expected-warning at -2 {{4 S64b}}
+  free(q);
+}