https://github.com/mark-zilber updated https://github.com/llvm/llvm-project/pull/233525
>From 46a47b302f9f5ec8a9eb5d8febff2d9448771127 Mon Sep 17 00:00:00 2001 From: Mark Zilber <[email protected]> Date: Sat, 17 Oct 2026 15:36:07 +0400 Subject: [PATCH 1/2] [analyzer] Add alpha.cplusplus.PlacementSelfCopy checker The checker finds placement new expressions that initialize the new object from the storage it is placed into, e.g. `::new (p) T(*p)`. This is the `ctordtor.PlacementSelfCopy` entry of the potential checkers list. When a constructor of the new object receives a reference to the target storage, the checker records the writes into that storage and reports a read of a part that has already been overwritten. Reads through `this` are allowed. CSA binds an initializer list only after evaluating all of its elements, so for `::new (p) Agg{...}` and array placement new the elements preceding the current one are treated as written. A copy or move constructor that is not inlined (a trivial copy, a constructor without a visible definition, or one that exceeded the analysis budget) is reported whenever its source is the target storage. Reports for `std::construct_at` would end inside the standard library and be suppressed, so they are emitted at the call site. Tested on abseil-cpp, protobuf and parts of LLVM (about 1400 translation units) with no reports. Part of #233519 --- clang/docs/ReleaseNotes.md | 6 + clang/docs/analyzer/checkers.md | 49 ++ .../clang/StaticAnalyzer/Checkers/Checkers.td | 5 + .../StaticAnalyzer/Checkers/CMakeLists.txt | 1 + .../Checkers/PlacementSelfCopyChecker.cpp | 438 ++++++++++++++++++ .../Inputs/system-header-simulator-cxx.h | 6 + clang/test/Analysis/placement-self-copy.cpp | 252 ++++++++++ 7 files changed, 757 insertions(+) create mode 100644 clang/lib/StaticAnalyzer/Checkers/PlacementSelfCopyChecker.cpp create mode 100644 clang/test/Analysis/placement-self-copy.cpp diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 9e5d92cae727cc..0b8ea41e3a3826 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -1255,6 +1255,12 @@ features cannot lower the translation-unit ABI level; % - Moved checkers % - Diagnostic changes +#### New checkers and features + +- Added the ``alpha.cplusplus.PlacementSelfCopy`` checker, which reports + placement new expressions and ``std::construct_at`` calls that initialize the + new object from the storage it is placed into, e.g. ``::new (p) T(*p)``. + #### Improvements - The lock-order-reversal check in ``alpha.unix.PthreadLock`` is now disabled by default. diff --git a/clang/docs/analyzer/checkers.md b/clang/docs/analyzer/checkers.md index e2bbf52943bb87..319b368ed7fa82 100644 --- a/clang/docs/analyzer/checkers.md +++ b/clang/docs/analyzer/checkers.md @@ -3503,6 +3503,55 @@ void bad_insert3(std::vector &v1, std::vector &v2) { } ``` +(alpha-cplusplus-placementselfcopy)= + +#### alpha.cplusplus.PlacementSelfCopy (C++) + +Check for placement new expressions that initialize the new object from the +storage it is placed into, e.g. `::new (p) T(*p)`. Construction overwrites that +storage, so a read of a part that has already been written sees the new value +of the object being created. + +If the constructor is inlined, the checker follows its writes to the storage +and reports the first read of an overwritten part. Reads through `this` are +allowed. The elements of an initializer list are treated as written in order. +A copy or move constructor that is not inlined, such as a trivial copy or a +constructor without a visible definition, is reported whenever its source is +the storage itself. Calls of `std::construct_at` are reported at the call site. + +```cpp +struct Buffer { + int *Data; + int Size; + Buffer(const Buffer &O) : Data(new int[O.Size]), Size(O.Size) { + for (int I = 0; I < Size; ++I) + Data[I] = O.Data[I]; // warn: placement new reads a part of its storage + // that it has already overwritten + } +}; + +void reconstruct(Buffer *B) { + ::new (B) Buffer(*B); +} + +struct Point { + int X, Y; +}; + +void swap_coordinates(Point *P) { + ::new (P) Point{P->Y, P->X}; // warn: P->X is read after X is initialized +} + +struct Trivial { + int X; +}; + +void trivial_copy(Trivial *T) { + ::new (T) Trivial(*T); // warn: object constructed by placement new is + // initialized from the storage it is placed into +} +``` + (alpha-cplusplus-smartptr)= #### alpha.cplusplus.SmartPtr (C++) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 9905649801e85b..ce71e6e501d6b1 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -813,6 +813,11 @@ def MismatchedIteratorChecker : Checker<"MismatchedIterator">, Dependencies<[IteratorModeling]>, Documentation<HasDocumentation>; +def PlacementSelfCopyChecker : Checker<"PlacementSelfCopy">, + HelpText<"Check for placement new expressions that initialize the new " + "object from the storage it is placed into">, + Documentation<HasDocumentation>; + def SmartPtrChecker: Checker<"SmartPtr">, HelpText<"Find the dereference of null SmrtPtr">, Dependencies<[SmartPtrModeling]>, diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index f136bd5434113b..a1c2c604020214 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -91,6 +91,7 @@ add_clang_library(clangStaticAnalyzerCheckers OpaqueSTLFunctionsModeling.cpp OSObjectCStyleCast.cpp PaddingChecker.cpp + PlacementSelfCopyChecker.cpp PointerArithChecker.cpp PointerSubChecker.cpp PthreadLockChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/PlacementSelfCopyChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PlacementSelfCopyChecker.cpp new file mode 100644 index 00000000000000..5b06836018ae13 --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/PlacementSelfCopyChecker.cpp @@ -0,0 +1,438 @@ +//===- PlacementSelfCopyChecker.cpp -----------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// Defines a checker that finds placement new expressions whose initializer +// reads a part of the target storage that has already been overwritten, e.g. +// `::new (p) T(*p)`. A copy or move constructor that is not inlined is +// reported whenever its source is the target storage. +// +//===----------------------------------------------------------------------===// + +#include "clang/AST/ExprCXX.h" +#include "clang/AST/ParentMap.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" + +using namespace clang; +using namespace ento; + +using PlacementKey = std::pair<const CXXNewExpr *, const StackFrame *>; +REGISTER_MAP_WITH_PROGRAMSTATE(PlacementTargets, PlacementKey, + const MemRegion *) +REGISTER_SET_WITH_PROGRAMSTATE(SelfSourcedTargets, const MemRegion *) +REGISTER_SET_WITH_PROGRAMSTATE(OverwrittenParts, const MemRegion *) +REGISTER_TRAIT_WITH_PROGRAMSTATE(PendingSelfCopy, const MemRegion *) +REGISTER_TRAIT_WITH_PROGRAMSTATE(ReadOverwritten, const MemRegion *) + +namespace { +class PlacementSelfCopyChecker + : public Checker<check::PostCall, check::PreCall, check::Bind, + check::Location, check::PostStmt<CXXNewExpr>> { + const BugType BT{this, "Placement new initialized from its own storage", + categories::MemoryError}; + const BugType ReadBT{this, "Placement new reads overwritten storage", + categories::MemoryError}; + + void reportRead(const Expr *E, const MemRegion *Overwritten, + CheckerContext &C) const; + void reportSelfCopy(const Expr *Arg, CheckerContext &C) const; + void reportPendingSelfCopy(const CallEvent &Call, CheckerContext &C) const; + void checkOverwrittenRead(const Stmt *S, const Expr *E, + const MemRegion *Source, CheckerContext &C) const; + +public: + void checkPostCall(const CallEvent &Call, CheckerContext &C) const; + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; + void checkBind(SVal Loc, SVal Val, const Stmt *S, bool AtDeclInit, + CheckerContext &C) const; + void checkLocation(SVal Loc, bool IsLoad, const Stmt *S, + CheckerContext &C) const; + void checkPostStmt(const CXXNewExpr *NE, CheckerContext &C) const; +}; +} // namespace + +namespace { +struct Extent { + const MemRegion *Base; + int64_t Begin; + std::optional<int64_t> End; +}; +} // namespace + +static std::optional<Extent> getExtent(const MemRegion *R, ASTContext &Ctx) { + RegionOffset Offset = R->getAsOffset(); + if (!Offset.isValid() || Offset.hasSymbolicOffset()) + return std::nullopt; + Extent E{Offset.getRegion(), Offset.getOffset(), std::nullopt}; + if (const auto *TR = dyn_cast<TypedValueRegion>(R)) { + QualType Ty = TR->getValueType(); + if (!Ty->isIncompleteType()) + E.End = E.Begin + Ctx.getTypeSize(Ty); + } + return E; +} + +static bool overlaps(const MemRegion *A, const MemRegion *B, ASTContext &Ctx) { + if (A == B || A->isSubRegionOf(B) || B->isSubRegionOf(A)) + return true; + std::optional<Extent> EA = getExtent(A, Ctx), EB = getExtent(B, Ctx); + if (!EA || !EB || EA->Base != EB->Base) + return false; + return (!EB->End || EA->Begin < *EB->End) && + (!EA->End || EB->Begin < *EA->End); +} + +static bool isWithin(const MemRegion *R, const MemRegion *Container, + ASTContext &Ctx) { + if (R == Container || R->isSubRegionOf(Container)) + return true; + std::optional<Extent> ER = getExtent(R, Ctx), EC = getExtent(Container, Ctx); + if (!ER || !EC || ER->Base != EC->Base || ER->Begin < EC->Begin) + return false; + return !EC->End || (ER->End && *ER->End <= *EC->End); +} + +static bool isThisBased(const Expr *E) { + while (E) { + E = E->IgnoreParenCasts(); + if (isa<CXXThisExpr>(E)) + return true; + if (const auto *ME = dyn_cast<MemberExpr>(E)) + E = ME->getBase(); + else if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(E)) + E = ASE->getBase(); + else if (const auto *BO = dyn_cast<BinaryOperator>(E); + BO && BO->isAssignmentOp()) + E = BO->getLHS(); + else if (const auto *UO = dyn_cast<UnaryOperator>(E); + UO && UO->getOpcode() == UO_Deref) + E = UO->getSubExpr(); + else + return false; + } + return false; +} + +static const MemRegion *findSelfSourcedTarget(ProgramStateRef State, + const MemRegion *R, + CheckerContext &C) { + for (const MemRegion *Target : State->get<SelfSourcedTargets>()) + if (isWithin(R, Target, C.getASTContext())) + return Target; + return nullptr; +} + +static const MemRegion *findOverwritten(ProgramStateRef State, + const MemRegion *R, CheckerContext &C) { + for (const MemRegion *W : State->get<OverwrittenParts>()) + if (overlaps(R, W, C.getASTContext())) + return W; + return nullptr; +} + +// CSA binds an init list only after evaluating all elements; recompute order. +static const MemRegion *walkInitList(PlacementKey Key, const MemRegion *Target, + const Stmt *S, const MemRegion *Source, + const MemRegion *&Before, + CheckerContext &C) { + Before = nullptr; + const auto &[NE, Frame] = Key; + if (Frame != C.getStackFrame()) + return nullptr; + const auto *Init = dyn_cast_if_present<InitListExpr>(NE->getInitializer()); + if (!Init) + return nullptr; + + const ParentMap &PM = C.getCurrentAnalysisDeclContext()->getParentMap(); + SmallVector<std::pair<const InitListExpr *, unsigned>, 4> Path; + for (const Stmt *Child = S, *Parent = PM.getParent(S); Parent; + Child = Parent, Parent = PM.getParent(Parent)) { + if (const auto *ILE = dyn_cast<InitListExpr>(Parent)) { + ArrayRef<Expr *> Inits = ILE->inits(); + const auto *It = llvm::find(Inits, Child); + if (It == Inits.end()) + return nullptr; + Path.emplace_back(ILE, It - Inits.begin()); + } + if (Parent == Init) + break; + } + if (Path.empty() || Path.back().first != Init) + return nullptr; + + ASTContext &Ctx = C.getASTContext(); + MemRegionManager &MRM = C.getStoreManager().getRegionManager(); + SValBuilder &SVB = C.getSValBuilder(); + const auto *R = cast<SubRegion>(Target); + for (const auto &[ILE, Index] : llvm::reverse(Path)) { + QualType Ty = ILE->getType(); + if (const auto *AT = Ctx.getAsArrayType(Ty)) { + QualType ElemTy = AT->getElementType(); + for (unsigned I = 0; I != Index && Source && !Before; ++I) { + const MemRegion *Elem = + MRM.getElementRegion(ElemTy, SVB.makeArrayIndex(I), R, Ctx); + if (overlaps(Source, Elem, Ctx)) + Before = Elem; + } + R = MRM.getElementRegion(ElemTy, SVB.makeArrayIndex(Index), R, Ctx); + continue; + } + + const auto *RD = Ty->getAsCXXRecordDecl(); + if (!RD || RD->isUnion() || RD->getNumBases()) + return nullptr; + unsigned I = 0; + const FieldRegion *Next = nullptr; + for (const FieldDecl *FD : RD->fields()) { + if (FD->isUnnamedBitField()) + continue; + const FieldRegion *Field = MRM.getFieldRegion(FD, R); + if (I++ == Index) { + Next = Field; + break; + } + if (Source && !Before && overlaps(Source, Field, Ctx)) + Before = Field; + } + if (!Next) + return nullptr; + R = Next; + } + return R; +} + +static const MemRegion *getDestination(PlacementKey Key, + const MemRegion *Target, + const CXXConstructorCall &Ctor, + const MemRegion *This, + CheckerContext &C) { + const Stmt *S = Ctor.getOriginExpr(); + if (Key.first->getConstructExpr() == S) + return Key.second == C.getStackFrame() ? This : nullptr; + const MemRegion *Before; + return walkInitList(Key, Target, S, nullptr, Before, C); +} + +void PlacementSelfCopyChecker::checkOverwrittenRead(const Stmt *S, + const Expr *E, + const MemRegion *Source, + CheckerContext &C) const { + if (isThisBased(E)) + return; + ProgramStateRef State = C.getState(); + Source = Source->StripCasts(); + const MemRegion *Overwritten = findSelfSourcedTarget(State, Source, C) + ? findOverwritten(State, Source, C) + : nullptr; + for (const auto &[Key, Target] : State->get<PlacementTargets>()) { + if (Overwritten) + break; + walkInitList(Key, Target, S, Source, Overwritten, C); + } + if (Overwritten) + reportRead(E, Overwritten, C); +} + +void PlacementSelfCopyChecker::reportRead(const Expr *E, + const MemRegion *Overwritten, + CheckerContext &C) const { + ExplodedNode *N = + C.generateErrorNode(C.getState()->set<ReadOverwritten>(Overwritten)); + if (!N) + return; + auto R = std::make_unique<PathSensitiveBugReport>( + ReadBT, + "Placement new reads a part of its storage that it has already " + "overwritten", + N); + R->addRange(E->getSourceRange()); + C.emitReport(std::move(R)); +} + +void PlacementSelfCopyChecker::reportSelfCopy(const Expr *Arg, + CheckerContext &C) const { + ExplodedNode *N = C.generateErrorNode(); + if (!N) + return; + auto R = std::make_unique<PathSensitiveBugReport>( + BT, + "Object constructed by placement new is initialized from the storage " + "it is placed into", + N); + R->addRange(Arg->getSourceRange()); + bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(Arg), *R); + C.emitReport(std::move(R)); +} + +// Reports inside the standard library are suppressed; report at the caller. +void PlacementSelfCopyChecker::reportPendingSelfCopy(const CallEvent &Call, + CheckerContext &C) const { + const MemRegion *Target = C.getState()->get<PendingSelfCopy>(); + const Expr *Origin = dyn_cast_if_present<Expr>(Call.getOriginExpr()); + if (!Target || !Origin || + C.getSourceManager().isInSystemHeader(Origin->getBeginLoc())) + return; + for (unsigned I = 0, E = Call.getNumArgs(); I != E; ++I) { + const MemRegion *Arg = Call.getArgSVal(I).getAsRegion(); + if (Arg && !Call.getArgExpr(I)->getType()->isPointerType() && + overlaps(Arg->StripCasts(), Target, C.getASTContext())) { + reportSelfCopy(Call.getArgExpr(I), C); + return; + } + } + C.addTransition(C.getState()->remove<PendingSelfCopy>()); +} + +void PlacementSelfCopyChecker::checkPostCall(const CallEvent &Call, + CheckerContext &C) const { + if (const auto *Alloc = dyn_cast<CXXAllocatorCall>(&Call)) { + const CXXNewExpr *NE = Alloc->getOriginExpr(); + const FunctionDecl *OperatorNew = NE->getOperatorNew(); + if (!OperatorNew || !OperatorNew->isReservedGlobalPlacementOperator()) + return; + const MemRegion *Target = Call.getReturnValue().getAsRegion(); + if (!Target) + return; + C.addTransition(C.getState()->set<PlacementTargets>({NE, C.getStackFrame()}, + Target->StripCasts())); + return; + } + + const auto *Ctor = dyn_cast<CXXConstructorCall>(&Call); + if (!Ctor) { + reportPendingSelfCopy(Call, C); + return; + } + const auto *CD = dyn_cast_if_present<CXXConstructorDecl>(Ctor->getDecl()); + if (!CD || !CD->isCopyOrMoveConstructor()) + return; + const MemRegion *This = Ctor->getCXXThisVal().getAsRegion(); + const MemRegion *Source = Ctor->getArgSVal(0).getAsRegion(); + if (!This || !Source) + return; + This = This->StripCasts(); + Source = Source->StripCasts(); + + ProgramStateRef State = C.getState(); + const Stmt *S = Ctor->getOriginExpr(); + for (const auto &[Key, Target] : State->get<PlacementTargets>()) { + const MemRegion *Dest = getDestination(Key, Target, *Ctor, This, C); + if (!Dest) + continue; + // Inlined constructors are checked per read unless built in a temporary. + if (C.wasInlined && isWithin(This, Target, C.getASTContext())) + return; + if (!overlaps(Source, Dest, C.getASTContext())) + return; + if (C.getSourceManager().isInSystemHeader(S->getBeginLoc())) + C.addTransition(State->set<PendingSelfCopy>(Target)); + else + reportSelfCopy(Ctor->getArgExpr(0), C); + return; + } +} + +void PlacementSelfCopyChecker::checkPreCall(const CallEvent &Call, + CheckerContext &C) const { + const auto *Ctor = dyn_cast<CXXConstructorCall>(&Call); + if (!Ctor) + return; + ProgramStateRef State = C.getState(); + if (State->get<PlacementTargets>().isEmpty()) + return; + ArrayRef<ParmVarDecl *> Params = Ctor->parameters(); + unsigned NumArgs = std::min<size_t>(Ctor->getNumArgs(), Params.size()); + for (unsigned I = 0; I != NumArgs; ++I) { + if (!Params[I]->getType()->isReferenceType()) + continue; + if (const MemRegion *Source = Ctor->getArgSVal(I).getAsRegion()) { + checkOverwrittenRead(Ctor->getOriginExpr(), Ctor->getArgExpr(I), Source, + C); + if (C.isDifferent()) + return; + } + } + + const MemRegion *This = Ctor->getCXXThisVal().getAsRegion(); + if (!This) + return; + This = This->StripCasts(); + for (const auto &[Key, Target] : State->get<PlacementTargets>()) { + const MemRegion *Dest = getDestination(Key, Target, *Ctor, This, C); + if (!Dest) + continue; + for (unsigned I = 0; I != NumArgs; ++I) { + const MemRegion *Source = Ctor->getArgSVal(I).getAsRegion(); + if (Params[I]->getType()->isReferenceType() && Source && + overlaps(Source->StripCasts(), Dest, C.getASTContext())) { + C.addTransition(State->add<SelfSourcedTargets>(Target)); + return; + } + } + return; + } +} + +void PlacementSelfCopyChecker::checkBind(SVal Loc, SVal Val, const Stmt *S, + bool AtDeclInit, + CheckerContext &C) const { + const MemRegion *R = Loc.getAsRegion(); + if (!R) + return; + R = R->StripCasts(); + ProgramStateRef State = C.getState(); + if (!findSelfSourcedTarget(State, R, C) || + State->contains<OverwrittenParts>(R)) + return; + const NoteTag *Note = + C.getNoteTag([this, R](PathSensitiveBugReport &BR) -> std::string { + if (&BR.getBugType() != &ReadBT || + BR.getErrorNode()->getState()->get<ReadOverwritten>() != R) + return ""; + return "Part of the storage is overwritten here"; + }); + C.addTransition(State->add<OverwrittenParts>(R), Note); +} + +void PlacementSelfCopyChecker::checkLocation(SVal Loc, bool IsLoad, + const Stmt *S, + CheckerContext &C) const { + if (!IsLoad || C.getState()->get<PlacementTargets>().isEmpty()) + return; + const auto *E = dyn_cast_if_present<Expr>(S); + const MemRegion *R = Loc.getAsRegion(); + if (E && R) + checkOverwrittenRead(S, E, R, C); +} + +void PlacementSelfCopyChecker::checkPostStmt(const CXXNewExpr *NE, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + PlacementKey Key{NE, C.getStackFrame()}; + const MemRegion *const *Target = State->get<PlacementTargets>(Key); + if (!Target) + return; + State = State->remove<PlacementTargets>(Key); + State = State->remove<SelfSourcedTargets>(*Target); + for (const MemRegion *W : State->get<OverwrittenParts>()) + if (isWithin(W, *Target, C.getASTContext())) + State = State->remove<OverwrittenParts>(W); + C.addTransition(State); +} + +void ento::registerPlacementSelfCopyChecker(CheckerManager &Mgr) { + Mgr.registerChecker<PlacementSelfCopyChecker>(); +} + +bool ento::shouldRegisterPlacementSelfCopyChecker(const CheckerManager &) { + return true; +} diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h index 78fe4994b8d44f..6a077b57f54c84 100644 --- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h +++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h @@ -1194,6 +1194,12 @@ void* operator new[] (std::size_t size, void* ptr) throw() { return ptr; }; void operator delete (void* ptr, void*) throw() {}; void operator delete[] (void* ptr, void*) throw() {}; +namespace std { +template <class T, class... Args> T *construct_at(T *p, Args &&...args) { + return ::new (static_cast<void *>(p)) T(static_cast<Args &&>(args)...); +} +} + namespace __cxxabiv1 { extern "C" { extern char *__cxa_demangle(const char *mangled_name, diff --git a/clang/test/Analysis/placement-self-copy.cpp b/clang/test/Analysis/placement-self-copy.cpp new file mode 100644 index 00000000000000..826c709e577535 --- /dev/null +++ b/clang/test/Analysis/placement-self-copy.cpp @@ -0,0 +1,252 @@ +// RUN: %clang_analyze_cc1 -std=c++20 %s \ +// RUN: -analyzer-checker=core,alpha.cplusplus.PlacementSelfCopy \ +// RUN: -analyzer-output=text -verify -triple x86_64-unknown-linux-gnu + +#include "Inputs/system-header-simulator-cxx.h" + +struct Trivial { + int X; +}; + +struct NonTrivial { + int *P; + NonTrivial(const NonTrivial &Other); + NonTrivial(NonTrivial &&Other); +}; + +void copy_from_itself_trivial(Trivial *Dst) { + ::new (Dst) Trivial(*Dst); // expected-warning{{Object constructed by placement new is initialized from the storage it is placed into}} expected-note{{Object constructed by placement new is initialized from the storage it is placed into}} +} + +void copy_from_itself(NonTrivial *Dst) { + ::new (Dst) NonTrivial(*Dst); // expected-warning{{Object constructed by placement new is initialized from the storage it is placed into}} expected-note{{Object constructed by placement new is initialized from the storage it is placed into}} +} + +void move_from_itself(NonTrivial *Dst) { + ::new (Dst) NonTrivial(static_cast<NonTrivial &&>(*Dst)); // expected-warning{{Object constructed by placement new is initialized from the storage it is placed into}} expected-note{{Object constructed by placement new is initialized from the storage it is placed into}} +} + +void copy_through_alias(NonTrivial *Dst) { + NonTrivial *Alias = Dst; // expected-note{{'Alias' initialized here}} + ::new (Dst) NonTrivial(*Alias); // expected-warning{{Object constructed by placement new is initialized from the storage it is placed into}} expected-note{{Object constructed by placement new is initialized from the storage it is placed into}} +} + +void copy_from_local(const NonTrivial &Src) { + NonTrivial Obj(Src); // expected-note{{'Obj' initialized here}} + ::new (&Obj) NonTrivial(Obj); // expected-warning{{Object constructed by placement new is initialized from the storage it is placed into}} expected-note{{Object constructed by placement new is initialized from the storage it is placed into}} +} + +struct Outer { + Trivial Inner; +}; + +void copy_from_enclosing_object(Outer *O) { + ::new (&O->Inner) Trivial(O->Inner); // expected-warning{{Object constructed by placement new is initialized from the storage it is placed into}} expected-note{{Object constructed by placement new is initialized from the storage it is placed into}} +} + +void copy_from_other(NonTrivial *Dst, NonTrivial *Src) { + ::new (Dst) NonTrivial(*Src); // no-warning +} + +struct Pair { + Trivial A; + Trivial B; +}; + +void copy_from_sibling_field(Pair *P) { + ::new (&P->A) Trivial(P->B); // no-warning +} + +struct Node { + Node *Next; + explicit Node(Node *N) : Next(N) {} +}; + +void self_referential_pointer(Node *Storage) { + ::new (Storage) Node(Storage); // no-warning +} + +void regular_new(NonTrivial *Src) { + NonTrivial *P = new NonTrivial(*Src); // no-warning + (void)P; +} + +struct Deep { + int *P; + int N; + Deep(const Deep &O) : P(new int[O.N]), N(O.N) { P[0] = O.P[0]; } // expected-note{{Part of the storage is overwritten here}} expected-warning{{Placement new reads a part of its storage that it has already overwritten}} expected-note{{Placement new reads a part of its storage that it has already overwritten}} +}; + +void deep_copy_reads_overwritten_pointer(Deep *Dst) { + ::new (Dst) Deep(*Dst); // expected-note{{Calling copy constructor for 'Deep'}} +} + +struct Swapped { + int X, Y; + Swapped(const Swapped &O) : X(O.Y), Y(O.X) {} // expected-note{{Part of the storage is overwritten here}} expected-warning{{Placement new reads a part of its storage that it has already overwritten}} expected-note{{Placement new reads a part of its storage that it has already overwritten}} +}; + +void swapped_copy(Swapped *Dst) { + ::new (Dst) Swapped(*Dst); // expected-note{{Calling copy constructor for 'Swapped'}} +} + +struct Memberwise { + int X, Y; + Memberwise(const Memberwise &O) : X(O.X), Y(O.Y) {} +}; + +void memberwise_copy(Memberwise *Dst) { + ::new (Dst) Memberwise(*Dst); // no-warning +} + +struct ReadsOwnField { + int X, Y; + ReadsOwnField(const ReadsOwnField &O) : X(O.X), Y(X + 1) {} +}; + +void reads_own_field(ReadsOwnField *Dst) { + ::new (Dst) ReadsOwnField(*Dst); // no-warning +} + +struct IgnoresSource { + int Field; + IgnoresSource(IgnoresSource &) { Field = 0; } +}; + +void ignores_source(IgnoresSource *Dst) { + ::new (Dst) IgnoresSource(*Dst); // no-warning +} + +struct FromInt { + int Field; + explicit FromInt(int &V) : Field(V + 1) {} +}; + +void reads_before_writing(FromInt *Dst) { + ::new (Dst) FromInt(Dst->Field); // no-warning +} + +struct HoldsTrivial { + Trivial T; + int K; + HoldsTrivial(const HoldsTrivial &O) : T(O.T), K(O.K) {} +}; + +void member_copied_before_overwrite(HoldsTrivial *Dst) { + ::new (Dst) HoldsTrivial(*Dst); // no-warning +} + +void construct_at_trivial(Trivial *Dst) { + std::construct_at(Dst, *Dst); // expected-warning{{Object constructed by placement new is initialized from the storage it is placed into}} expected-note{{Object constructed by placement new is initialized from the storage it is placed into}} +} + +struct SwappedInStdlib { + int X, Y; + SwappedInStdlib(const SwappedInStdlib &O) : X(O.Y), Y(O.X) {} // expected-note{{Part of the storage is overwritten here}} expected-warning{{Placement new reads a part of its storage that it has already overwritten}} expected-note{{Placement new reads a part of its storage that it has already overwritten}} +}; + +void construct_at_swapped(SwappedInStdlib *Dst) { + std::construct_at(Dst, *Dst); // expected-note{{Calling 'construct_at<SwappedInStdlib, SwappedInStdlib &>'}} + // expected-note@Inputs/system-header-simulator-cxx.h:* {{Calling copy constructor for 'SwappedInStdlib'}} +} + +void construct_at_other(Trivial *Dst, Trivial *Src) { + std::construct_at(Dst, *Src); // no-warning +} + +void array_trivial(Trivial *Dst) { + ::new (Dst) Trivial[1]{*Dst}; // expected-warning{{Object constructed by placement new is initialized from the storage it is placed into}} expected-note{{Object constructed by placement new is initialized from the storage it is placed into}} +} + +void array_deep(Deep *Dst) { + ::new (Dst) Deep[1]{*Dst}; // expected-warning{{Object constructed by placement new is initialized from the storage it is placed into}} expected-note{{Object constructed by placement new is initialized from the storage it is placed into}} +} + +void array_second_element(Trivial *Dst) { + ::new (Dst) Trivial[2]{Dst[1], Dst[0]}; // expected-warning{{Placement new reads a part of its storage that it has already overwritten}} expected-note{{Placement new reads a part of its storage that it has already overwritten}} +} + +void array_other(Trivial *Dst, Trivial *Src) { + ::new (Dst) Trivial[1]{*Src}; // no-warning +} + +struct Agg { + int X; + int Y; +}; + +void aggregate_swapped(Agg *Dst) { + ::new (Dst) Agg{Dst->Y, Dst->X}; // expected-warning{{Placement new reads a part of its storage that it has already overwritten}} expected-note{{Placement new reads a part of its storage that it has already overwritten}} +} + +void aggregate_same_order(Agg *Dst) { + ::new (Dst) Agg{Dst->X, Dst->Y}; // no-warning +} + +struct Nested { + Agg Inner; + int Z; +}; + +void nested_aggregate(Nested *Dst) { + ::new (Dst) Nested{{Dst->Z, Dst->Inner.X}, 0}; // expected-warning{{Placement new reads a part of its storage that it has already overwritten}} expected-note{{Placement new reads a part of its storage that it has already overwritten}} +} + +void aggregate_copy(Agg *Dst) { + ::new (Dst) Agg{*Dst}; // expected-warning{{Object constructed by placement new is initialized from the storage it is placed into}} expected-note{{Object constructed by placement new is initialized from the storage it is placed into}} +} + +struct ChainedReset { + int *Begin, *End; + ChainedReset(const ChainedReset &) { Begin = End = nullptr; } +}; + +void chained_assignment_in_self_copy(ChainedReset *Dst) { + ::new (Dst) ChainedReset(*Dst); // no-warning +} + +struct Stream { + int *Cur, *End; + explicit Stream(int &Source) { Cur = End = nullptr; (void)Source; } +}; + +void chained_assignment_from_other(Stream *Dst, int &Other) { + ::new (Dst) Stream(Other); // no-warning +} + +struct Ends { + int Values[2]; + int &get(int I) { return Values[I]; } + explicit Ends(const int (&Init)[2]) : Values{Init[0], Init[1]} { + get(0) += 1; + } +}; + +void member_call_from_other(Ends *Dst, const int (&Init)[2]) { + ::new (Dst) Ends(Init); // no-warning +} + +struct Recursive; +void place(Recursive *Dst, const Recursive &Src, Recursive *Scratch, + const Recursive *Other, int Depth); + +struct Recursive { + int X, Y; + Recursive(const Recursive &O, Recursive *Scratch, const Recursive *Other, + int Depth) + : X(O.Y) { // expected-note{{Part of the storage is overwritten here}} + if (Depth) // expected-note{{'Depth' is 1}} expected-note{{Taking true branch}} + place(Scratch, *Other, nullptr, nullptr, 0); + Y = O.X; // expected-warning{{Placement new reads a part of its storage that it has already overwritten}} expected-note{{Placement new reads a part of its storage that it has already overwritten}} + } +}; + +void place(Recursive *Dst, const Recursive &Src, Recursive *Scratch, + const Recursive *Other, int Depth) { + ::new (Dst) Recursive(Src, Scratch, Other, Depth); // expected-note{{Calling constructor for 'Recursive'}} +} + +void same_new_expression_in_nested_frame(Recursive *Dst, Recursive *Scratch, + const Recursive *Other) { + place(Dst, *Dst, Scratch, Other, 1); // expected-note{{Calling 'place'}} +} >From f6852cd1d997d432fb5caed80e0997e086323193 Mon Sep 17 00:00:00 2001 From: Mark Zilber <[email protected]> Date: Sat, 24 Oct 2026 17:59:38 +0400 Subject: [PATCH 2/2] [analyzer] PlacementSelfCopy: track the old object during construction Report any access to the target storage through the old object while the new object is being constructed. Such values are unspecified by [class.cdtor]/2 whatever order the constructor copies in, so a memberwise copy is reported as well. Accesses through `this` are allowed. The checker follows the old object through the parameters, local and global variables bound to it, `this` of the methods called on it, and pointers derived from it by arithmetic, std::move, std::addressof, conditional operators or methods that return a pointer into it. An access in the new-initializer itself is reported too. A call that is not inlined and gets the old object by reference or as its object argument is reported, unless the argument only contains the target, e.g. a pool with an inline buffer. A lambda that captures the old object is reported when it is created. Other fixes: - the target is limited to the placed object and is taken from the placement argument, so an operator new declared without a body works too; - a region of unknown size overlaps another one only if one lies within the other, and a symbolic region gets its size from the pointee type; - base subobjects keep their own extent, and empty records are skipped; - remove two redundant dyn_cast_if_present calls. Add tests with CRTP, templates and an operator new without a body. --- clang/docs/analyzer/checkers.md | 58 +- .../Checkers/PlacementSelfCopyChecker.cpp | 565 +++++++++++------- clang/test/Analysis/placement-self-copy.cpp | 565 +++++++++++++++--- 3 files changed, 861 insertions(+), 327 deletions(-) diff --git a/clang/docs/analyzer/checkers.md b/clang/docs/analyzer/checkers.md index 319b368ed7fa82..b139470f47cb74 100644 --- a/clang/docs/analyzer/checkers.md +++ b/clang/docs/analyzer/checkers.md @@ -3507,41 +3507,69 @@ void bad_insert3(std::vector &v1, std::vector &v2) { #### alpha.cplusplus.PlacementSelfCopy (C++) -Check for placement new expressions that initialize the new object from the -storage it is placed into, e.g. `::new (p) T(*p)`. Construction overwrites that -storage, so a read of a part that has already been written sees the new value -of the object being created. - -If the constructor is inlined, the checker follows its writes to the storage -and reports the first read of an overwritten part. Reads through `this` are -allowed. The elements of an initializer list are treated as written in order. -A copy or move constructor that is not inlined, such as a trivial copy or a -constructor without a visible definition, is reported whenever its source is -the storage itself. Calls of `std::construct_at` are reported at the call site. +Check for placement new expressions that access the storage they are placed +into through the old object while the new object is being constructed, e.g. +`::new (p) T(*p)`. A value read during the construction through a glvalue not +obtained from `this` is unspecified ([class.cdtor]/2); writes through such a +glvalue are reported as well. Calls of +`std::construct_at` are checked as well; a report that would end inside the +standard library is emitted at the call site. + +The checker follows the reference or pointer that received the old storage +through variables, pointer arithmetic, the functions it is passed to and the +methods called on it, and reports the first of these operations on it: + +- a read or a write of the storage; +- passing it to a function or calling its method that is not inlined, except + for calls that only return their argument, such as `std::move`, + `std::forward`, `std::as_const` and `std::addressof`; +- capturing it in a lambda, unless a pointer to it is captured. + +Taking, storing, comparing and shifting its address, accessing members without +reading them and calling inlined methods that read nothing are allowed, and so +are all accesses through `this` or its copies. An object that only contains the +storage, such as a pool with an inline buffer, is not the old object; only its +accesses that overlap the storage are reported. ```cpp struct Buffer { int *Data; int Size; - Buffer(const Buffer &O) : Data(new int[O.Size]), Size(O.Size) { + Buffer(const Buffer &O) + : Data(new int[O.Size]), // warn: placement new accesses the storage it + // is placed into during construction of + // the new object + Size(O.Size) { for (int I = 0; I < Size; ++I) - Data[I] = O.Data[I]; // warn: placement new reads a part of its storage - // that it has already overwritten + Data[I] = O.Data[I]; } }; void reconstruct(Buffer *B) { ::new (B) Buffer(*B); } +``` + +An access of the old object in the new-initializer itself is reported as well, +whatever the order of the elements. +```cpp struct Point { int X, Y; }; void swap_coordinates(Point *P) { - ::new (P) Point{P->Y, P->X}; // warn: P->X is read after X is initialized + ::new (P) Point{P->Y, P->X}; // warn: placement new accesses the storage it + // is placed into during construction of + // the new object } +``` + +When placement new itself calls a copy or move constructor that is not inlined, +such as a trivial copy or a constructor without a visible definition, and its +source is in the storage, it is reported as a self-copy. +```cpp struct Trivial { int X; }; diff --git a/clang/lib/StaticAnalyzer/Checkers/PlacementSelfCopyChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PlacementSelfCopyChecker.cpp index 5b06836018ae13..cc0da471642f3d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/PlacementSelfCopyChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/PlacementSelfCopyChecker.cpp @@ -6,15 +6,17 @@ // //===----------------------------------------------------------------------===// // -// Defines a checker that finds placement new expressions whose initializer -// reads a part of the target storage that has already been overwritten, e.g. -// `::new (p) T(*p)`. A copy or move constructor that is not inlined is -// reported whenever its source is the target storage. +// Defines a checker that finds placement new expressions that access the +// target storage through the old object while the new object is being +// constructed, e.g. `::new (p) T(*p)`. A call that is not inlined is assumed +// to access the old object passed to it; when it is the copy or move +// constructor called by placement new itself, it is reported as a self-copy. // //===----------------------------------------------------------------------===// #include "clang/AST/ExprCXX.h" #include "clang/AST/ParentMap.h" +#include "clang/Basic/Builtins.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" @@ -28,26 +30,26 @@ using namespace ento; using PlacementKey = std::pair<const CXXNewExpr *, const StackFrame *>; REGISTER_MAP_WITH_PROGRAMSTATE(PlacementTargets, PlacementKey, const MemRegion *) -REGISTER_SET_WITH_PROGRAMSTATE(SelfSourcedTargets, const MemRegion *) -REGISTER_SET_WITH_PROGRAMSTATE(OverwrittenParts, const MemRegion *) +// Variables that refer to the old storage, mapped to the placement target. +REGISTER_MAP_WITH_PROGRAMSTATE(SourceVars, const MemRegion *, const MemRegion *) +// Frames of methods called on the old storage, mapped to the placement target. +REGISTER_MAP_WITH_PROGRAMSTATE(SourceFrames, const StackFrame *, + const MemRegion *) REGISTER_TRAIT_WITH_PROGRAMSTATE(PendingSelfCopy, const MemRegion *) -REGISTER_TRAIT_WITH_PROGRAMSTATE(ReadOverwritten, const MemRegion *) namespace { class PlacementSelfCopyChecker : public Checker<check::PostCall, check::PreCall, check::Bind, - check::Location, check::PostStmt<CXXNewExpr>> { + check::Location, check::PostStmt<LambdaExpr>, + check::PostStmt<CXXNewExpr>> { const BugType BT{this, "Placement new initialized from its own storage", categories::MemoryError}; - const BugType ReadBT{this, "Placement new reads overwritten storage", - categories::MemoryError}; + const BugType AccessBT{this, "Placement new accesses its own storage", + categories::MemoryError}; - void reportRead(const Expr *E, const MemRegion *Overwritten, - CheckerContext &C) const; + void reportAccess(const Expr *E, CheckerContext &C) const; void reportSelfCopy(const Expr *Arg, CheckerContext &C) const; void reportPendingSelfCopy(const CallEvent &Call, CheckerContext &C) const; - void checkOverwrittenRead(const Stmt *S, const Expr *E, - const MemRegion *Source, CheckerContext &C) const; public: void checkPostCall(const CallEvent &Call, CheckerContext &C) const; @@ -56,6 +58,7 @@ class PlacementSelfCopyChecker CheckerContext &C) const; void checkLocation(SVal Loc, bool IsLoad, const Stmt *S, CheckerContext &C) const; + void checkPostStmt(const LambdaExpr *LE, CheckerContext &C) const; void checkPostStmt(const CXXNewExpr *NE, CheckerContext &C) const; }; } // namespace @@ -68,191 +71,270 @@ struct Extent { }; } // namespace +// Base class subobjects keep their own extent. +static const MemRegion *stripCasts(const MemRegion *R) { + return R->StripCasts(/*StripBaseAndDerivedCasts=*/false); +} + static std::optional<Extent> getExtent(const MemRegion *R, ASTContext &Ctx) { RegionOffset Offset = R->getAsOffset(); if (!Offset.isValid() || Offset.hasSymbolicOffset()) return std::nullopt; Extent E{Offset.getRegion(), Offset.getOffset(), std::nullopt}; - if (const auto *TR = dyn_cast<TypedValueRegion>(R)) { - QualType Ty = TR->getValueType(); - if (!Ty->isIncompleteType()) - E.End = E.Begin + Ctx.getTypeSize(Ty); - } + QualType Ty; + if (const auto *TR = dyn_cast<TypedValueRegion>(R)) + Ty = TR->getValueType(); + else if (const auto *SR = dyn_cast<SymbolicRegion>(R)) + Ty = SR->getPointeeStaticType(); + if (!Ty.isNull() && !Ty->isIncompleteType()) + E.End = E.Begin + Ctx.getTypeSize(Ty); return E; } -static bool overlaps(const MemRegion *A, const MemRegion *B, ASTContext &Ctx) { - if (A == B || A->isSubRegionOf(B) || B->isSubRegionOf(A)) - return true; +// Returns the extents of A and B if both are known within the same base. +static std::optional<std::pair<Extent, Extent>> +getKnownExtents(const MemRegion *A, const MemRegion *B, ASTContext &Ctx) { std::optional<Extent> EA = getExtent(A, Ctx), EB = getExtent(B, Ctx); - if (!EA || !EB || EA->Base != EB->Base) - return false; - return (!EB->End || EA->Begin < *EB->End) && - (!EA->End || EB->Begin < *EA->End); + if (!EA || !EB || !EA->End || !EB->End || EA->Base != EB->Base) + return std::nullopt; + return std::make_pair(*EA, *EB); +} + +static bool overlaps(const MemRegion *A, const MemRegion *B, ASTContext &Ctx) { + if (auto E = getKnownExtents(A, B, Ctx)) + return E->first.Begin < *E->second.End && E->second.Begin < *E->first.End; + A = stripCasts(A); + return A == stripCasts(B) || A->isSubRegionOf(B) || B->isSubRegionOf(A); } static bool isWithin(const MemRegion *R, const MemRegion *Container, ASTContext &Ctx) { - if (R == Container || R->isSubRegionOf(Container)) + if (auto E = getKnownExtents(R, Container, Ctx)) + return E->second.Begin <= E->first.Begin && *E->first.End <= *E->second.End; + R = stripCasts(R); + return R == stripCasts(Container) || R->isSubRegionOf(Container); +} + +// Limits R to the placed object instead of the whole buffer it lives in. +static const MemRegion *getPlacedRegion(const CXXNewExpr *NE, + const MemRegion *R, CheckerContext &C) { + ASTContext &Ctx = C.getASTContext(); + QualType Ty = NE->getAllocatedType(); + if (NE->isArray()) { + std::optional<const Expr *> Size = NE->getArraySize(); + std::optional<llvm::APSInt> N = + Size && *Size ? (*Size)->getIntegerConstantExpr(Ctx) : std::nullopt; + if (!N) + return R; + Ty = + Ctx.getConstantArrayType(Ty, *N, nullptr, ArraySizeModifier::Normal, 0); + } + const auto *SR = dyn_cast<SubRegion>(R); + const auto *TR = dyn_cast<TypedValueRegion>(R); + if (!SR || (TR && Ctx.hasSameUnqualifiedType(TR->getValueType(), Ty))) + return R; + return C.getStoreManager().getRegionManager().getElementRegion( + Ty, C.getSValBuilder().makeArrayIndex(0), SR, Ctx); +} + +// Calls that return their argument or its address without accessing it, e.g. +// `std::move(X)`, `std::addressof(X)` or `&X` with an overloaded `operator&`. +static bool isPassThroughCall(const CallExpr *CE) { + if (!CE || CE->getNumArgs() != 1) + return false; + if (const auto *OCE = dyn_cast<CXXOperatorCallExpr>(CE)) + return OCE->getOperator() == OO_Amp; + const FunctionDecl *FD = CE->getDirectCallee(); + switch (FD ? FD->getBuiltinID() : 0) { + case Builtin::BIaddressof: + case Builtin::BI__addressof: + case Builtin::BI__builtin_addressof: + case Builtin::BIas_const: + case Builtin::BIforward: + case Builtin::BIforward_like: + case Builtin::BImove: + case Builtin::BImove_if_noexcept: return true; - std::optional<Extent> ER = getExtent(R, Ctx), EC = getExtent(Container, Ctx); - if (!ER || !EC || ER->Base != EC->Base || ER->Begin < EC->Begin) + default: return false; - return !EC->End || (ER->End && *ER->End <= *EC->End); + } } -static bool isThisBased(const Expr *E) { +// Collects the variables that E accesses memory through, e.g. `O` for +// `O->X[1]`, `O.A + 1` or `O.data()`, and both `A` and `B` for +// `(C ? A : B).X`. +static void collectBaseVars(const Expr *E, + SmallVectorImpl<const VarDecl *> &Vars, + bool &ViaThis) { while (E) { E = E->IgnoreParenCasts(); - if (isa<CXXThisExpr>(E)) - return true; - if (const auto *ME = dyn_cast<MemberExpr>(E)) + if (const auto *DRE = dyn_cast<DeclRefExpr>(E)) { + if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) + Vars.push_back(VD); + return; + } + if (isa<CXXThisExpr>(E)) { + ViaThis = true; + return; + } + if (const auto *CO = dyn_cast<AbstractConditionalOperator>(E)) { + collectBaseVars(CO->getTrueExpr(), Vars, ViaThis); + E = CO->getFalseExpr(); + } else if (const auto *ME = dyn_cast<MemberExpr>(E)) E = ME->getBase(); else if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(E)) E = ASE->getBase(); else if (const auto *BO = dyn_cast<BinaryOperator>(E); BO && BO->isAssignmentOp()) E = BO->getLHS(); + else if (BO && BO->isAdditiveOp()) + E = BO->getLHS()->getType()->isPointerType() ? BO->getLHS() + : BO->getRHS(); else if (const auto *UO = dyn_cast<UnaryOperator>(E); - UO && UO->getOpcode() == UO_Deref) + UO && + (UO->getOpcode() == UO_Deref || UO->getOpcode() == UO_AddrOf || + UO->isIncrementDecrementOp())) E = UO->getSubExpr(); + else if (const auto *CE = dyn_cast<CallExpr>(E); isPassThroughCall(CE)) + E = CE->getArg(0); + else if (const auto *MCE = dyn_cast<CXXMemberCallExpr>(E)) + E = MCE->getImplicitObjectArgument(); else - return false; + return; } +} + +static bool isEmptyRecord(const MemRegion *R) { + const auto *TR = dyn_cast<TypedValueRegion>(R); + const CXXRecordDecl *RD = + TR ? TR->getValueType()->getAsCXXRecordDecl() : nullptr; + return RD && RD->hasDefinition() && RD->isEmpty(); +} + +static bool isInInitializer(const CXXNewExpr *NE, const Stmt *S, + CheckerContext &C) { + const Expr *Init = NE->getInitializer(); + const ParentMap &PM = C.getCurrentAnalysisDeclContext()->getParentMap(); + for (const Stmt *P = S; P && P != NE; P = PM.getParent(P)) + if (P == Init) + return true; return false; } -static const MemRegion *findSelfSourcedTarget(ProgramStateRef State, - const MemRegion *R, +// Returns the target whose new-initializer S uses Source in. +static const MemRegion *findInitializedTarget(const Stmt *S, + const MemRegion *Source, CheckerContext &C) { - for (const MemRegion *Target : State->get<SelfSourcedTargets>()) - if (isWithin(R, Target, C.getASTContext())) + if (isEmptyRecord(Source)) + return nullptr; + for (const auto &[Key, Target] : C.getState()->get<PlacementTargets>()) + if (Key.second == C.getStackFrame() && isInInitializer(Key.first, S, C) && + overlaps(Source, Target, C.getASTContext())) return Target; return nullptr; } -static const MemRegion *findOverwritten(ProgramStateRef State, - const MemRegion *R, CheckerContext &C) { - for (const MemRegion *W : State->get<OverwrittenParts>()) - if (overlaps(R, W, C.getASTContext())) - return W; +// Captured variables and `this` in a lambda body belong to the frame of the +// function that defines the lambda. Only pointers and `this` get here: other +// captures of the old object are reported when the lambda is created. +static const StackFrame *getOwningFrame(const VarDecl *VD, + const StackFrame *Frame) { + while (Frame) { + const auto *MD = dyn_cast_if_present<CXXMethodDecl>(Frame->getDecl()); + if (!MD || !MD->getParent()->isLambda() || + (VD && VD->getDeclContext() == MD)) + return Frame; + const auto *Outer = dyn_cast<Decl>(MD->getParent()->getDeclContext()); + do + Frame = Frame->getParent(); + while (Frame && Frame->getDecl() != Outer); + } return nullptr; } -// CSA binds an init list only after evaluating all elements; recompute order. -static const MemRegion *walkInitList(PlacementKey Key, const MemRegion *Target, - const Stmt *S, const MemRegion *Source, - const MemRegion *&Before, - CheckerContext &C) { - Before = nullptr; - const auto &[NE, Frame] = Key; - if (Frame != C.getStackFrame()) +// Returns the target that E accesses through a variable that refers to it, +// or through `this` of a method called on the old storage. +static const MemRegion *findSourceTarget(const Expr *E, const MemRegion *Source, + CheckerContext &C) { + if (isEmptyRecord(Source)) return nullptr; - const auto *Init = dyn_cast_if_present<InitListExpr>(NE->getInitializer()); - if (!Init) - return nullptr; - - const ParentMap &PM = C.getCurrentAnalysisDeclContext()->getParentMap(); - SmallVector<std::pair<const InitListExpr *, unsigned>, 4> Path; - for (const Stmt *Child = S, *Parent = PM.getParent(S); Parent; - Child = Parent, Parent = PM.getParent(Parent)) { - if (const auto *ILE = dyn_cast<InitListExpr>(Parent)) { - ArrayRef<Expr *> Inits = ILE->inits(); - const auto *It = llvm::find(Inits, Child); - if (It == Inits.end()) - return nullptr; - Path.emplace_back(ILE, It - Inits.begin()); - } - if (Parent == Init) - break; + SmallVector<const VarDecl *, 2> Vars; + bool ViaThis = false; + collectBaseVars(E, Vars, ViaThis); + ProgramStateRef State = C.getState(); + auto Matches = [&](const MemRegion *const *Target) { + return Target && overlaps(Source, *Target, C.getASTContext()); + }; + for (const VarDecl *VD : Vars) { + const StackFrame *Owner = getOwningFrame(VD, C.getStackFrame()); + const MemRegion *Var = + Owner ? State->getLValue(VD, Owner).getAsRegion() : nullptr; + if (const MemRegion *const *T = Var ? State->get<SourceVars>(Var) : nullptr; + Matches(T)) + return *T; } - if (Path.empty() || Path.back().first != Init) - return nullptr; - - ASTContext &Ctx = C.getASTContext(); - MemRegionManager &MRM = C.getStoreManager().getRegionManager(); - SValBuilder &SVB = C.getSValBuilder(); - const auto *R = cast<SubRegion>(Target); - for (const auto &[ILE, Index] : llvm::reverse(Path)) { - QualType Ty = ILE->getType(); - if (const auto *AT = Ctx.getAsArrayType(Ty)) { - QualType ElemTy = AT->getElementType(); - for (unsigned I = 0; I != Index && Source && !Before; ++I) { - const MemRegion *Elem = - MRM.getElementRegion(ElemTy, SVB.makeArrayIndex(I), R, Ctx); - if (overlaps(Source, Elem, Ctx)) - Before = Elem; - } - R = MRM.getElementRegion(ElemTy, SVB.makeArrayIndex(Index), R, Ctx); - continue; - } + const StackFrame *Owner = + ViaThis ? getOwningFrame(nullptr, C.getStackFrame()) : nullptr; + if (const MemRegion *const *T = + Owner ? State->get<SourceFrames>(Owner) : nullptr; + Matches(T)) + return *T; + return nullptr; +} - const auto *RD = Ty->getAsCXXRecordDecl(); - if (!RD || RD->isUnion() || RD->getNumBases()) - return nullptr; - unsigned I = 0; - const FieldRegion *Next = nullptr; - for (const FieldDecl *FD : RD->fields()) { - if (FD->isUnnamedBitField()) - continue; - const FieldRegion *Field = MRM.getFieldRegion(FD, R); - if (I++ == Index) { - Next = Field; - break; - } - if (Source && !Before && overlaps(Source, Field, Ctx)) - Before = Field; - } - if (!Next) - return nullptr; - R = Next; - } - return R; +// Returns the target that E refers to in call S, either through the old +// storage or directly in the new-initializer. +static const MemRegion *findTarget(const Expr *E, const MemRegion *Source, + const Stmt *S, CheckerContext &C) { + if (const MemRegion *Target = findSourceTarget(E, Source, C)) + return Target; + return findInitializedTarget(S, Source, C); } -static const MemRegion *getDestination(PlacementKey Key, - const MemRegion *Target, - const CXXConstructorCall &Ctor, - const MemRegion *This, - CheckerContext &C) { - const Stmt *S = Ctor.getOriginExpr(); - if (Key.first->getConstructExpr() == S) - return Key.second == C.getStackFrame() ? This : nullptr; - const MemRegion *Before; - return walkInitList(Key, Target, S, nullptr, Before, C); +// Returns whether E refers to a part of the old storage, not to an object that +// merely contains it, such as a pool with an inline buffer. +static bool refersToOldObject(const Expr *E, const MemRegion *Source, + const Stmt *S, CheckerContext &C) { + const MemRegion *Target = findTarget(E, Source, S, C); + if (!Target || !isWithin(Source, Target, C.getASTContext())) + return false; + // An enclosing object of the same size, e.g. `O->Inner` for a target `Inner`. + return stripCasts(Target) == stripCasts(Source) || + !Target->isSubRegionOf(Source); } -void PlacementSelfCopyChecker::checkOverwrittenRead(const Stmt *S, - const Expr *E, - const MemRegion *Source, - CheckerContext &C) const { - if (isThisBased(E)) - return; - ProgramStateRef State = C.getState(); - Source = Source->StripCasts(); - const MemRegion *Overwritten = findSelfSourcedTarget(State, Source, C) - ? findOverwritten(State, Source, C) - : nullptr; - for (const auto &[Key, Target] : State->get<PlacementTargets>()) { - if (Overwritten) - break; - walkInitList(Key, Target, S, Source, Overwritten, C); +// Returns the argument that passes the old object to a call that is not +// inlined. Such a call may access it in any way. +static const Expr *findOpaqueSourceUse(const CallEvent &Call, + CheckerContext &C) { + if (isPassThroughCall(dyn_cast_if_present<CallExpr>(Call.getOriginExpr()))) + return nullptr; + if (const auto *IC = dyn_cast<CXXInstanceCall>(&Call)) { + const Expr *Obj = IC->getCXXThisExpr(); + const MemRegion *Source = IC->getCXXThisVal().getAsRegion(); + if (Obj && Source && + refersToOldObject(Obj, Source, Call.getOriginExpr(), C)) + return Obj; + } + for (unsigned I = 0, E = Call.getNumArgs(); I != E; ++I) { + const MemRegion *Source = Call.getArgSVal(I).getAsRegion(); + const Expr *Arg = Call.getArgExpr(I); + if (Source && Arg && + refersToOldObject(Arg, Source, Call.getOriginExpr(), C)) + return Arg; } - if (Overwritten) - reportRead(E, Overwritten, C); + return nullptr; } -void PlacementSelfCopyChecker::reportRead(const Expr *E, - const MemRegion *Overwritten, - CheckerContext &C) const { - ExplodedNode *N = - C.generateErrorNode(C.getState()->set<ReadOverwritten>(Overwritten)); +void PlacementSelfCopyChecker::reportAccess(const Expr *E, + CheckerContext &C) const { + ExplodedNode *N = C.generateErrorNode(); if (!N) return; auto R = std::make_unique<PathSensitiveBugReport>( - ReadBT, - "Placement new reads a part of its storage that it has already " - "overwritten", + AccessBT, + "Placement new accesses the storage it is placed into during " + "construction of the new object", N); R->addRange(E->getSourceRange()); C.emitReport(std::move(R)); @@ -277,14 +359,14 @@ void PlacementSelfCopyChecker::reportSelfCopy(const Expr *Arg, void PlacementSelfCopyChecker::reportPendingSelfCopy(const CallEvent &Call, CheckerContext &C) const { const MemRegion *Target = C.getState()->get<PendingSelfCopy>(); - const Expr *Origin = dyn_cast_if_present<Expr>(Call.getOriginExpr()); + const Expr *Origin = Call.getOriginExpr(); if (!Target || !Origin || C.getSourceManager().isInSystemHeader(Origin->getBeginLoc())) return; for (unsigned I = 0, E = Call.getNumArgs(); I != E; ++I) { const MemRegion *Arg = Call.getArgSVal(I).getAsRegion(); if (Arg && !Call.getArgExpr(I)->getType()->isPointerType() && - overlaps(Arg->StripCasts(), Target, C.getASTContext())) { + overlaps(Arg, Target, C.getASTContext())) { reportSelfCopy(Call.getArgExpr(I), C); return; } @@ -299,119 +381,138 @@ void PlacementSelfCopyChecker::checkPostCall(const CallEvent &Call, const FunctionDecl *OperatorNew = NE->getOperatorNew(); if (!OperatorNew || !OperatorNew->isReservedGlobalPlacementOperator()) return; - const MemRegion *Target = Call.getReturnValue().getAsRegion(); + // The operator may be declared without a body. + const MemRegion *Target = + Alloc->getArgSVal(Alloc->getNumImplicitArgs()).getAsRegion(); if (!Target) return; - C.addTransition(C.getState()->set<PlacementTargets>({NE, C.getStackFrame()}, - Target->StripCasts())); + Target = getPlacedRegion(NE, stripCasts(Target), C); + C.addTransition( + C.getState()->set<PlacementTargets>({NE, C.getStackFrame()}, Target)); return; } + // Inlined calls are checked per read. const auto *Ctor = dyn_cast<CXXConstructorCall>(&Call); - if (!Ctor) { - reportPendingSelfCopy(Call, C); - return; - } - const auto *CD = dyn_cast_if_present<CXXConstructorDecl>(Ctor->getDecl()); - if (!CD || !CD->isCopyOrMoveConstructor()) - return; - const MemRegion *This = Ctor->getCXXThisVal().getAsRegion(); - const MemRegion *Source = Ctor->getArgSVal(0).getAsRegion(); - if (!This || !Source) - return; - This = This->StripCasts(); - Source = Source->StripCasts(); - - ProgramStateRef State = C.getState(); - const Stmt *S = Ctor->getOriginExpr(); - for (const auto &[Key, Target] : State->get<PlacementTargets>()) { - const MemRegion *Dest = getDestination(Key, Target, *Ctor, This, C); - if (!Dest) - continue; - // Inlined constructors are checked per read unless built in a temporary. - if (C.wasInlined && isWithin(This, Target, C.getASTContext())) + if (!C.wasInlined && !C.getState()->get<PlacementTargets>().isEmpty()) { + const CXXConstructorDecl *CD = Ctor ? Ctor->getDecl() : nullptr; + const MemRegion *Source = CD && CD->isCopyOrMoveConstructor() + ? Ctor->getArgSVal(0).getAsRegion() + : nullptr; + if (const MemRegion *Target = + Source ? findInitializedTarget(Ctor->getOriginExpr(), Source, C) + : nullptr) { + if (C.getSourceManager().isInSystemHeader( + Ctor->getOriginExpr()->getBeginLoc())) + C.addTransition(C.getState()->set<PendingSelfCopy>(Target)); + else + reportSelfCopy(Ctor->getArgExpr(0), C); return; - if (!overlaps(Source, Dest, C.getASTContext())) + } + if (const Expr *E = findOpaqueSourceUse(Call, C)) { + reportAccess(E, C); return; - if (C.getSourceManager().isInSystemHeader(S->getBeginLoc())) - C.addTransition(State->set<PendingSelfCopy>(Target)); - else - reportSelfCopy(Ctor->getArgExpr(0), C); - return; + } } + if (!Ctor) + reportPendingSelfCopy(Call, C); } void PlacementSelfCopyChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { - const auto *Ctor = dyn_cast<CXXConstructorCall>(&Call); - if (!Ctor) - return; ProgramStateRef State = C.getState(); if (State->get<PlacementTargets>().isEmpty()) return; - ArrayRef<ParmVarDecl *> Params = Ctor->parameters(); - unsigned NumArgs = std::min<size_t>(Ctor->getNumArgs(), Params.size()); - for (unsigned I = 0; I != NumArgs; ++I) { - if (!Params[I]->getType()->isReferenceType()) - continue; - if (const MemRegion *Source = Ctor->getArgSVal(I).getAsRegion()) { - checkOverwrittenRead(Ctor->getOriginExpr(), Ctor->getArgExpr(I), Source, - C); - if (C.isDifferent()) - return; - } + if (const auto *IC = dyn_cast<CXXInstanceCall>(&Call)) { + const Expr *Obj = IC->getCXXThisExpr(); + const MemRegion *Source = IC->getCXXThisVal().getAsRegion(); + const MemRegion *Target = + Obj && Source ? findTarget(Obj, Source, Call.getOriginExpr(), C) + : nullptr; + if (const StackFrame *Frame = + Target ? Call.getCalleeStackFrame(C.blockCount()) : nullptr) + State = State->set<SourceFrames>(Frame, Target); } - - const MemRegion *This = Ctor->getCXXThisVal().getAsRegion(); - if (!This) - return; - This = This->StripCasts(); - for (const auto &[Key, Target] : State->get<PlacementTargets>()) { - const MemRegion *Dest = getDestination(Key, Target, *Ctor, This, C); - if (!Dest) + ArrayRef<ParmVarDecl *> Params = Call.parameters(); + unsigned NumArgs = std::min<size_t>(Call.getNumArgs(), Params.size()); + for (unsigned I = 0; I != NumArgs; ++I) { + QualType ParamTy = Params[I]->getType(); + const MemRegion *Source = Call.getArgSVal(I).getAsRegion(); + const Expr *Arg = Call.getArgExpr(I); + if ((!ParamTy->isReferenceType() && !ParamTy->isPointerType()) || !Source || + !Arg) continue; - for (unsigned I = 0; I != NumArgs; ++I) { - const MemRegion *Source = Ctor->getArgSVal(I).getAsRegion(); - if (Params[I]->getType()->isReferenceType() && Source && - overlaps(Source->StripCasts(), Dest, C.getASTContext())) { - C.addTransition(State->add<SelfSourcedTargets>(Target)); - return; - } - } - return; + const MemRegion *Target = findTarget(Arg, Source, Call.getOriginExpr(), C); + if (const ParamVarRegion *Param = + Target ? Call.getParameterLocation(I, C.blockCount()) : nullptr) + State = State->set<SourceVars>(Param, Target); } + C.addTransition(State); } void PlacementSelfCopyChecker::checkBind(SVal Loc, SVal Val, const Stmt *S, bool AtDeclInit, CheckerContext &C) const { - const MemRegion *R = Loc.getAsRegion(); - if (!R) - return; - R = R->StripCasts(); ProgramStateRef State = C.getState(); - if (!findSelfSourcedTarget(State, R, C) || - State->contains<OverwrittenParts>(R)) + const auto *Var = dyn_cast_or_null<VarRegion>(Loc.getAsRegion()); + if (!Var || State->get<PlacementTargets>().isEmpty()) + return; + QualType Ty = Var->getDecl()->getType(); + if (!Ty->isReferenceType() && !Ty->isPointerType()) return; - const NoteTag *Note = - C.getNoteTag([this, R](PathSensitiveBugReport &BR) -> std::string { - if (&BR.getBugType() != &ReadBT || - BR.getErrorNode()->getState()->get<ReadOverwritten>() != R) - return ""; - return "Part of the storage is overwritten here"; - }); - C.addTransition(State->add<OverwrittenParts>(R), Note); + const Expr *Init = nullptr; + if (AtDeclInit) + Init = Var->getDecl()->getInit(); + else if (const auto *BO = dyn_cast_if_present<BinaryOperator>(S); + BO && BO->getOpcode() == BO_Assign) + Init = BO->getRHS(); + else + Init = dyn_cast_if_present<Expr>(S); + const MemRegion *Source = Val.getAsRegion(); + const MemRegion *Target = + Init && Source ? findSourceTarget(Init, Source, C) : nullptr; + if (Target) + C.addTransition(State->set<SourceVars>(Var, Target)); + else if (State->contains<SourceVars>(Var)) + C.addTransition(State->remove<SourceVars>(Var)); } void PlacementSelfCopyChecker::checkLocation(SVal Loc, bool IsLoad, const Stmt *S, CheckerContext &C) const { - if (!IsLoad || C.getState()->get<PlacementTargets>().isEmpty()) + if (C.getState()->get<PlacementTargets>().isEmpty()) return; const auto *E = dyn_cast_if_present<Expr>(S); const MemRegion *R = Loc.getAsRegion(); - if (E && R) - checkOverwrittenRead(S, E, R, C); + if (!E || !R) + return; + if (findTarget(E, R, S, C)) + reportAccess(E, C); +} + +// A lambda that captures the old object reads or modifies it. +void PlacementSelfCopyChecker::checkPostStmt(const LambdaExpr *LE, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + if (State->get<SourceVars>().isEmpty()) + return; + for (const LambdaCapture &Capture : LE->captures()) { + const auto *VD = Capture.capturesVariable() + ? dyn_cast<VarDecl>(Capture.getCapturedVar()) + : nullptr; + if (!VD || VD->getType()->isPointerType()) + continue; + const MemRegion *Var = + State->getLValue(VD, C.getStackFrame()).getAsRegion(); + const MemRegion *const *Target = + Var ? State->get<SourceVars>(Var) : nullptr; + const MemRegion *Source = + Target ? State->getSVal(Var).getAsRegion() : nullptr; + if (Source && isWithin(Source, *Target, C.getASTContext())) { + reportAccess(LE, C); + return; + } + } } void PlacementSelfCopyChecker::checkPostStmt(const CXXNewExpr *NE, @@ -422,10 +523,12 @@ void PlacementSelfCopyChecker::checkPostStmt(const CXXNewExpr *NE, if (!Target) return; State = State->remove<PlacementTargets>(Key); - State = State->remove<SelfSourcedTargets>(*Target); - for (const MemRegion *W : State->get<OverwrittenParts>()) - if (isWithin(W, *Target, C.getASTContext())) - State = State->remove<OverwrittenParts>(W); + for (const auto &[Var, VarTarget] : State->get<SourceVars>()) + if (VarTarget == *Target) + State = State->remove<SourceVars>(Var); + for (const auto &[Frame, FrameTarget] : State->get<SourceFrames>()) + if (FrameTarget == *Target) + State = State->remove<SourceFrames>(Frame); C.addTransition(State); } diff --git a/clang/test/Analysis/placement-self-copy.cpp b/clang/test/Analysis/placement-self-copy.cpp index 826c709e577535..0cc36bbd451d4c 100644 --- a/clang/test/Analysis/placement-self-copy.cpp +++ b/clang/test/Analysis/placement-self-copy.cpp @@ -1,6 +1,26 @@ // RUN: %clang_analyze_cc1 -std=c++20 %s \ // RUN: -analyzer-checker=core,alpha.cplusplus.PlacementSelfCopy \ // RUN: -analyzer-output=text -verify -triple x86_64-unknown-linux-gnu +// RUN: %clang_analyze_cc1 -std=c++20 %s -DDECLARED_PLACEMENT_NEW \ +// RUN: -analyzer-checker=core,alpha.cplusplus.PlacementSelfCopy \ +// RUN: -verify=declared -triple x86_64-unknown-linux-gnu + +#ifdef DECLARED_PLACEMENT_NEW + +// The reserved placement operator has no body, so its return value is not the +// placement argument. +void *operator new(__SIZE_TYPE__, void *P) noexcept; + +struct Swapped { + int X, Y; + Swapped(const Swapped &O) : X(O.Y), Y(O.X) {} // declared-warning{{Placement new accesses the storage it is placed into during construction of the new object}} +}; + +void swapped_copy(Swapped *Dst) { + ::new (Dst) Swapped(*Dst); +} + +#else #include "Inputs/system-header-simulator-cxx.h" @@ -48,6 +68,18 @@ void copy_from_other(NonTrivial *Dst, NonTrivial *Src) { ::new (Dst) NonTrivial(*Src); // no-warning } +void copy_to_previous_element(Trivial *P) { + ::new (P - 1) Trivial(*P); // no-warning +} + +void copy_to_next_element(NonTrivial *P) { + ::new (P + 1) NonTrivial(*P); // no-warning +} + +void copy_to_partially_overlapping_bytes(Trivial *P) { + ::new ((char *)P + 2) Trivial(*P); // expected-warning{{Object constructed by placement new is initialized from the storage it is placed into}} expected-note{{Object constructed by placement new is initialized from the storage it is placed into}} +} + struct Pair { Trivial A; Trivial B; @@ -71,69 +103,59 @@ void regular_new(NonTrivial *Src) { (void)P; } -struct Deep { - int *P; - int N; - Deep(const Deep &O) : P(new int[O.N]), N(O.N) { P[0] = O.P[0]; } // expected-note{{Part of the storage is overwritten here}} expected-warning{{Placement new reads a part of its storage that it has already overwritten}} expected-note{{Placement new reads a part of its storage that it has already overwritten}} -}; - -void deep_copy_reads_overwritten_pointer(Deep *Dst) { - ::new (Dst) Deep(*Dst); // expected-note{{Calling copy constructor for 'Deep'}} -} - -struct Swapped { +struct Memberwise { int X, Y; - Swapped(const Swapped &O) : X(O.Y), Y(O.X) {} // expected-note{{Part of the storage is overwritten here}} expected-warning{{Placement new reads a part of its storage that it has already overwritten}} expected-note{{Placement new reads a part of its storage that it has already overwritten}} + Memberwise(const Memberwise &O) : X(O.X), Y(O.Y) {} // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} }; -void swapped_copy(Swapped *Dst) { - ::new (Dst) Swapped(*Dst); // expected-note{{Calling copy constructor for 'Swapped'}} +void memberwise_copy(Memberwise *Dst) { + ::new (Dst) Memberwise(*Dst); // expected-note{{Calling copy constructor for 'Memberwise'}} } -struct Memberwise { - int X, Y; - Memberwise(const Memberwise &O) : X(O.X), Y(O.Y) {} +struct IgnoresSource { + int Field; + IgnoresSource(IgnoresSource &) { Field = 0; } }; -void memberwise_copy(Memberwise *Dst) { - ::new (Dst) Memberwise(*Dst); // no-warning +void ignores_source(IgnoresSource *Dst) { + ::new (Dst) IgnoresSource(*Dst); // no-warning } struct ReadsOwnField { int X, Y; - ReadsOwnField(const ReadsOwnField &O) : X(O.X), Y(X + 1) {} + ReadsOwnField(const ReadsOwnField &) : X(1), Y(X + 1) {} }; void reads_own_field(ReadsOwnField *Dst) { ::new (Dst) ReadsOwnField(*Dst); // no-warning } -struct IgnoresSource { - int Field; - IgnoresSource(IgnoresSource &) { Field = 0; } +struct ChainedReset { + int *Begin, *End; + ChainedReset(const ChainedReset &) { Begin = End = nullptr; } }; -void ignores_source(IgnoresSource *Dst) { - ::new (Dst) IgnoresSource(*Dst); // no-warning +void chained_assignment_in_self_copy(ChainedReset *Dst) { + ::new (Dst) ChainedReset(*Dst); // no-warning } -struct FromInt { - int Field; - explicit FromInt(int &V) : Field(V + 1) {} +struct Ends { + int Values[2]; + int &get(int I) { return Values[I]; } + Ends(const Ends &) : Values{1, 2} { get(0) += 1; } }; -void reads_before_writing(FromInt *Dst) { - ::new (Dst) FromInt(Dst->Field); // no-warning +void member_call_on_this_in_self_copy(Ends *Dst) { + ::new (Dst) Ends(*Dst); // no-warning } -struct HoldsTrivial { - Trivial T; - int K; - HoldsTrivial(const HoldsTrivial &O) : T(O.T), K(O.K) {} +struct FromInt { + int Field; + explicit FromInt(int &V) : Field(V + 1) {} // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} }; -void member_copied_before_overwrite(HoldsTrivial *Dst) { - ::new (Dst) HoldsTrivial(*Dst); // no-warning +void reads_member_by_reference(FromInt *Dst) { + ::new (Dst) FromInt(Dst->Field); // expected-note{{Calling constructor for 'FromInt'}} } void construct_at_trivial(Trivial *Dst) { @@ -142,7 +164,7 @@ void construct_at_trivial(Trivial *Dst) { struct SwappedInStdlib { int X, Y; - SwappedInStdlib(const SwappedInStdlib &O) : X(O.Y), Y(O.X) {} // expected-note{{Part of the storage is overwritten here}} expected-warning{{Placement new reads a part of its storage that it has already overwritten}} expected-note{{Placement new reads a part of its storage that it has already overwritten}} + SwappedInStdlib(const SwappedInStdlib &O) : X(O.Y), Y(O.X) {} // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} }; void construct_at_swapped(SwappedInStdlib *Dst) { @@ -158,12 +180,8 @@ void array_trivial(Trivial *Dst) { ::new (Dst) Trivial[1]{*Dst}; // expected-warning{{Object constructed by placement new is initialized from the storage it is placed into}} expected-note{{Object constructed by placement new is initialized from the storage it is placed into}} } -void array_deep(Deep *Dst) { - ::new (Dst) Deep[1]{*Dst}; // expected-warning{{Object constructed by placement new is initialized from the storage it is placed into}} expected-note{{Object constructed by placement new is initialized from the storage it is placed into}} -} - void array_second_element(Trivial *Dst) { - ::new (Dst) Trivial[2]{Dst[1], Dst[0]}; // expected-warning{{Placement new reads a part of its storage that it has already overwritten}} expected-note{{Placement new reads a part of its storage that it has already overwritten}} + ::new (Dst) Trivial[2]{Dst[1], Dst[0]}; // expected-warning{{Object constructed by placement new is initialized from the storage it is placed into}} expected-note{{Object constructed by placement new is initialized from the storage it is placed into}} } void array_other(Trivial *Dst, Trivial *Src) { @@ -175,12 +193,8 @@ struct Agg { int Y; }; -void aggregate_swapped(Agg *Dst) { - ::new (Dst) Agg{Dst->Y, Dst->X}; // expected-warning{{Placement new reads a part of its storage that it has already overwritten}} expected-note{{Placement new reads a part of its storage that it has already overwritten}} -} - void aggregate_same_order(Agg *Dst) { - ::new (Dst) Agg{Dst->X, Dst->Y}; // no-warning + ::new (Dst) Agg{Dst->X, Dst->Y}; // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} } struct Nested { @@ -189,43 +203,13 @@ struct Nested { }; void nested_aggregate(Nested *Dst) { - ::new (Dst) Nested{{Dst->Z, Dst->Inner.X}, 0}; // expected-warning{{Placement new reads a part of its storage that it has already overwritten}} expected-note{{Placement new reads a part of its storage that it has already overwritten}} + ::new (Dst) Nested{{Dst->Z, Dst->Inner.X}, 0}; // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} } void aggregate_copy(Agg *Dst) { ::new (Dst) Agg{*Dst}; // expected-warning{{Object constructed by placement new is initialized from the storage it is placed into}} expected-note{{Object constructed by placement new is initialized from the storage it is placed into}} } -struct ChainedReset { - int *Begin, *End; - ChainedReset(const ChainedReset &) { Begin = End = nullptr; } -}; - -void chained_assignment_in_self_copy(ChainedReset *Dst) { - ::new (Dst) ChainedReset(*Dst); // no-warning -} - -struct Stream { - int *Cur, *End; - explicit Stream(int &Source) { Cur = End = nullptr; (void)Source; } -}; - -void chained_assignment_from_other(Stream *Dst, int &Other) { - ::new (Dst) Stream(Other); // no-warning -} - -struct Ends { - int Values[2]; - int &get(int I) { return Values[I]; } - explicit Ends(const int (&Init)[2]) : Values{Init[0], Init[1]} { - get(0) += 1; - } -}; - -void member_call_from_other(Ends *Dst, const int (&Init)[2]) { - ::new (Dst) Ends(Init); // no-warning -} - struct Recursive; void place(Recursive *Dst, const Recursive &Src, Recursive *Scratch, const Recursive *Other, int Depth); @@ -233,11 +217,11 @@ void place(Recursive *Dst, const Recursive &Src, Recursive *Scratch, struct Recursive { int X, Y; Recursive(const Recursive &O, Recursive *Scratch, const Recursive *Other, - int Depth) - : X(O.Y) { // expected-note{{Part of the storage is overwritten here}} + int Depth) { if (Depth) // expected-note{{'Depth' is 1}} expected-note{{Taking true branch}} place(Scratch, *Other, nullptr, nullptr, 0); - Y = O.X; // expected-warning{{Placement new reads a part of its storage that it has already overwritten}} expected-note{{Placement new reads a part of its storage that it has already overwritten}} + X = O.Y; // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} + Y = O.X; } }; @@ -250,3 +234,422 @@ void same_new_expression_in_nested_frame(Recursive *Dst, Recursive *Scratch, const Recursive *Other) { place(Dst, *Dst, Scratch, Other, 1); // expected-note{{Calling 'place'}} } + +template <typename Derived> struct ReinitMixin { + void reinit(const Derived &Source) { + ::new (static_cast<Derived *>(this)) Derived(Source); // expected-note{{Calling implicit copy constructor for 'WithString'}} + } +}; + +struct WithString : ReinitMixin<WithString> { // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} + int Value; + std::string Name; + WithString(int V, std::string N) : Value(V), Name(N) {} +}; + +void crtp_reinit_from_itself(WithString &Obj) { + Obj.reinit(Obj); // expected-note{{Calling 'ReinitMixin::reinit'}} +} + +template <typename T> struct Box { + T Value; + Box(const Box &O) : Value(O.Value) {} // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} +}; + +void class_template_non_trivial_member(Box<NonTrivial> *Dst) { + ::new (Dst) Box<NonTrivial>(*Dst); // expected-note{{Calling copy constructor for 'Box<NonTrivial>'}} +} + +struct Base1 { + int A; +}; +struct Base2 { + int B; +}; + +struct TwoBases : Base1, Base2 { + int X; + TwoBases(const TwoBases &O) : Base1(O), Base2(O), X(O.X) {} // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} +}; + +void bases_copied_from_itself(TwoBases *Dst) { + ::new (Dst) TwoBases(*Dst); // expected-note{{Calling copy constructor for 'TwoBases'}} +} + +struct Empty {}; + +struct EmptyBase : Empty { + int X; + EmptyBase(const EmptyBase &O) + : Empty(O), + X(O.X) {} // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} +}; + +void empty_base_is_not_read(EmptyBase *Dst) { + ::new (Dst) EmptyBase(*Dst); // expected-note{{Calling copy constructor for 'EmptyBase'}} +} + +struct WritesOther { + int X; + explicit WritesOther(int &Other) { + Other = 1; // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} + X = Other; + } +}; + +void larger_buffer(void *Buf) { + ::new (Buf) WritesOther(*(int *)((char *)Buf + 64)); // no-warning +} + +void aliases_the_object(void *Buf) { + ::new (Buf) WritesOther(*(int *)Buf); // expected-note{{Calling constructor for 'WritesOther'}} +} + +struct ThisAliasHelper; +int getX(const ThisAliasHelper &S); + +struct ThisAliasHelper { + int X, Y; + ThisAliasHelper(const ThisAliasHelper &) { + X = 1; + Y = getX(*this) + 1; + } +}; + +int getX(const ThisAliasHelper &S) { return S.X; } + +void this_passed_to_helper(ThisAliasHelper *Dst) { + ::new (Dst) ThisAliasHelper(*Dst); // no-warning +} + +struct LocalThisAlias { + int X, Y; + LocalThisAlias(const LocalThisAlias &) { + X = 1; + auto *Self = this; + Y = Self->X + 1; + } +}; + +void this_stored_in_local(LocalThisAlias *Dst) { + ::new (Dst) LocalThisAlias(*Dst); // no-warning +} + +struct SourceByPointer { + int X, Y; + explicit SourceByPointer(const SourceByPointer *O) : X(O->Y), Y(O->X) {} // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} +}; + +void source_by_pointer(SourceByPointer *Dst) { + ::new (Dst) SourceByPointer(Dst); // expected-note{{Calling constructor for 'SourceByPointer'}} +} + +struct SourceToHelper; +int getY(const SourceToHelper &S); + +struct SourceToHelper { + int X, Y; + SourceToHelper(const SourceToHelper &O) { + X = 1; + Y = getY(O); // expected-note{{Calling 'getY'}} + } +}; + +int getY(const SourceToHelper &S) { return S.Y; } // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} + +void source_passed_to_helper(SourceToHelper *Dst) { + ::new (Dst) SourceToHelper(*Dst); // expected-note{{Calling copy constructor for 'SourceToHelper'}} +} + +struct SourceInLocal { + int X, Y; + SourceInLocal(const SourceInLocal &O) { + X = 1; + const SourceInLocal *Src = &O; + Y = Src->Y; // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} + } +}; + +void source_stored_in_local(SourceInLocal *Dst) { + ::new (Dst) SourceInLocal(*Dst); // expected-note{{Calling copy constructor for 'SourceInLocal'}} +} + +struct LocalReassigned { + int X, Y; + LocalReassigned(const LocalReassigned &O, const LocalReassigned &Other) { + const LocalReassigned *P = &O; + P = &Other; + X = P->Y; + Y = 0; + } +}; + +void local_reassigned_to_other(LocalReassigned *Dst, LocalReassigned *Other) { + ::new (Dst) LocalReassigned(*Dst, *Other); // no-warning +} + +struct MethodOnSource { + int X, Y; + int getX() const { return X; } // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} + MethodOnSource(const MethodOnSource &O) { + X = 1; + Y = O.getX(); // expected-note{{Calling 'MethodOnSource::getX'}} + } +}; + +void method_called_on_source(MethodOnSource *Dst) { + ::new (Dst) MethodOnSource(*Dst); // expected-note{{Calling copy constructor for 'MethodOnSource'}} +} + +struct LambdaCapture { + int X, Y; + LambdaCapture(const LambdaCapture &O) { + X = 1; + Y = [&] { return O.Y; }(); // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} + } +}; + +void source_captured_by_lambda(LambdaCapture *Dst) { + ::new (Dst) LambdaCapture(*Dst); // expected-note{{Calling copy constructor for 'LambdaCapture'}} +} + +struct LambdaCapturesPointer { + int X; + const LambdaCapturesPointer *Next; + explicit LambdaCapturesPointer(const LambdaCapturesPointer *O) : X(0) { + Next = [O] { return O; }(); + } +}; + +void pointer_captured_by_lambda(LambdaCapturesPointer *Dst) { + ::new (Dst) LambdaCapturesPointer(Dst); // no-warning +} + +struct Conditional { + int X, Y; + Conditional(const Conditional &O, const Conditional &Other, bool B) { + X = 1; + const Conditional &R = B ? O : Other; // expected-note{{'B' is true}} expected-note{{'?' condition is true}} + Y = R.Y; // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} + } +}; + +void source_chosen_by_conditional(Conditional *Dst, Conditional *Other) { + ::new (Dst) Conditional(*Dst, *Other, true); // expected-note{{Calling constructor for 'Conditional'}} +} + +void other_chosen_by_conditional(Conditional *Dst, Conditional *Other) { + ::new (Dst) Conditional(*Dst, *Other, false); // no-warning +} + +struct AddressOfSource { + int X, Y; + AddressOfSource(const AddressOfSource &O) { + X = 1; + const AddressOfSource *P = __builtin_addressof(O); + Y = P->Y; // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} + } +}; + +void address_of_source(AddressOfSource *Dst) { + ::new (Dst) AddressOfSource(*Dst); // expected-note{{Calling copy constructor for 'AddressOfSource'}} +} + +struct OpaqueMethodOnSource { + int X, Y; + int get() const; + OpaqueMethodOnSource(const OpaqueMethodOnSource &O) { + X = 1; + Y = O.get(); // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} + } +}; + +void opaque_method_called_on_source(OpaqueMethodOnSource *Dst) { + ::new (Dst) OpaqueMethodOnSource(*Dst); // expected-note{{Calling copy constructor for 'OpaqueMethodOnSource'}} +} + +struct StoresAddress; +const StoresAddress *Remembered; +void remember(const StoresAddress &R) { Remembered = &R; } + +struct StoresAddress { + int X; + StoresAddress(const StoresAddress &O) : X(0) { remember(O); } +}; + +void callee_stores_address_of_source(StoresAddress *Dst) { + ::new (Dst) StoresAddress(*Dst); // no-warning +} + +struct ReadsStoredAddress; +const ReadsStoredAddress *RememberedToRead; +void rememberToRead(const ReadsStoredAddress &R) { RememberedToRead = &R; } + +struct ReadsStoredAddress { + int X, Y; + ReadsStoredAddress(const ReadsStoredAddress &O) { + X = 1; + rememberToRead(O); + Y = RememberedToRead->X; // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} + } +}; + +void read_through_stored_address(ReadsStoredAddress *Dst) { + ::new (Dst) ReadsStoredAddress(*Dst); // expected-note{{Calling copy constructor for 'ReadsStoredAddress'}} +} + +struct MovedFrom { + int X, Y; + MovedFrom(MovedFrom &&O) : X(O.Y), Y(O.X) {} // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} +}; + +void move_from_itself_with_std_move(MovedFrom *Dst) { + ::new (Dst) MovedFrom(std::move(*Dst)); // expected-note{{Calling move constructor for 'MovedFrom'}} +} + +struct Pool { + int Count; + alignas(8) char Buf[64]; + void note(); + int peek() const { return Buf[0]; } // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} +}; + +struct PoolItem { + int V; + explicit PoolItem(Pool &P) : V(P.Count) { P.note(); } +}; + +PoolItem *pool_method_not_inlined(Pool &P) { + return ::new (P.Buf) PoolItem(P); // no-warning +} + +struct PoolItemLambda { + int V; + explicit PoolItemLambda(Pool &P) : V(0) { + [&] { P.note(); }(); + } +}; + +PoolItemLambda *pool_captured_by_lambda(Pool &P) { + return ::new (P.Buf) PoolItemLambda(P); // no-warning +} + +struct PoolItemPeek { + int V; + explicit PoolItemPeek(Pool &P) : V(P.peek()) {} // expected-note{{Calling 'Pool::peek'}} +}; + +PoolItemPeek *pool_reads_buffer(Pool &P) { + return ::new (P.Buf) PoolItemPeek(P); // expected-note{{Calling constructor for 'PoolItemPeek'}} +} + +struct ArithmeticOnMember { + int A[4]; + int S; + ArithmeticOnMember(const ArithmeticOnMember &O) : S(0) { + const int *P = O.A + 1; + S = *P; // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} + } +}; + +void read_after_pointer_arithmetic(ArithmeticOnMember *Dst) { + ::new (Dst) ArithmeticOnMember(*Dst); // expected-note{{Calling copy constructor for 'ArithmeticOnMember'}} +} + +struct IncrementsSource { + int A[4]; + int S; + IncrementsSource(const IncrementsSource &O) : S(0) { + const int *P = O.A; + ++P; + S = *P; // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} + } +}; + +void read_after_increment(IncrementsSource *Dst) { + ::new (Dst) IncrementsSource(*Dst); // expected-note{{Calling copy constructor for 'IncrementsSource'}} +} + +struct LambdaDereferencesPointer { + int X, Y; + explicit LambdaDereferencesPointer(const LambdaDereferencesPointer *O) + : X(1) { + Y = [O] { return O->Y; }(); // expected-note{{Calling 'operator()'}} + } // expected-warning@-1{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note@-1{{Placement new accesses the storage it is placed into during construction of the new object}} +}; + +void pointer_dereferenced_in_lambda(LambdaDereferencesPointer *Dst) { + ::new (Dst) LambdaDereferencesPointer(Dst); // expected-note{{Calling constructor for 'LambdaDereferencesPointer'}} +} + +extern "C" void *memcpy(void *, const void *, __SIZE_TYPE__); + +struct CopiesBytes { + int X, Y; + CopiesBytes(const CopiesBytes &O) { + memcpy((void *)this, &O, sizeof(CopiesBytes)); // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} + } +}; + +void memcpy_from_source(CopiesBytes *Dst) { + ::new (Dst) CopiesBytes(*Dst); // expected-note{{Calling copy constructor for 'CopiesBytes'}} +} + +struct MemberAddress { + int X; + int *P; + MemberAddress(const MemberAddress &O) : X(1), P(const_cast<int *>(&O.X)) {} +}; + +void address_of_member_is_not_read(MemberAddress *Dst) { + ::new (Dst) MemberAddress(*Dst); // no-warning +} + +struct ShiftsPointer { + ShiftsPointer *Next; + explicit ShiftsPointer(ShiftsPointer *N) : Next(N + 1) {} +}; + +void pointer_arithmetic_is_not_read(ShiftsPointer *Dst) { + ::new (Dst) ShiftsPointer(Dst); // no-warning +} + +struct MethodWithoutRead { + int X; + int answer() const { return 42; } + MethodWithoutRead(const MethodWithoutRead &O) : X(O.answer()) {} +}; + +void method_that_reads_nothing(MethodWithoutRead *Dst) { + ::new (Dst) MethodWithoutRead(*Dst); // no-warning +} + +struct DataOnSource { + int A[4]; + int S; + const int *data() const { return A; } + DataOnSource(const DataOnSource &O) : S(0) { + const int *P = O.data(); + S = P[1]; // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} + } +}; + +void read_through_returned_pointer(DataOnSource *Dst) { + ::new (Dst) DataOnSource(*Dst); // expected-note{{Calling copy constructor for 'DataOnSource'}} +} + +struct ThisInLambda { + int X, Y; + int get() const { + return [this] { return X; }(); // expected-warning{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Placement new accesses the storage it is placed into during construction of the new object}} expected-note{{Calling 'operator()'}} + } + ThisInLambda(const ThisInLambda &O) : X(1) { + Y = O.get(); // expected-note{{Calling 'ThisInLambda::get'}} + } +}; + +void this_captured_in_method_on_source(ThisInLambda *Dst) { + ::new (Dst) ThisInLambda(*Dst); // expected-note{{Calling copy constructor for 'ThisInLambda'}} +} + +#endif _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
