[analyzer] Retain address space information in getElementRegion

[NagyDonat]

The factory method MemRegionManager::getElementRegion() is the main way of constructing ElementRegion objects, which are widespread in the analysis and may represent array elements (as lvalues), pointer arithmetics and type conversions.

This factory method used to strip all qualifiers from the type associated with the element (after canonicalizing it), but the address space qualifier can affect the size of a pointer type, so stripping it caused an assertion failure (in evalBinOpLL):

clang: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:785: void assertEqualBitWidths(clang::ento::ProgramStateRef, clang::ento::Loc, clang::ento::Loc): Assertion `RhsBitwidth == LhsBitwidth && "RhsLoc and LhsLoc bitwidth must be same!"' failed.

[llvmbot]

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Donát Nagy (NagyDonat)

Changes

The factory method MemRegionManager::getElementRegion() is the main way of constructing ElementRegion objects, which are widespread in the analysis and may represent array elements (as lvalues), pointer arithmetics and type conversions.

This factory method used to strip all qualifiers from the type associated with the element (after canonicalizing it), but the address space qualifier can affect the size of a pointer type, so stripping it caused an assertion failure (in evalBinOpLL):

clang: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:785: void assertEqualBitWidths(clang::ento::ProgramStateRef, clang::ento::Loc, clang::ento::Loc): Assertion `RhsBitwidth == LhsBitwidth && "RhsLoc and LhsLoc bitwidth must be same!"' failed.

---

Full diff: https://github.com/llvm/llvm-project/pull/151370.diff

2 Files Affected:

• (modified) clang/lib/StaticAnalyzer/Core/MemRegion.cpp (+10)
• (added) clang/test/Analysis/element-region-address-space.c (+11)

diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index 3e68373028b10..0058a0ddfb0df 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1219,6 +1219,16 @@ MemRegionManager::getElementRegion(QualType elementType, NonLoc Idx,
                                    const ASTContext &Ctx) {
   QualType T = Ctx.getCanonicalType(elementType).getUnqualifiedType();
 
+  // The address space must be preserved because some target-specific address
+  // spaces influence the size of the pointer value which is represented by the
+  // element region.
+  LangAS AS = elementType.getAddressSpace();
+  if (AS != LangAS::Default) {
+    Qualifiers Quals;
+    Quals.setAddressSpace(AS);
+    T = Ctx.getQualifiedType(T, Quals);
+  }
+
   llvm::FoldingSetNodeID ID;
   ElementRegion::ProfileRegion(ID, T, Idx, superRegion);
 
diff --git a/clang/test/Analysis/element-region-address-space.c b/clang/test/Analysis/element-region-address-space.c
new file mode 100644
index 0000000000000..fddc1bb840231
--- /dev/null
+++ b/clang/test/Analysis/element-region-address-space.c
@@ -0,0 +1,11 @@
+// RUN: %clang_analyze_cc1 -triple amdgcn-unknown-unknown \
+// RUN: -analyzer-checker=core,unix.Malloc -verify %s
+
+// expected-no-diagnostics
+//
+// By default, pointers are 64-bits.
+#define ADDRESS_SPACE_32BITS __attribute__((address_space(3)))
+
+int test(ADDRESS_SPACE_32BITS int *p, ADDRESS_SPACE_32BITS void *q) {
+  return p == q;
+}

[vabridgers]

LGTM, thanks for coming up with a good solution for this problem.

[vabridgers]

LGTM, thanks for PR!

[steakhal]

I find this solution a lot more robust.
I'll think about this before casting my vote.

[steakhal]

LGTM modulo suggestions.
Thanks both of you.