Skip to content

Commit 846bfd7

Browse files
authored
[LifetimeSafety] Avoid assert on variadic placement new (llvm#199588)
Avoid assuming that a placement allocation function has a second `ParmVarDecl` before checking whether that parameter is `void*`. Variadic `operator new(size_t, ...)` can have a placement argument matched by the ellipsis instead. As of AI Usage: Codex is used to help rephrase part of the new comments. Closes llvm#199584
1 parent 5acb952 commit 846bfd7

3 files changed

Lines changed: 50 additions & 27 deletions

File tree

clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> {
7676

7777
void handlePointerArithmetic(const BinaryOperator *BO);
7878

79+
void handlePlacementNew(const CXXNewExpr *NE, OriginList *NewList);
80+
7981
void handleCXXCtorInitializer(const CXXCtorInitializer *CII);
8082

8183
void handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds);

clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp

Lines changed: 37 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -631,38 +631,48 @@ void FactsGenerator::VisitArraySubscriptExpr(const ArraySubscriptExpr *ASE) {
631631
Dst->getOuterOriginID(), Src->getOuterOriginID(), /*Kill=*/true));
632632
}
633633

634+
void FactsGenerator::handlePlacementNew(const CXXNewExpr *NE,
635+
OriginList *NewList) {
636+
// Model only the standard single-argument placement new form, where the
637+
// placement argument corresponds to a void* allocation-function parameter.
638+
// Other placement forms, such as std::nothrow, are not modeled as providing
639+
// storage for the returned pointer.
640+
if (NE->getNumPlacementArgs() != 1)
641+
return;
642+
643+
const FunctionDecl *OperatorNew = NE->getOperatorNew();
644+
if (OperatorNew->getNumParams() <= 1)
645+
return;
646+
647+
const auto *Arg =
648+
OperatorNew->getParamDecl(1)->getType()->getAs<PointerType>();
649+
if (!Arg || !Arg->isVoidPointerType())
650+
return;
651+
652+
// Use the placement argument before the implicit conversion to void*, so
653+
// inner origins are still available.
654+
const Expr *PlacementArg = NE->getPlacementArg(0);
655+
if (const auto *ICE = dyn_cast<ImplicitCastExpr>(PlacementArg);
656+
ICE && ICE->getCastKind() == CK_BitCast &&
657+
PlacementArg->getType()->isVoidPointerType())
658+
PlacementArg = ICE->getSubExpr();
659+
OriginList *PlacementList = getOriginsList(*PlacementArg);
660+
// FIXME: General placement arguments need separate handling to overwrite
661+
// the right origins.
662+
663+
// The pointer returned by placement new comes from the placement
664+
// argument.
665+
if (PlacementList)
666+
CurrentBlockFacts.push_back(FactMgr.createFact<OriginFlowFact>(
667+
NewList->getOuterOriginID(), PlacementList->getOuterOriginID(), true));
668+
}
669+
634670
void FactsGenerator::VisitCXXNewExpr(const CXXNewExpr *NE) {
635671
OriginList *NewList = getOriginsList(*NE);
636672
const Expr *Init = NE->getInitializer();
637673

638-
// Check if we have a placement new where the second argument is void*, to
639-
// avoid flowing from non-pointer parameters, such as std::nothrow.
640-
// And that the placement parameter num is 1,
641-
// that is to mostly limit to standard library placement new.
642674
if (NE->getNumPlacementArgs() == 1) {
643-
if (const auto *Arg = NE->getOperatorNew()
644-
->getParamDecl(1)
645-
->getType()
646-
->getAs<PointerType>();
647-
Arg && Arg->isVoidPointerType()) {
648-
// Use the placement argument before the implicit conversion to void*, so
649-
// inner origins are still available.
650-
const Expr *PlacementArg = NE->getPlacementArg(0);
651-
if (const auto *ICE = dyn_cast<ImplicitCastExpr>(PlacementArg);
652-
ICE && ICE->getCastKind() == CK_BitCast &&
653-
PlacementArg->getType()->isVoidPointerType())
654-
PlacementArg = ICE->getSubExpr();
655-
OriginList *PlacementList = getOriginsList(*PlacementArg);
656-
// FIXME: General placement arguments need separate handling to overwrite
657-
// the right origins.
658-
659-
// The pointer returned by placement new comes from the placement
660-
// argument.
661-
if (PlacementList)
662-
CurrentBlockFacts.push_back(FactMgr.createFact<OriginFlowFact>(
663-
NewList->getOuterOriginID(), PlacementList->getOuterOriginID(),
664-
true));
665-
}
675+
handlePlacementNew(NE, NewList);
666676
} else {
667677
const Loan *L = createLoan(FactMgr, NE);
668678
CurrentBlockFacts.push_back(

clang/test/Sema/warn-lifetime-safety.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2982,6 +2982,17 @@ void placement_new_heap_then_delete_use_after_free() {
29822982
(void)*p; // expected-note {{later used here}}
29832983
}
29842984

2985+
struct PlacementArg {};
2986+
2987+
struct VariadicPlacementNew {
2988+
void *operator new(std::size_t, ...);
2989+
};
2990+
2991+
void variadic_placement_new() {
2992+
PlacementArg arg;
2993+
(void)new (arg) VariadicPlacementNew;
2994+
}
2995+
29852996
int* foo(int* x [[clang::lifetimebound]], int* y [[clang::lifetimebound]]);
29862997

29872998
void placement_new_delete_result_of_lifetimebound_call() {

0 commit comments

Comments
 (0)