[clang] [alpha.webkit.NoDeleteChecker] Treat a r-value smart pointer argument or return value as no-delete. (PR #200912)
Balázs Benics via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 2 06:09:21 PDT 2026
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/200912
>From 3ec0c08b9c3f6d4376b1e6699019e44730d52a05 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 29 May 2026 11:48:52 -0700
Subject: [PATCH 1/3] [alpha.webkit.NoDeleteChecker] Treat a r-value smart
pointer argument or return value as no-delete.
Skip the checkin g of the destructor of T in ExprWithCleanups / CXXBindTemporaryExpr when
returning or passing in a function argument using r-value reference since such a construct never
invokes delete.
---
.../Checkers/WebKit/PtrTypesSemantics.cpp | 27 +++++-
.../Analysis/Checkers/WebKit/call-args.cpp | 17 ++++
.../Analysis/Checkers/WebKit/mock-types.h | 1 +
.../Checkers/WebKit/nodelete-annotation.cpp | 82 ++++++++++++++++++-
.../WebKit/nodelete-lazy-initialize.cpp | 36 ++++++++
5 files changed, 158 insertions(+), 5 deletions(-)
create mode 100644 clang/test/Analysis/Checkers/WebKit/nodelete-lazy-initialize.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 2ca34ff0587e1..7568eabb7932c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -713,9 +713,8 @@ class TrivialFunctionAnalysisVisitor
}
bool VisitReturnStmt(const ReturnStmt *RS) {
- // A return statement is allowed as long as the return value is trivial.
if (auto *RV = RS->getRetValue())
- return Visit(RV);
+ return VisitIgnoringTempWithoutDestruction(RV);
return true;
}
@@ -895,15 +894,35 @@ class TrivialFunctionAnalysisVisitor
bool checkArguments(const CallExpr *CE) {
for (const Expr *Arg : CE->arguments()) {
- if (Arg && !Visit(Arg))
+ if (Arg && !VisitIgnoringTempWithoutDestruction(Arg))
return false;
}
return true;
}
+ bool VisitIgnoringTempWithoutDestruction(const Expr *Arg) {
+ QualType OriginalQT = Arg->getType();
+ auto *Type = OriginalQT.getTypePtrOrNull();
+ if (!Type)
+ return Visit(Arg);
+ auto *CXXRD = Type->getAsCXXRecordDecl();
+ if (!CXXRD || !isSmartPtrClass(safeGetName(CXXRD)))
+ return Visit(Arg);
+ Arg = Arg->IgnoreParenCasts();
+ if (!Arg->isPRValue())
+ return Visit(Arg);
+ if (auto *ExprWithClean = dyn_cast<ExprWithCleanups>(Arg))
+ Arg = ExprWithClean->getSubExpr()->IgnoreParenCasts();
+ if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(Arg)) {
+ if (OriginalQT == BTE->getType())
+ return Visit(BTE->getSubExpr());
+ }
+ return Visit(Arg);
+ }
+
bool VisitCXXConstructExpr(const CXXConstructExpr *CE) {
for (const Expr *Arg : CE->arguments()) {
- if (Arg && !Visit(Arg))
+ if (Arg && !VisitIgnoringTempWithoutDestruction(Arg))
return false;
}
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args.cpp b/clang/test/Analysis/Checkers/WebKit/call-args.cpp
index f15991134c58a..bcd3a9592d55d 100644
--- a/clang/test/Analysis/Checkers/WebKit/call-args.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/call-args.cpp
@@ -557,4 +557,21 @@ namespace call_with_weak_ptr {
weakPtr->method();
// expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
}
+
+ struct Provider {
+ RefCountableWithWeakPtr* provide();
+ };
+ int intValue();
+
+ struct Container {
+ Container(Provider& provider)
+ : m_weakPtr(provider.provide())
+ , m_value(intValue())
+ { }
+
+ private:
+ WeakPtr<RefCountableWithWeakPtr> m_weakPtr;
+ int m_value;
+ };
+
}
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h
index af63268ac9695..de5c2d35f2408 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -202,6 +202,7 @@ template <typename T> struct RefPtr {
t = o.t;
o.t = tmp;
}
+ operator T*() { return t; }
T *get() const { return t; }
T *operator->() const { return t; }
T &operator*() const { return *t; }
diff --git a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
index e0dfab150e0a4..d93aa0f78bc87 100644
--- a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
@@ -316,10 +316,23 @@ class Derived : public Base<Type> {
};
struct Data {
- static Ref<Data> create() {
+ static Ref<Data> [[clang::annotate_type("webkit.nodelete")]] create() {
return adoptRef(*new Data);
}
+ static Ref<Data> [[clang::annotate_type("webkit.nodelete")]] create(double) {
+ return adoptRef(*new Data(RefCountable::create()->next()));
+ // expected-warning at -1{{A function 'create' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+ }
+
+ static Data* [[clang::annotate_type("webkit.nodelete")]] create(int) {
+ return adoptRef(new Data); // expected-warning{{A function 'create' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+ }
+
+ static Ref<Data> create(const char*) {
+ return std::move(adoptRef(*new Data));
+ }
+
void ref() {
++refCount;
}
@@ -338,6 +351,7 @@ struct Data {
protected:
Data() = default;
+ Data(RefCountable*) { }
private:
unsigned refCount { 0 };
@@ -562,3 +576,69 @@ struct MemberAssignment {
RefPtr<SomeObject> m_someObject;
Vector<Ref<SomeObject>> m_objects;
};
+
+namespace copy_elision_edge_cases {
+
+// These cases all inhibit NRVO/copy elision (so a real move or copy constructor runs into the return slot),
+// but none of them perform any local destruction:
+// - Moved-from operands are emptied; their dtors are no-ops at the caller.
+// - Globals/statics are not destructed here at all.
+// - The return-slot temporary is destructed by the caller, not by us.
+// All the mock Ref<T> copy/move ctors only manipulate pointers and a
+// refcount, so the trivial-ctor analysis correctly classifies these as
+// safe. The tests below document that the checker accepts each shape.
+
+Ref<RefCountable> [[clang::annotate_type("webkit.nodelete")]] returnStdMoved(Ref<RefCountable>&& obj) {
+ return std::move(obj);
+}
+
+Ref<RefCountable> [[clang::annotate_type("webkit.nodelete")]] returnFromBranches(bool b, Ref<RefCountable>&& a, Ref<RefCountable>&& c) {
+ if (b)
+ return std::move(a);
+ return std::move(c);
+}
+
+Ref<RefCountable> [[clang::annotate_type("webkit.nodelete")]] returnDerefedParam(Ref<RefCountable>* param) {
+ return *param;
+}
+
+// Returning a by-value parameter also requires a real copy/move construction.
+// The function is still flagged here because of unsafe-parameter diagnostic that fires for the parameter declaration.
+Ref<RefCountable> [[clang::annotate_type("webkit.nodelete")]] returnByValueParam(Ref<RefCountable> param) {
+ // expected-warning at -1{{A function 'returnByValueParam' has [[clang::annotate_type("webkit.nodelete")]] but it contains a parameter 'param' which could destruct an object}}
+ return param;
+}
+
+extern Ref<RefCountable> g_ref;
+Ref<RefCountable> [[clang::annotate_type("webkit.nodelete")]] returnGlobal() {
+ return g_ref;
+}
+
+struct StaticHolder {
+ static Ref<RefCountable> s_ref;
+};
+Ref<RefCountable> [[clang::annotate_type("webkit.nodelete")]] returnClassStatic() {
+ return StaticHolder::s_ref;
+}
+
+} // namespace copy_elision_edge_cases
+
+namespace temp_object_typecheck {
+
+struct Tracked {
+ Tracked();
+ ~Tracked();
+};
+
+Tracked [[clang::annotate_type("webkit.nodelete")]] makeTracked();
+
+struct Box {
+ Box(const Tracked&) {}
+};
+
+Box [[clang::annotate_type("webkit.nodelete")]] makeBox() {
+ return Box(makeTracked());
+ // expected-warning at -1{{A function 'makeBox' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+}
+
+} // namespace temp_object_typecheck
\ No newline at end of file
diff --git a/clang/test/Analysis/Checkers/WebKit/nodelete-lazy-initialize.cpp b/clang/test/Analysis/Checkers/WebKit/nodelete-lazy-initialize.cpp
new file mode 100644
index 0000000000000..aabe949aae07d
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/nodelete-lazy-initialize.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.NoDeleteChecker -verify %s
+
+#include "mock-types.h"
+
+void crash();
+
+template<typename T, typename U>
+[[clang::suppress]] inline void [[clang::annotate_type("webkit.nodelete")]] lazyInitialize(const RefPtr<T>& ptr, Ref<U>&& obj)
+{
+ if (ptr)
+ crash();
+ const_cast<RefPtr<T>&>(ptr) = WTF::move(obj);
+}
+
+struct RefObj {
+ static Ref<RefObj> [[clang::annotate_type("webkit.nodelete")]] create(int = 0);
+ void ref() const;
+ void deref() const;
+ int value() const;
+};
+
+struct Container {
+
+ void [[clang::annotate_type("webkit.nodelete")]] foo() {
+ if (!m_bar)
+ lazyInitialize(m_bar, RefObj::create());
+ }
+
+ void [[clang::annotate_type("webkit.nodelete")]] bar() {
+ if (!m_bar)
+ lazyInitialize(m_bar, RefObj::create(RefObj::create()->value()));
+ // expected-warning at -1{{A function 'bar' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+ }
+
+ const RefPtr<RefObj> m_bar;
+};
>From 0b2bcb6d62f7d057b332d102ea8b930800fb5a08 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Tue, 2 Jun 2026 12:28:13 +0100
Subject: [PATCH 2/3] [analyzer][WebKit] NoDelete: only elide returned-prvalue
temporaries
The TrivialFunctionAnalysis temporary-destructor skip added for
webkit.nodelete was applied to both return values and call/constructor
arguments. It is only sound for return values: a returned class prvalue
is constructed directly into the caller's return slot (C++17 guaranteed
copy elision), so the temporary is destructed by the caller, not in the
analyzed function.
An argument temporary's lifetime instead ends at the full-expression in
the *caller* (the caller destroys arguments, e.g. per the Itanium C++
ABI), so its destructor runs in the analyzed function and may invoke
delete. Whether it actually does depends on the callee taking ownership,
which the checker does not prove -- so skipping it produced false
negatives (e.g. `sink(make())` was accepted while a discarded `make();`
was flagged, despite both destroying the same temporary at the same
point).
Restrict the skip to return values: revert checkArguments() and
VisitCXXConstructExpr() to the normal Visit(), and rename the helper to
visitReturnValueElidingTemp() with a comment explaining the rationale.
Verified against -O2 codegen (arm64-apple-darwin): a returned prvalue
tail-calls the factory with the sret pointer forwarded and emits no
destructor, whereas every argument form (by value, rvalue reference,
const reference) and a discarded temporary emit a ~T() call in the
analyzed function -- matching the Itanium rule that the caller destroys
arguments.
Tests: nodelete-lazy-initialize.cpp's foo() now correctly warns (its
argument temporary is no longer elided); add a characterization namespace
to nodelete-annotation.cpp pinning the returns-vs-arguments boundary.
Assisted-By: claude
---
.../Checkers/WebKit/PtrTypesSemantics.cpp | 24 ++++++++--
.../Checkers/WebKit/nodelete-annotation.cpp | 45 ++++++++++++++++++-
.../WebKit/nodelete-lazy-initialize.cpp | 5 +++
3 files changed, 69 insertions(+), 5 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 7568eabb7932c..30291f5acca07 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -713,8 +713,12 @@ class TrivialFunctionAnalysisVisitor
}
bool VisitReturnStmt(const ReturnStmt *RS) {
+ // A return statement is allowed as long as the return value is trivial. A
+ // returned smart-pointer prvalue is special: under guaranteed copy elision
+ // the temporary *is* the function's return slot, so it is destructed by the
+ // caller, not here. Hence we may ignore that temporary's destructor.
if (auto *RV = RS->getRetValue())
- return VisitIgnoringTempWithoutDestruction(RV);
+ return visitReturnValueElidingTemp(RV);
return true;
}
@@ -894,13 +898,25 @@ class TrivialFunctionAnalysisVisitor
bool checkArguments(const CallExpr *CE) {
for (const Expr *Arg : CE->arguments()) {
- if (Arg && !VisitIgnoringTempWithoutDestruction(Arg))
+ if (Arg && !Visit(Arg))
return false;
}
return true;
}
- bool VisitIgnoringTempWithoutDestruction(const Expr *Arg) {
+ // Triviality check for a return value that may elide a smart-pointer
+ // temporary's destructor.
+ //
+ // This is only valid for *return values*: a returned class prvalue is
+ // constructed directly into the function's return slot (C++17 guaranteed copy
+ // elision), so the temporary is destructed by the caller rather than here.
+ //
+ // It is deliberately NOT applied to call/constructor arguments. An argument
+ // temporary's lifetime ends at the full-expression *in this function* (the
+ // caller destroys arguments, e.g. per the Itanium C++ ABI), so its destructor
+ // runs here and may invoke delete. Proving otherwise would require
+ // interprocedural ownership analysis, so arguments are checked normally.
+ bool visitReturnValueElidingTemp(const Expr *Arg) {
QualType OriginalQT = Arg->getType();
auto *Type = OriginalQT.getTypePtrOrNull();
if (!Type)
@@ -922,7 +938,7 @@ class TrivialFunctionAnalysisVisitor
bool VisitCXXConstructExpr(const CXXConstructExpr *CE) {
for (const Expr *Arg : CE->arguments()) {
- if (Arg && !VisitIgnoringTempWithoutDestruction(Arg))
+ if (Arg && !Visit(Arg))
return false;
}
diff --git a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
index d93aa0f78bc87..6c0c0820543ce 100644
--- a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
@@ -641,4 +641,47 @@ Box [[clang::annotate_type("webkit.nodelete")]] makeBox() {
// expected-warning at -1{{A function 'makeBox' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
}
-} // namespace temp_object_typecheck
\ No newline at end of file
+} // namespace temp_object_typecheck
+
+namespace argument_temporaries_are_not_elided {
+
+// Only a *returned* prvalue is elided into the caller's return slot. A
+// smart-pointer temporary passed as a call argument is destructed in this
+// function at the end of the full-expression (the caller destroys arguments),
+// so its destructor -- which may run delete -- is correctly flagged, no matter
+// how the callee binds it (by value, by rvalue reference, or by const
+// reference). The factory and sinks are annotated no-delete so the only
+// possible offender is the argument temporary's destruction.
+
+Ref<RefCountable> [[clang::annotate_type("webkit.nodelete")]] makeRef();
+void [[clang::annotate_type("webkit.nodelete")]] sinkByValue(Ref<RefCountable>);
+void [[clang::annotate_type("webkit.nodelete")]] sinkByRvalueRef(Ref<RefCountable>&&);
+void [[clang::annotate_type("webkit.nodelete")]] observeByConstRef(const Ref<RefCountable>&);
+
+// Returned prvalue: constructed into the caller's return slot -> no local
+// destruction here.
+Ref<RefCountable> [[clang::annotate_type("webkit.nodelete")]] returnedPrvalueIsElided() {
+ return makeRef();
+}
+
+void [[clang::annotate_type("webkit.nodelete")]] passedByValueIsFlagged() {
+ sinkByValue(makeRef());
+ // expected-warning at -1{{A function 'passedByValueIsFlagged' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+}
+
+void [[clang::annotate_type("webkit.nodelete")]] passedByRvalueRefIsFlagged() {
+ sinkByRvalueRef(makeRef());
+ // expected-warning at -1{{A function 'passedByRvalueRefIsFlagged' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+}
+
+void [[clang::annotate_type("webkit.nodelete")]] passedByConstRefIsFlagged() {
+ observeByConstRef(makeRef());
+ // expected-warning at -1{{A function 'passedByConstRefIsFlagged' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+}
+
+void [[clang::annotate_type("webkit.nodelete")]] discardedTemporaryIsFlagged() {
+ makeRef();
+ // expected-warning at -1{{A function 'discardedTemporaryIsFlagged' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+}
+
+} // namespace argument_temporaries_are_not_elided
diff --git a/clang/test/Analysis/Checkers/WebKit/nodelete-lazy-initialize.cpp b/clang/test/Analysis/Checkers/WebKit/nodelete-lazy-initialize.cpp
index aabe949aae07d..6d8ddb13e817e 100644
--- a/clang/test/Analysis/Checkers/WebKit/nodelete-lazy-initialize.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/nodelete-lazy-initialize.cpp
@@ -24,6 +24,11 @@ struct Container {
void [[clang::annotate_type("webkit.nodelete")]] foo() {
if (!m_bar)
lazyInitialize(m_bar, RefObj::create());
+ // expected-warning at -1{{A function 'foo' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+ // The 'RefObj::create()' temporary is passed as an argument, so its
+ // lifetime ends at this full-expression in 'foo' (the caller destroys
+ // arguments) and its destructor may run delete. Only returned prvalues
+ // are elided, so this is correctly flagged.
}
void [[clang::annotate_type("webkit.nodelete")]] bar() {
>From d81f8bb03cd56120e1108f4b7b392905413fc9a8 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Tue, 2 Jun 2026 12:30:55 +0100
Subject: [PATCH 3/3] [analyzer][WebKit] NoDelete: compare canonical types when
eliding a returned temporary
visitReturnValueElidingTemp() decided whether the bound temporary is the
returned object using exact QualType equality (OriginalQT == BTE->getType()).
Exact equality is sensitive to sugar (typedefs/aliases) and cv-qualifiers, so
it can silently fail to recognise the temporary even when it is the same
smart-pointer type.
Compare canonical, unqualified types instead, which states the intent
directly. This is defensive hardening: on the return path the return-value
expression and its bound temporary share the same type spelling, so no
existing test distinguishes the two comparisons; the canonical form guards
against future sugar/qualifier drift. A characterization test
(returned_prvalue_typedef) documents that a typedef'd returned prvalue is
elided.
Assisted-By: claude
---
.../Checkers/WebKit/PtrTypesSemantics.cpp | 7 ++++++-
.../Checkers/WebKit/nodelete-annotation.cpp | 16 ++++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 30291f5acca07..d5ed7fc78148a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -930,7 +930,12 @@ class TrivialFunctionAnalysisVisitor
if (auto *ExprWithClean = dyn_cast<ExprWithCleanups>(Arg))
Arg = ExprWithClean->getSubExpr()->IgnoreParenCasts();
if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(Arg)) {
- if (OriginalQT == BTE->getType())
+ // Only elide when the temporary *is* the returned object, i.e. it has the
+ // same smart-pointer type as the return value. Compare canonical,
+ // unqualified types rather than relying on exact QualType identity, which
+ // is sensitive to sugar (typedefs/aliases) and cv-qualifiers.
+ if (OriginalQT.getCanonicalType().getUnqualifiedType() ==
+ BTE->getType().getCanonicalType().getUnqualifiedType())
return Visit(BTE->getSubExpr());
}
return Visit(Arg);
diff --git a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
index 6c0c0820543ce..a9c50cfb1f45f 100644
--- a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
@@ -685,3 +685,19 @@ void [[clang::annotate_type("webkit.nodelete")]] discardedTemporaryIsFlagged() {
}
} // namespace argument_temporaries_are_not_elided
+
+namespace returned_prvalue_typedef {
+
+// A returned prvalue spelled through a typedef/alias is still the return-slot
+// object and must be elided. The elision relies on a canonical, unqualified
+// type comparison rather than exact QualType identity.
+
+using RefRC = Ref<RefCountable>;
+RefRC [[clang::annotate_type("webkit.nodelete")]] makeAlias();
+
+Ref<RefCountable> [[clang::annotate_type("webkit.nodelete")]] returnTypedefPrvalue() {
+ return makeAlias(); // no warning: the elided temporary is the return slot.
+}
+
+} // namespace returned_prvalue_typedef
+
More information about the cfe-commits
mailing list