[analyzer] Improve handling of placement new in `PointerArith` #155855

alejandro-alvarez-sonarsource · 2025-08-28T15:23:44Z

This pull improves the handling of placement new inPointerArith, fixing one family of false positives, and one of negatives:

False Positives

  Buffer buffer;
  int* array = new (&buffer) int[10];
  ++array; // there should be no warning

The code above should flag the memory region buffer as reinterpreted, very much as reinterpret_cast would do. Note that in this particular case the placement new is inlined so the engine can track that *array points to the same region as buffer.

This is no-op if the placement new is opaque.

False Negatives

  Buffer buffer;
  int* array = new (&buffer) int;
  ++array; // there should be a warning

In this case, there is an implicit cast to void* when calling placement new. The memory region was marked as reinterpreted, and therefore later pointer arithmetic will not raise. I have added a condition to not consider a cast to void* as a reinterpretation, as an array of voids does not make much sense.

There are still some limitations, of course. For starters, if a single int is created in place of an array of unsigned char of exactly the same size, it will still be considered as an array. A convoluted example to make the point that I think it makes sense not to raise in this situation is in the test checkPlacementNewSlices.

CPP-6868

Flag the region as if it was `reinterpret_cast`-ed

llvmbot · 2025-08-28T15:24:18Z

@llvm/pr-subscribers-clang

Author: Alejandro Álvarez Ayllón (alejandro-alvarez-sonarsource)

Changes

This pull improves the handling of placement new inPointerArith, fixing one family of false positives, and one of negatives:

False Positives

  Buffer buffer;
  int* array = new (&amp;buffer) int[10];
  ++array; // there should be no warning

The code above should flag the memory region buffer as reinterpreted, very much as reinterpret_cast would do. Note that in this particular case the placement new is inlined so the engine can track that *array points to the same region as buffer.

This is no-op if the placement new is opaque.

False Negatives

  Buffer buffer;
  int* array = new (&amp;buffer) int;
  ++array; // there should be a warning

In this case, there is an implicit cast to void* when calling placement new. The memory region was marked as reinterpreted, and therefore later pointer arithmetic will not raise. I have added a condition to not consider a cast to void* as a reinterpretation, as an array of voids does not make much sense.

There are still some limitations, of course. For starters, if a single int is created in place of an array of unsigned char of exactly the same size, it will still be considered as an array. A convoluted example to make the point that I think it makes sense not to raise in this situation is in the test checkPlacementNewSlices.

CPP-6868

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

3 Files Affected:

(modified) clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp (+31-1)
(modified) clang/test/Analysis/PR24184.cpp (+1-1)
(modified) clang/test/Analysis/ptr-arith.cpp (+121)

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/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 {
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 = &bump;
+  ++why; // expected-warning {{Pointer arithmetic on non-array variables relies on memory layout, which is dangerous [alpha.core.PointerArithm]}}
+}

llvmbot · 2025-08-28T15:24:19Z

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

Author: Alejandro Álvarez Ayllón (alejandro-alvarez-sonarsource)

Changes

This pull improves the handling of placement new inPointerArith, fixing one family of false positives, and one of negatives:

False Positives

  Buffer buffer;
  int* array = new (&amp;buffer) int[10];
  ++array; // there should be no warning

The code above should flag the memory region buffer as reinterpreted, very much as reinterpret_cast would do. Note that in this particular case the placement new is inlined so the engine can track that *array points to the same region as buffer.

This is no-op if the placement new is opaque.

False Negatives

  Buffer buffer;
  int* array = new (&amp;buffer) int;
  ++array; // there should be a warning

In this case, there is an implicit cast to void* when calling placement new. The memory region was marked as reinterpreted, and therefore later pointer arithmetic will not raise. I have added a condition to not consider a cast to void* as a reinterpretation, as an array of voids does not make much sense.

There are still some limitations, of course. For starters, if a single int is created in place of an array of unsigned char of exactly the same size, it will still be considered as an array. A convoluted example to make the point that I think it makes sense not to raise in this situation is in the test checkPlacementNewSlices.

CPP-6868

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

3 Files Affected:

(modified) clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp (+31-1)
(modified) clang/test/Analysis/PR24184.cpp (+1-1)
(modified) clang/test/Analysis/ptr-arith.cpp (+121)

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/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 {
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 = &bump;
+  ++why; // expected-warning {{Pointer arithmetic on non-array variables relies on memory layout, which is dangerous [alpha.core.PointerArithm]}}
+}

steakhal

