Author: Alejandro Álvarez Ayllón Date: 2025-09-05T16:36:59+02:00 New Revision: 80d4e2439bc28d028a694729270e67eab76f15bd
URL: https://github.com/llvm/llvm-project/commit/80d4e2439bc28d028a694729270e67eab76f15bd DIFF: https://github.com/llvm/llvm-project/commit/80d4e2439bc28d028a694729270e67eab76f15bd.diff LOG: [analyzer] Improve handling of placement new in `PointerArith` (#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 Added: Modified: clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp clang/test/Analysis/PR24184.cpp clang/test/Analysis/ptr-arith.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp index 30e01e73eb18f..f2fc9219912ad 100644 --- a/clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp @@ -74,6 +74,22 @@ class PointerArithChecker REGISTER_MAP_WITH_PROGRAMSTATE(RegionState, const MemRegion *, AllocKind) +static bool isArrayPlacementNew(const CXXNewExpr *NE) { + return NE->isArray() && NE->getNumPlacementArgs() > 0; +} + +static 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; +} + void PointerArithChecker::checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const { // TODO: intentional leak. Some information is garbage collected too early, @@ -244,13 +260,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..dec1ed19b5dbd 100644 --- a/clang/test/Analysis/ptr-arith.cpp +++ b/clang/test/Analysis/ptr-arith.cpp @@ -165,3 +165,113 @@ void LValueToRValueBitCast_dumps(void *p, char (*array)[8]) { unsigned long ptr_arithmetic(void *p) { return __builtin_bit_cast(unsigned long, p) + 1; // no-crash } + +struct AllocOpaqueFlag {}; + +void *operator new(unsigned long, void *ptr) 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; + +struct Buffer { + char buf[100]; + int padding; +}; + +void checkPlacementNewArryInObject() { + Buffer buffer; + int *array = new (&buffer) int[10]; + ++array; // no warning + (void)*array; +} + +void checkPlacementNewArrayInObjectOpaque() { + Buffer buffer; + int *array = new (&buffer, AllocOpaqueFlag{}) int[10]; + ++array; // no warning + (void)*array; +} + +void checkPlacementNewArrayInArray() { + char buffer[100]; + int *array = new (buffer) int[10]; + ++array; // no warning + (void)*array; +} + +void checkPlacementNewArrayInArrayOpaque() { + char buffer[100]; + int *array = new (buffer, AllocOpaqueFlag{}) int; + ++array; // no warning + (void)*array; +} + +void checkPlacementNewObjectInObject() { + Buffer buffer; + 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; + ++array; // no warning (allocator is opaque) + (void)*array; +} + +void checkPlacementNewObjectInArray() { + char buffer[sizeof(int)]; + int *array = new (buffer) int; + ++array; // no warning (FN) + (void)*array; +} + +void checkPlacementNewObjectInArrayOpaque() { + char buffer[sizeof(int)]; + int *array = new (buffer, AllocOpaqueFlag{}) int; + ++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]}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
