[clang] [clang][analyzer] Relax assertion for non-default address spaces in the cstring checker (PR #153498)

[Isaac Nudelman via cfe-commits]

https://github.com/nuudlman updated https://github.com/llvm/llvm-project/pull/153498

>From 9b7fd4839440ade71e1b9561b158dd37969b7def Mon Sep 17 00:00:00 2001
From: Isaac Nudelman <62861466+nuudlman at users.noreply.github.com>
Date: Wed, 13 Aug 2025 23:02:11 +0200
Subject: [PATCH 1/4] Support non-default address spaces in the cstring checker

---
 .../StaticAnalyzer/Checkers/CStringChecker.cpp  |  6 +++---
 .../Analysis/element-region-address-space.c     | 17 ++++++++++++++++-
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 0e5fc0a074938..b16118e86c0c1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -1129,9 +1129,9 @@ bool CStringChecker::isFirstBufInBound(CheckerContext &C, ProgramStateRef State,
   if (!ER)
     return true; // cf top comment.
 
-  // FIXME: Does this crash when a non-standard definition
-  // of a library function is encountered?
-  assert(ER->getValueType() == C.getASTContext().CharTy &&
+  // Support library functions defined with non-default address spaces
+  assert(ER->getValueType().getCanonicalType().getUnqualifiedType() ==
+             C.getASTContext().CharTy &&
          "isFirstBufInBound should only be called with char* ElementRegions");
 
   // Get the size of the array.
diff --git a/clang/test/Analysis/element-region-address-space.c b/clang/test/Analysis/element-region-address-space.c
index dd7066240fef6..6d657eeedca30 100644
--- a/clang/test/Analysis/element-region-address-space.c
+++ b/clang/test/Analysis/element-region-address-space.c
@@ -1,11 +1,26 @@
 // RUN: %clang_analyze_cc1 -triple amdgcn-unknown-unknown \
-// RUN:   -analyzer-checker=core -verify %s
+// RUN:   -analyzer-checker=core,unix -verify %s
 
 // expected-no-diagnostics
 //
 // By default, pointers are 64-bits.
+#define ADDRESS_SPACE_64BITS __attribute__((address_space(0)))
 #define ADDRESS_SPACE_32BITS __attribute__((address_space(3)))
 
 int test(ADDRESS_SPACE_32BITS int *p, ADDRESS_SPACE_32BITS void *q) {
   return p == q; // no-crash
 }
+
+// Make sure that the cstring checker handles non-default address spaces
+ADDRESS_SPACE_64BITS void *
+memcpy(ADDRESS_SPACE_64BITS void *,
+       ADDRESS_SPACE_32BITS const void *,
+       long unsigned int);
+
+typedef struct {
+  char m[1];
+} k;
+
+void l(ADDRESS_SPACE_32BITS char *p, ADDRESS_SPACE_64BITS k *n) {
+  memcpy(&n->m[0], p, 4);
+}
\ No newline at end of file

>From 22e3c626504dbbbcacfd452371f5593faf82b60f Mon Sep 17 00:00:00 2001
From: Isaac Nudelman <isaac.nudelman at utexas.edu>
Date: Thu, 14 Aug 2025 19:05:02 -0500
Subject: [PATCH 2/4] Clean up test based on feedback

---
 clang/test/Analysis/element-region-address-space.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/clang/test/Analysis/element-region-address-space.c b/clang/test/Analysis/element-region-address-space.c
index 6d657eeedca30..737bacc2ea5a8 100644
--- a/clang/test/Analysis/element-region-address-space.c
+++ b/clang/test/Analysis/element-region-address-space.c
@@ -1,5 +1,6 @@
 // RUN: %clang_analyze_cc1 -triple amdgcn-unknown-unknown \
-// RUN:   -analyzer-checker=core,unix -verify %s
+// RUN:   -Wno-incompatible-library-redeclaration \
+// RUN:   -analyzer-checker=core,unix.cstring.BadSizeArg -verify %s
 
 // expected-no-diagnostics
 //
@@ -17,10 +18,10 @@ memcpy(ADDRESS_SPACE_64BITS void *,
        ADDRESS_SPACE_32BITS const void *,
        long unsigned int);
 
-typedef struct {
-  char m[1];
-} k;
+ADDRESS_SPACE_64BITS struct {
+  char m[16];
+} n;
 
-void l(ADDRESS_SPACE_32BITS char *p, ADDRESS_SPACE_64BITS k *n) {
-  memcpy(&n->m[0], p, 4);
+void avoid_cstring_checker_crash(ADDRESS_SPACE_32BITS char *p) {
+  memcpy(&n.m[0], p, 4); // no-crash
 }
\ No newline at end of file

>From 2363494bdb3b0452019f425215cc1c2a4829e028 Mon Sep 17 00:00:00 2001
From: Isaac Nudelman <isaac.nudelman at utexas.edu>
Date: Sat, 16 Aug 2025 14:23:22 -0500
Subject: [PATCH 3/4] Re-enable all unix checkers to ensure they are compatible
 with non-default address spaces

---
 clang/test/Analysis/element-region-address-space.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/test/Analysis/element-region-address-space.c b/clang/test/Analysis/element-region-address-space.c
index 737bacc2ea5a8..da590556848b9 100644
--- a/clang/test/Analysis/element-region-address-space.c
+++ b/clang/test/Analysis/element-region-address-space.c
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 -triple amdgcn-unknown-unknown \
 // RUN:   -Wno-incompatible-library-redeclaration \
-// RUN:   -analyzer-checker=core,unix.cstring.BadSizeArg -verify %s
+// RUN:   -analyzer-checker=core,unix -verify %s
 
 // expected-no-diagnostics
 //
@@ -24,4 +24,4 @@ ADDRESS_SPACE_64BITS struct {
 
 void avoid_cstring_checker_crash(ADDRESS_SPACE_32BITS char *p) {
   memcpy(&n.m[0], p, 4); // no-crash
-}
\ No newline at end of file
+}

>From 47db358c73193d93ddd1896cb908e8c50d67af3d Mon Sep 17 00:00:00 2001
From: Isaac Nudelman <isaac.nudelman at utexas.edu>
Date: Sat, 16 Aug 2025 14:29:39 -0500
Subject: [PATCH 4/4] Use convenience method for extring the canonical and
 unqualified type

---
 clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index b16118e86c0c1..cfc6d34a75ca2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -1130,7 +1130,7 @@ bool CStringChecker::isFirstBufInBound(CheckerContext &C, ProgramStateRef State,
     return true; // cf top comment.
 
   // Support library functions defined with non-default address spaces
-  assert(ER->getValueType().getCanonicalType().getUnqualifiedType() ==
+  assert(ER->getValueType()->getCanonicalTypeUnqualified() ==
              C.getASTContext().CharTy &&
          "isFirstBufInBound should only be called with char* ElementRegions");