Author: Balazs Benics Date: 2023-07-11T08:50:59+02:00 New Revision: ffcf214b5d27453119575d4e075cac483d659024
URL: https://github.com/llvm/llvm-project/commit/ffcf214b5d27453119575d4e075cac483d659024 DIFF: https://github.com/llvm/llvm-project/commit/ffcf214b5d27453119575d4e075cac483d659024.diff LOG: [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 Added: clang/test/Analysis/globals.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 f89e71a4f157b7..16db6b249dc92b 100644 --- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp +++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp @@ -1077,13 +1077,16 @@ const VarRegion *MemRegionManager::getVarRegion(const VarDecl *D, } } - 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); } diff --git a/clang/test/Analysis/globals.c b/clang/test/Analysis/globals.c new file mode 100644 index 00000000000000..a0dae9d64b3c2e --- /dev/null +++ b/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}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits