[clang] ffcf214 - [analyzer] NonParamVarRegion should prefer definition over canonical decl

[Balazs Benics via cfe-commits]

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}}
+}