Looks sweet. I did the first round, I'll let others to finish the rest while I'm on vacation

steakhal · 2025-08-28T16:41:53Z

clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp

@@ -74,6 +74,26 @@ class PointerArithChecker

 REGISTER_MAP_WITH_PROGRAMSTATE(RegionState, const MemRegion *, AllocKind)

+namespace {


We follow the LLVM style guide, which promotes the keyword static. Anon namespaces should be used for class declarations though, but not bundling multiple decls into it usually.

Replaced with static

steakhal · 2025-08-28T16:45:08Z

clang/test/Analysis/ptr-arith.cpp

+struct AllocOpaqueFlag {};
+
+void* operator new(unsigned long, void *ptr) noexcept { return ptr; }
+void* operator new(unsigned long, void *ptr, AllocOpaqueFlag const&) noexcept { return ptr; }


I was expecting the AllocOpaqueFlag overloads to be opaque.

C&P mistake. Made them opaque, which makes one of the warning disappear (as it probably should).

steakhal · 2025-08-28T16:46:16Z

clang/test/Analysis/ptr-arith.cpp

+
+void checkPlacementNewArryInObject() {
+  Buffer buffer;
+  int* array = new (&buffer) int[10];


So I guess, alignment requirements are not checked. Thats sad, but We could extend this checker later.

This is a common mistake I think.

steakhal · 2025-08-28T16:49:06Z

clang/test/Analysis/ptr-arith.cpp

+  char *offset;
+public:
+  BumpAlloc(int n): buffer(new char[n]), offset{buffer} {}
+  ~BumpAlloc() {delete [] buffer;}


Could you clangformat the test code?
(The new parts ofc)

steakhal · 2025-08-28T16:49:49Z

clang/test/Analysis/ptr-arith.cpp

+void checkPlacementNewObjectInArray() {
+  char buffer[sizeof(int)];
+  int* array = new (buffer) int;
+  escape(array);


Why do we need to escape these btw?

Not really, I tend to do that to avoid warnings about garbage values, but, of course, that checker is disabled here. Removed as they do nothing in this case.

alejandro-alvarez-sonarsource added 2 commits August 28, 2025 16:44

CPP-6868 Fix FP in PointerArith for arrays created with placement-new

a206f1e

Flag the region as if it was `reinterpret_cast`-ed

Add new positive (which I believe is a TP)

f81b5e9

llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Aug 28, 2025

steakhal requested changes Aug 28, 2025

View reviewed changes

alejandro-alvarez-sonarsource added 2 commits August 29, 2025 08:14

Feedback

4b13c0a

Opaque allocator

b548e81

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[analyzer] Improve handling of placement new in `PointerArith` #155855

[analyzer] Improve handling of placement new in `PointerArith` #155855

Uh oh!

alejandro-alvarez-sonarsource commented Aug 28, 2025

Uh oh!

llvmbot commented Aug 28, 2025

False Positives

False Negatives

Uh oh!

llvmbot commented Aug 28, 2025

False Positives

False Negatives

Uh oh!

steakhal left a comment

Uh oh!

steakhal Aug 28, 2025

Uh oh!

alejandro-alvarez-sonarsource Aug 29, 2025

Uh oh!

steakhal Aug 28, 2025

Uh oh!

alejandro-alvarez-sonarsource Aug 29, 2025

Uh oh!

steakhal Aug 28, 2025

Uh oh!

steakhal Aug 28, 2025

Uh oh!

alejandro-alvarez-sonarsource Aug 29, 2025

Uh oh!

steakhal Aug 28, 2025

Uh oh!

alejandro-alvarez-sonarsource Aug 29, 2025

Uh oh!

Uh oh!

		@@ -74,6 +74,26 @@ class PointerArithChecker

		REGISTER_MAP_WITH_PROGRAMSTATE(RegionState, const MemRegion *, AllocKind)

		namespace {

[analyzer] Improve handling of placement new in PointerArith #155855

Are you sure you want to change the base?

[analyzer] Improve handling of placement new in PointerArith #155855

Uh oh!

Conversation

alejandro-alvarez-sonarsource commented Aug 28, 2025

False Positives

False Negatives

Uh oh!

llvmbot commented Aug 28, 2025

False Positives

False Negatives

Uh oh!

llvmbot commented Aug 28, 2025

False Positives

False Negatives

Uh oh!

steakhal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

[analyzer] Improve handling of placement new in `PointerArith` #155855

[analyzer] Improve handling of placement new in `PointerArith` #155855