Skip to content

Commit ffcf214

Browse files
committed
[analyzer] NonParamVarRegion should prefer definition over canonical decl
When we construct a `NonParamVarRegion`, we canonicalize the decl to always use the same entity for consistency. At the moment that is the canonical decl - which is the first decl in the redecl chain. However, this can cause problems with tentative declarations and extern declarations if we declare an array with unknown bounds. Consider this C example: https://godbolt.org/z/Kdvr11EqY ```lang=C typedef typeof(sizeof(int)) size_t; size_t clang_analyzer_getExtent(const void *p); void clang_analyzer_dump(size_t n); extern const unsigned char extern_redecl[]; const unsigned char extern_redecl[] = { 1,2,3,4 }; const unsigned char tentative_redecl[]; const unsigned char tentative_redecl[] = { 1,2,3,4 }; const unsigned char direct_decl[] = { 1,2,3,4 }; void test_redeclaration_extent(void) { clang_analyzer_dump(clang_analyzer_getExtent(direct_decl)); // 4 clang_analyzer_dump(clang_analyzer_getExtent(extern_redecl)); // should be 4 instead of Unknown clang_analyzer_dump(clang_analyzer_getExtent(tentative_redecl)); // should be 4 instead of Unknown } ``` The `getType()` of the canonical decls for the forward declared globals, will return `IncompleteArrayType`, unlike the `getDefinition()->getType()`, which would have returned `ConstantArrayType` of 4 elements. This makes the `MemRegionManager::getStaticSize()` return `Unknown` as the extent for the array variables, leading to FNs. To resolve this, I think we should prefer the definition decl (if present) over the canonical decl when constructing `NonParamVarRegion`s. FYI The canonicalization of the decl was introduced by D57619 in 2019. Differential Revision: https://reviews.llvm.org/D154827
1 parent f79ad31 commit ffcf214

File tree

2 files changed

+22
-1
lines changed

2 files changed

+22
-1
lines changed

clang/lib/StaticAnalyzer/Core/MemRegion.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1077,13 +1077,16 @@ const VarRegion *MemRegionManager::getVarRegion(const VarDecl *D,
10771077
}
10781078
}
10791079

1080-
return getSubRegion<NonParamVarRegion>(D, sReg);
1080+
return getNonParamVarRegion(D, sReg);
10811081
}
10821082

10831083
const NonParamVarRegion *
10841084
MemRegionManager::getNonParamVarRegion(const VarDecl *D,
10851085
const MemRegion *superR) {
1086+
// Prefer the definition over the canonical decl as the canonical form.
10861087
D = D->getCanonicalDecl();
1088+
if (const VarDecl *Def = D->getDefinition())
1089+
D = Def;
10871090
return getSubRegion<NonParamVarRegion>(D, superR);
10881091
}
10891092

clang/test/Analysis/globals.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
2+
3+
typedef typeof(sizeof(int)) size_t;
4+
size_t clang_analyzer_getExtent(const void *p);
5+
void clang_analyzer_dump(size_t n);
6+
7+
extern const unsigned char extern_redecl[];
8+
const unsigned char extern_redecl[] = { 1,2,3,4 };
9+
const unsigned char tentative_redecl[];
10+
const unsigned char tentative_redecl[] = { 1,2,3,4 };
11+
12+
const unsigned char direct_decl[] = { 1,2,3,4 };
13+
14+
void test_redeclaration_extent(void) {
15+
clang_analyzer_dump(clang_analyzer_getExtent(direct_decl)); // expected-warning {{4 S64b}}
16+
clang_analyzer_dump(clang_analyzer_getExtent(extern_redecl)); // expected-warning {{4 S64b}}
17+
clang_analyzer_dump(clang_analyzer_getExtent(tentative_redecl)); // expected-warning {{4 S64b}}
18+
}

0 commit comments

Comments
 (0)