steakhal created this revision.
steakhal added reviewers: xazax.hun, donat.nagy, NoQ.
Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, Szelethus,
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware.
Herald added a reviewer: Szelethus.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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>:
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
<https://reviews.llvm.org/D57619> in 2019.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D154827
Files:
clang/lib/StaticAnalyzer/Core/MemRegion.cpp
clang/test/Analysis/globals.c
Index: clang/test/Analysis/globals.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/globals.c
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify
%s
+
+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)); //
expected-warning {{4 S64b}}
+ clang_analyzer_dump(clang_analyzer_getExtent(extern_redecl)); //
expected-warning {{4 S64b}}
+ clang_analyzer_dump(clang_analyzer_getExtent(tentative_redecl)); //
expected-warning {{4 S64b}}
+}
Index: clang/lib/StaticAnalyzer/Core/MemRegion.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1077,13 +1077,16 @@
}
}
- return getSubRegion<NonParamVarRegion>(D, sReg);
+ return getNonParamVarRegion(D, sReg);
}
const NonParamVarRegion *
MemRegionManager::getNonParamVarRegion(const VarDecl *D,
const MemRegion *superR) {
+ // Prefer the definition over the canonical decl as the canonical form.
D = D->getCanonicalDecl();
+ if (const VarDecl *Def = D->getDefinition())
+ D = Def;
return getSubRegion<NonParamVarRegion>(D, superR);
}
Index: clang/test/Analysis/globals.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/globals.c
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+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)); // expected-warning {{4 S64b}}
+ clang_analyzer_dump(clang_analyzer_getExtent(extern_redecl)); // expected-warning {{4 S64b}}
+ clang_analyzer_dump(clang_analyzer_getExtent(tentative_redecl)); // expected-warning {{4 S64b}}
+}
Index: clang/lib/StaticAnalyzer/Core/MemRegion.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1077,13 +1077,16 @@
}
}
- return getSubRegion<NonParamVarRegion>(D, sReg);
+ return getNonParamVarRegion(D, sReg);
}
const NonParamVarRegion *
MemRegionManager::getNonParamVarRegion(const VarDecl *D,
const MemRegion *superR) {
+ // Prefer the definition over the canonical decl as the canonical form.
D = D->getCanonicalDecl();
+ if (const VarDecl *Def = D->getDefinition())
+ D = Def;
return getSubRegion<NonParamVarRegion>(D, superR);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
