From 360ced3b8fd2cfb9f2a26deb739e6c381e98b9a5 Mon Sep 17 00:00:00 2001 From: Balazs Benics Date: Wed, 25 Aug 2021 16:12:17 +0200 Subject: [PATCH] [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 --- clang/lib/StaticAnalyzer/Core/MemRegion.cpp | 23 +++++-- clang/test/Analysis/flexible-array-members.c | 96 ++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 5 deletions(-) create mode 100644 clang/test/Analysis/flexible-array-members.c diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp index bd725ee..1a614d4 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(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(Ty)) - return UnknownVal(); - } + const auto isFlexibleArrayMemberCandidate = [this](QualType Ty) -> bool { + const ArrayType *AT = Ctx.getAsArrayType(Ty); + if (!AT) + return false; + if (isa(AT)) + return true; + + if (const auto *CAT = dyn_cast(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 0000000..23a8d1f --- /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@-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); +} -- 2.7.4