https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/155855
From a206f1e24596554f2d7221b795c1196e01d74a5b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alva...@sonarsource.com>
Date: Mon, 25 Aug 2025 17:02:59 +0200
Subject: [PATCH 1/4] CPP-6868 Fix FP in `PointerArith` for arrays created with
placement-new
Flag the region as if it was `reinterpret_cast`-ed
---
.../Checkers/PointerArithChecker.cpp | 32 ++++-
clang/test/Analysis/ptr-arith.cpp | 121 ++++++++++++++++++
2 files changed, 152 insertions(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp
index 30e01e73eb18f..c4a968950e8bc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp
@@ -74,6 +74,26 @@ class PointerArithChecker
REGISTER_MAP_WITH_PROGRAMSTATE(RegionState, const MemRegion *, AllocKind)
+namespace {
+
+bool isArrayPlacementNew(const CXXNewExpr *NE) {
+ return NE->isArray() && NE->getNumPlacementArgs() > 0;
+}
+
+ProgramStateRef markSuperRegionReinterpreted(ProgramStateRef State,
+ const MemRegion *Region) {
+ while (const auto *BaseRegion = dyn_cast<CXXBaseObjectRegion>(Region)) {
+ Region = BaseRegion->getSuperRegion();
+ }
+ if (const auto *ElemRegion = dyn_cast<ElementRegion>(Region)) {
+ State = State->set<RegionState>(ElemRegion->getSuperRegion(),
+ AllocKind::Reinterpreted);
+ }
+ return State;
+}
+
+} // namespace
+
void PointerArithChecker::checkDeadSymbols(SymbolReaper &SR,
CheckerContext &C) const {
// TODO: intentional leak. Some information is garbage collected too early,
@@ -244,13 +264,23 @@ void PointerArithChecker::checkPostStmt(const CXXNewExpr
*NE,
const MemRegion *Region = AllocedVal.getAsRegion();
if (!Region)
return;
+
+ // For array placement-new, mark the original region as reinterpreted
+ if (isArrayPlacementNew(NE)) {
+ State = markSuperRegionReinterpreted(State, Region);
+ }
+
State = State->set<RegionState>(Region, Kind);
C.addTransition(State);
}
void PointerArithChecker::checkPostStmt(const CastExpr *CE,
CheckerContext &C) const {
- if (CE->getCastKind() != CastKind::CK_BitCast)
+ // Casts to `void*` happen, for instance, on placement new calls.
+ // We consider `void*` not to erase the type information about the underlying
+ // region.
+ if (CE->getCastKind() != CastKind::CK_BitCast ||
+ CE->getType()->isVoidPointerType())
return;
const Expr *CastedExpr = CE->getSubExpr();
diff --git a/clang/test/Analysis/ptr-arith.cpp
b/clang/test/Analysis/ptr-arith.cpp
index ec1c75c0c4063..39bb239f51ca7 100644
--- a/clang/test/Analysis/ptr-arith.cpp
+++ b/clang/test/Analysis/ptr-arith.cpp
@@ -165,3 +165,124 @@ void LValueToRValueBitCast_dumps(void *p, char
(*array)[8]) {
unsigned long ptr_arithmetic(void *p) {
return __builtin_bit_cast(unsigned long, p) + 1; // no-crash
}
+
+
+void escape(int*);
+
+struct AllocOpaqueFlag {};
+
+void* operator new(unsigned long, void *ptr) noexcept { return ptr; }
+void* operator new(unsigned long, void *ptr, AllocOpaqueFlag const&) noexcept
{ return ptr; }
+
+void* operator new[](unsigned long, void* ptr) noexcept { return ptr; }
+void* operator new[](unsigned long, void* ptr, AllocOpaqueFlag const&);
+
+struct Buffer {
+ char buf[100];
+ int padding;
+};
+
+void checkPlacementNewArryInObject() {
+ Buffer buffer;
+ int* array = new (&buffer) int[10];
+ escape(array);
+ ++array; // no warning
+ (void)*array;
+}
+
+void checkPlacementNewArrayInObjectOpaque() {
+ Buffer buffer;
+ int* array = new (&buffer, AllocOpaqueFlag{}) int[10];
+ escape(array);
+ ++array; // no warning
+ (void)*array;
+}
+
+void checkPlacementNewArrayInArray() {
+ char buffer[100];
+ int* array = new (buffer) int[10];
+ escape(array);
+ ++array; // no warning
+ (void)*array;
+}
+
+void checkPlacementNewArrayInArrayOpaque() {
+ char buffer[100];
+ int* array = new (buffer, AllocOpaqueFlag{}) int;
+ escape(array);
+ ++array; // no warning
+ (void)*array;
+}
+
+void checkPlacementNewObjectInObject() {
+ Buffer buffer;
+ int* array = new (&buffer) int;
+ escape(array);
+ ++array; // expected-warning{{Pointer arithmetic on non-array variables
relies on memory layout, which is dangerous}}
+ (void)*array;
+}
+
+void checkPlacementNewObjectInObjectOpaque() {
+ Buffer buffer;
+ int* array = new (&buffer, AllocOpaqueFlag{}) int;
+ escape(array);
+ ++array; // expected-warning{{Pointer arithmetic on non-array variables
relies on memory layout, which is dangerous}}
+ (void)*array;
+}
+
+void checkPlacementNewObjectInArray() {
+ char buffer[sizeof(int)];
+ int* array = new (buffer) int;
+ escape(array);
+ ++array; // no warning (FN)
+ (void)*array;
+}
+
+void checkPlacementNewObjectInArrayOpaque() {
+ char buffer[sizeof(int)];
+ int* array = new (buffer, AllocOpaqueFlag{}) int;
+ escape(array);
+ ++array; // no warning (FN)
+ (void)*array;
+}
+
+void checkPlacementNewSlices() {
+ const int N = 10;
+ char buffer[sizeof(int) * N] = {0};
+ int *start = new (buffer) int{0};
+ for (int i = 1; i < N; i++) {
+ auto *ptr = new int(buffer[i * sizeof(int)]);
+ *ptr = i;
+ }
+ ++start; // no warning
+ (void*)start;
+}
+
+class BumpAlloc {
+ char *buffer;
+ char *offset;
+public:
+ BumpAlloc(int n): buffer(new char[n]), offset{buffer} {}
+ ~BumpAlloc() {delete [] buffer;}
+
+ void* alloc(unsigned long size) {
+ auto* ptr = offset;
+ offset += size;
+ return ptr;
+ }
+};
+
+void* operator new(unsigned long size, BumpAlloc &ba) {
+ return ba.alloc(size);
+}
+
+void checkPlacementSlab() {
+ BumpAlloc bump{10};
+
+ int *ptr = new (bump) int{0};
+ ++ptr; // no warning
+ (void)*ptr;
+
+ BumpAlloc *why = ≎
+ ++why; // expected-warning {{Pointer arithmetic on non-array variables
relies on memory layout, which is dangerous [alpha.core.PointerArithm]}}
+}
From f81b5e9bf9694b3a81166011f8731168fd6e2532 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alva...@sonarsource.com>
Date: Thu, 28 Aug 2025 16:50:40 +0200
Subject: [PATCH 2/4] Add new positive (which I believe is a TP)
---
clang/test/Analysis/PR24184.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/test/Analysis/PR24184.cpp b/clang/test/Analysis/PR24184.cpp
index 172d3e7e33864..c73271b87de98 100644
--- a/clang/test/Analysis/PR24184.cpp
+++ b/clang/test/Analysis/PR24184.cpp
@@ -56,7 +56,7 @@ void fn1_1(void *p1, const void *p2) { p1 != p2; }
void fn2_1(uint32_t *p1, unsigned char *p2, uint32_t p3) {
unsigned i = 0;
for (0; i < p3; i++)
- fn1_1(p1 + i, p2 + i * 0);
+ fn1_1(p1 + i, p2 + i * 0); // expected-warning {{Pointer arithmetic on
non-array variables relies on memory layout, which is dangerous}}
}
struct A_1 {
From 4b13c0af7d9addc130ab32963f3f575062f9e948 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alva...@sonarsource.com>
Date: Fri, 29 Aug 2025 08:14:41 +0200
Subject: [PATCH 3/4] Feedback
---
.../Checkers/PointerArithChecker.cpp | 10 +---
clang/test/Analysis/ptr-arith.cpp | 57 ++++++++-----------
2 files changed, 28 insertions(+), 39 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp
index c4a968950e8bc..f2fc9219912ad 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp
@@ -74,14 +74,12 @@ class PointerArithChecker
REGISTER_MAP_WITH_PROGRAMSTATE(RegionState, const MemRegion *, AllocKind)
-namespace {
-
-bool isArrayPlacementNew(const CXXNewExpr *NE) {
+static bool isArrayPlacementNew(const CXXNewExpr *NE) {
return NE->isArray() && NE->getNumPlacementArgs() > 0;
}
-ProgramStateRef markSuperRegionReinterpreted(ProgramStateRef State,
- const MemRegion *Region) {
+static ProgramStateRef markSuperRegionReinterpreted(ProgramStateRef State,
+ const MemRegion *Region) {
while (const auto *BaseRegion = dyn_cast<CXXBaseObjectRegion>(Region)) {
Region = BaseRegion->getSuperRegion();
}
@@ -92,8 +90,6 @@ ProgramStateRef markSuperRegionReinterpreted(ProgramStateRef
State,
return State;
}
-} // namespace
-
void PointerArithChecker::checkDeadSymbols(SymbolReaper &SR,
CheckerContext &C) const {
// TODO: intentional leak. Some information is garbage collected too early,
diff --git a/clang/test/Analysis/ptr-arith.cpp
b/clang/test/Analysis/ptr-arith.cpp
index 39bb239f51ca7..9f1bc6b2f24d0 100644
--- a/clang/test/Analysis/ptr-arith.cpp
+++ b/clang/test/Analysis/ptr-arith.cpp
@@ -166,16 +166,18 @@ unsigned long ptr_arithmetic(void *p) {
return __builtin_bit_cast(unsigned long, p) + 1; // no-crash
}
-
-void escape(int*);
-
struct AllocOpaqueFlag {};
-void* operator new(unsigned long, void *ptr) noexcept { return ptr; }
-void* operator new(unsigned long, void *ptr, AllocOpaqueFlag const&) noexcept
{ return ptr; }
+void *operator new(unsigned long, void *ptr) noexcept { return ptr; }
+void *operator new(unsigned long, void *ptr, AllocOpaqueFlag const &) noexcept
{
+ return ptr;
+}
-void* operator new[](unsigned long, void* ptr) noexcept { return ptr; }
-void* operator new[](unsigned long, void* ptr, AllocOpaqueFlag const&);
+void *operator new[](unsigned long, void *ptr) noexcept { return ptr; }
+void *operator new[](unsigned long, void *ptr,
+ AllocOpaqueFlag const &) noexcept {
+ return ptr;
+}
struct Buffer {
char buf[100];
@@ -184,64 +186,56 @@ struct Buffer {
void checkPlacementNewArryInObject() {
Buffer buffer;
- int* array = new (&buffer) int[10];
- escape(array);
+ int *array = new (&buffer) int[10];
++array; // no warning
(void)*array;
}
void checkPlacementNewArrayInObjectOpaque() {
Buffer buffer;
- int* array = new (&buffer, AllocOpaqueFlag{}) int[10];
- escape(array);
+ int *array = new (&buffer, AllocOpaqueFlag{}) int[10];
++array; // no warning
(void)*array;
}
void checkPlacementNewArrayInArray() {
char buffer[100];
- int* array = new (buffer) int[10];
- escape(array);
+ int *array = new (buffer) int[10];
++array; // no warning
(void)*array;
}
void checkPlacementNewArrayInArrayOpaque() {
char buffer[100];
- int* array = new (buffer, AllocOpaqueFlag{}) int;
- escape(array);
+ int *array = new (buffer, AllocOpaqueFlag{}) int;
++array; // no warning
(void)*array;
}
void checkPlacementNewObjectInObject() {
Buffer buffer;
- int* array = new (&buffer) int;
- escape(array);
+ int *array = new (&buffer) int;
++array; // expected-warning{{Pointer arithmetic on non-array variables
relies on memory layout, which is dangerous}}
(void)*array;
}
void checkPlacementNewObjectInObjectOpaque() {
Buffer buffer;
- int* array = new (&buffer, AllocOpaqueFlag{}) int;
- escape(array);
- ++array; // expected-warning{{Pointer arithmetic on non-array variables
relies on memory layout, which is dangerous}}
+ int *array = new (&buffer, AllocOpaqueFlag{}) int;
+ ++array; // expected-warning{{Pointer arithmetic on non-array variables
relies on memory layout, which is dangerous}}
(void)*array;
}
void checkPlacementNewObjectInArray() {
char buffer[sizeof(int)];
- int* array = new (buffer) int;
- escape(array);
+ int *array = new (buffer) int;
++array; // no warning (FN)
(void)*array;
}
void checkPlacementNewObjectInArrayOpaque() {
char buffer[sizeof(int)];
- int* array = new (buffer, AllocOpaqueFlag{}) int;
- escape(array);
+ int *array = new (buffer, AllocOpaqueFlag{}) int;
++array; // no warning (FN)
(void)*array;
}
@@ -255,26 +249,25 @@ void checkPlacementNewSlices() {
*ptr = i;
}
++start; // no warning
- (void*)start;
+ (void *)start;
}
class BumpAlloc {
char *buffer;
char *offset;
+
public:
- BumpAlloc(int n): buffer(new char[n]), offset{buffer} {}
- ~BumpAlloc() {delete [] buffer;}
+ BumpAlloc(int n) : buffer(new char[n]), offset{buffer} {}
+ ~BumpAlloc() { delete[] buffer; }
- void* alloc(unsigned long size) {
- auto* ptr = offset;
+ void *alloc(unsigned long size) {
+ auto *ptr = offset;
offset += size;
return ptr;
}
};
-void* operator new(unsigned long size, BumpAlloc &ba) {
- return ba.alloc(size);
-}
+void *operator new(unsigned long size, BumpAlloc &ba) { return ba.alloc(size);
}
void checkPlacementSlab() {
BumpAlloc bump{10};
From b548e8132bf5d38f34a6edd6695063a6daf26da8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alva...@sonarsource.com>
Date: Fri, 29 Aug 2025 08:17:51 +0200
Subject: [PATCH 4/4] Opaque allocator
---
clang/test/Analysis/ptr-arith.cpp | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/clang/test/Analysis/ptr-arith.cpp
b/clang/test/Analysis/ptr-arith.cpp
index 9f1bc6b2f24d0..dec1ed19b5dbd 100644
--- a/clang/test/Analysis/ptr-arith.cpp
+++ b/clang/test/Analysis/ptr-arith.cpp
@@ -169,15 +169,11 @@ unsigned long ptr_arithmetic(void *p) {
struct AllocOpaqueFlag {};
void *operator new(unsigned long, void *ptr) noexcept { return ptr; }
-void *operator new(unsigned long, void *ptr, AllocOpaqueFlag const &) noexcept
{
- return ptr;
-}
+void *operator new(unsigned long, void *ptr, AllocOpaqueFlag const &) noexcept;
void *operator new[](unsigned long, void *ptr) noexcept { return ptr; }
void *operator new[](unsigned long, void *ptr,
- AllocOpaqueFlag const &) noexcept {
- return ptr;
-}
+ AllocOpaqueFlag const &) noexcept;
struct Buffer {
char buf[100];
@@ -222,7 +218,7 @@ void checkPlacementNewObjectInObject() {
void checkPlacementNewObjectInObjectOpaque() {
Buffer buffer;
int *array = new (&buffer, AllocOpaqueFlag{}) int;
- ++array; // expected-warning{{Pointer arithmetic on non-array variables
relies on memory layout, which is dangerous}}
+ ++array; // no warning (allocator is opaque)
(void)*array;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
