https://github.com/alejandro-alvarez-sonarsource created https://github.com/llvm/llvm-project/pull/155855
This pull improves the handling of placement new in`PointerArith`, fixing one family of false positives, and one of negatives: ### False Positives ```cpp 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 ```cpp 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 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/2] 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/2] 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 { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits