[analyzer] Ignore IncompleteArrayTypes in getStaticSize() for FAMs
authorBalazs Benics <balazs.benics@sigmatechnology.se>
Wed, 25 Aug 2021 14:12:17 +0000 (16:12 +0200)
committerBalazs Benics <balazs.benics@sigmatechnology.se>
Wed, 25 Aug 2021 14:12:17 +0000 (16:12 +0200)
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

clang/lib/StaticAnalyzer/Core/MemRegion.cpp
clang/test/Analysis/flexible-array-members.c [new file with mode: 0644]

index bd725ee..1a614d4 100644 (file)
@@ -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 (file)
index 0000000..23a8d1f
--- /dev/null
@@ -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@-2 {{4 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 {{4 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 {{4 U64b}}
+  // expected-warning@-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@-2 {{4 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 {{4 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 {{4 U64b}}
+  // expected-warning@-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@-2 {{8 S64b}}
+  // expected-warning@-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@-2 {{8 U64b}}
+  // expected-warning@-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@-2 {{8 U64b}}
+  // expected-warning@-2 {{4 S64b}}
+  free(q);
+}