[clang] [WebKit Checkers] Trace through temporaries in tryToFindPtrOrigin (PR #224877)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Sep 20 14:36:38 PDT 2026
https://github.com/geoffreygaren updated https://github.com/llvm/llvm-project/pull/224877
>From b3a0c3c1634ee373c427d5be33c80b05e1101b4a Mon Sep 17 00:00:00 2001
From: Geoff Garen <ggaren at apple.com>
Date: Tue, 25 Aug 2026 19:29:20 -0700
Subject: [PATCH] [WebKit Checkers] Trace through temporaries in
tryToFindPtrOrigin
RefPtr checking skips temporaries, reporting a path through any temporary
as unsafe. This is mostly correct, but not always. For example, the
following is a false positive:
// makeKey() returns a temporary
RefCountable* p = condition(makeKey()) ? guardian.ptr() : nullptr;
In the upcoming Borrow checker, it's even more important to trace through
temporaries because not tracing an expression can drop a `lifetimebound`
link, resulting in false **negatives**.
This patch adds tracing through temporaries. The logic is:
* In function call arguments, temporaries are lifetime safe because the
full expression does not end until the call returns
* In ranged for loops, temporaries are lifetime safe because lifetime
extends past the full expression to the duration of the loop (C++ P2718)
* Otherwise, temporaries are not lifetime safe
Assisted-by: Claude
---
.../Checkers/WebKit/ASTUtils.cpp | 93 ++++++++++++-------
.../StaticAnalyzer/Checkers/WebKit/ASTUtils.h | 12 ++-
.../WebKit/RawPtrRefCallArgsChecker.cpp | 5 +-
.../WebKit/RawPtrRefLocalVarsChecker.cpp | 8 +-
.../Checkers/WebKit/uncounted-local-vars.cpp | 24 +++++
5 files changed, 103 insertions(+), 39 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index eeb0d8535d8ff..9d9926c96ebeb 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -15,6 +15,7 @@
#include "clang/AST/ExprObjC.h"
#include "clang/AST/StmtVisitor.h"
#include <optional>
+#include <utility>
namespace clang {
@@ -22,24 +23,33 @@ bool isSafePtr(clang::CXXRecordDecl *Decl) {
return isRefCounted(Decl) || isCheckedPtr(Decl);
}
-bool tryToFindPtrOrigin(
+static bool tryToFindPtrOriginImpl(
const Expr *E, bool StopAtFirstRefCountedObj,
std::function<bool(const clang::CXXRecordDecl *)> isSafePtr,
std::function<bool(const clang::QualType)> isSafePtrType,
std::function<bool(const clang::Decl *)> isSafeGlobalDecl,
- std::function<bool(const clang::Expr *, bool)> callback) {
+ std::function<bool(const clang::Expr *, bool /*IsSafe*/,
+ bool /*CrossedShortLivedTemporary*/)>
+ callback,
+ bool CrossedShortLivedTemporary) {
while (E) {
if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
if (auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl())) {
auto QT = VD->getType();
auto IsImmortal = safeGetName(VD) == "NSApp";
if (VD->hasGlobalStorage() && (IsImmortal || QT.isConstQualified()))
- return callback(E, true);
+ return callback(E, /*IsSafe=*/true, CrossedShortLivedTemporary);
if (VD->hasGlobalStorage() && isSafeGlobalDecl(VD))
- return callback(E, true);
+ return callback(E, /*IsSafe=*/true, CrossedShortLivedTemporary);
}
}
+ if (auto *Cleanups = dyn_cast<ExprWithCleanups>(E)) {
+ E = Cleanups->getSubExpr();
+ continue;
+ }
if (auto *tempExpr = dyn_cast<MaterializeTemporaryExpr>(E)) {
+ if (tempExpr->getStorageDuration() == SD_FullExpression)
+ CrossedShortLivedTemporary = true;
E = tempExpr->getSubExpr();
continue;
}
@@ -50,13 +60,13 @@ bool tryToFindPtrOrigin(
if (auto *tempExpr = dyn_cast<CXXConstructExpr>(E)) {
if (auto *C = tempExpr->getConstructor()) {
if (auto *Class = C->getParent(); Class && isSafePtr(Class))
- return callback(E, true);
+ return callback(E, /*IsSafe=*/true, CrossedShortLivedTemporary);
break;
}
}
if (auto *TempExpr = dyn_cast<CXXUnresolvedConstructExpr>(E)) {
if (isSafePtrType(TempExpr->getTypeAsWritten()))
- return callback(TempExpr, true);
+ return callback(TempExpr, /*IsSafe=*/true, CrossedShortLivedTemporary);
}
if (auto *POE = dyn_cast<PseudoObjectExpr>(E)) {
if (auto *RF = POE->getResultExpr()) {
@@ -73,22 +83,24 @@ bool tryToFindPtrOrigin(
continue;
}
if (auto *Expr = dyn_cast<ConditionalOperator>(E)) {
- return tryToFindPtrOrigin(Expr->getTrueExpr(), StopAtFirstRefCountedObj,
- isSafePtr, isSafePtrType, isSafeGlobalDecl,
- callback) &&
- tryToFindPtrOrigin(Expr->getFalseExpr(), StopAtFirstRefCountedObj,
- isSafePtr, isSafePtrType, isSafeGlobalDecl,
- callback);
+ return tryToFindPtrOriginImpl(Expr->getTrueExpr(),
+ StopAtFirstRefCountedObj, isSafePtr,
+ isSafePtrType, isSafeGlobalDecl, callback,
+ CrossedShortLivedTemporary) &&
+ tryToFindPtrOriginImpl(Expr->getFalseExpr(),
+ StopAtFirstRefCountedObj, isSafePtr,
+ isSafePtrType, isSafeGlobalDecl, callback,
+ CrossedShortLivedTemporary);
}
if (auto *cast = dyn_cast<CastExpr>(E)) {
if (StopAtFirstRefCountedObj) {
if (auto *ConversionFunc =
dyn_cast_or_null<FunctionDecl>(cast->getConversionFunction())) {
if (isCtorOfSafePtr(ConversionFunc))
- return callback(E, true);
+ return callback(E, /*IsSafe=*/true, CrossedShortLivedTemporary);
}
if (isa<CXXFunctionalCastExpr>(E) && isSafePtrType(cast->getType()))
- return callback(E, true);
+ return callback(E, /*IsSafe=*/true, CrossedShortLivedTemporary);
}
// FIXME: This can give false "origin" that would lead to false negatives
// in checkers. See https://reviews.llvm.org/D37023 for reference.
@@ -100,12 +112,12 @@ bool tryToFindPtrOrigin(
if (Callee->hasAttr<CFReturnsRetainedAttr>() ||
Callee->hasAttr<NSReturnsRetainedAttr>() ||
Callee->hasAttr<NSReturnsAutoreleasedAttr>()) {
- return callback(E, true);
+ return callback(E, /*IsSafe=*/true, CrossedShortLivedTemporary);
}
}
if (isSafePtrType(call->getType()))
- return callback(E, true);
+ return callback(E, /*IsSafe=*/true, CrossedShortLivedTemporary);
if (auto *memberCall = dyn_cast<CXXMemberCallExpr>(call)) {
if (auto *decl = memberCall->getMethodDecl()) {
@@ -113,7 +125,7 @@ bool tryToFindPtrOrigin(
if (IsGetterOfRefCt && *IsGetterOfRefCt) {
E = memberCall->getImplicitObjectArgument();
if (StopAtFirstRefCountedObj) {
- return callback(E, true);
+ return callback(E, /*IsSafe=*/true, CrossedShortLivedTemporary);
}
continue;
}
@@ -142,7 +154,7 @@ bool tryToFindPtrOrigin(
if (auto *callee = call->getDirectCallee()) {
if (isCtorOfSafePtr(callee)) {
if (StopAtFirstRefCountedObj)
- return callback(E, true);
+ return callback(E, /*IsSafe=*/true, CrossedShortLivedTemporary);
E = call->getArg(0);
continue;
@@ -154,10 +166,10 @@ bool tryToFindPtrOrigin(
}
if (isSafePtrType(callee->getReturnType()))
- return callback(E, true);
+ return callback(E, /*IsSafe=*/true, CrossedShortLivedTemporary);
if (isSingleton(callee))
- return callback(E, true);
+ return callback(E, /*IsSafe=*/true, CrossedShortLivedTemporary);
if (callee->isInStdNamespace() && safeGetName(callee) == "forward") {
E = call->getArg(0);
@@ -174,11 +186,11 @@ bool tryToFindPtrOrigin(
Name == "NSStringFromSelector" || Name == "NSSelectorFromString" ||
Name == "NSStringFromClass" || Name == "NSClassFromString" ||
Name == "NSStringFromProtocol" || Name == "NSProtocolFromString")
- return callback(E, true);
+ return callback(E, /*IsSafe=*/true, CrossedShortLivedTemporary);
} else if (auto *CalleeE = call->getCallee()) {
if (auto *E = dyn_cast<DeclRefExpr>(CalleeE->IgnoreParenCasts())) {
if (isSingleton(E->getFoundDecl()))
- return callback(E, true);
+ return callback(E, /*IsSafe=*/true, CrossedShortLivedTemporary);
}
if (auto *MemberExpr = dyn_cast<CXXDependentScopeMemberExpr>(CalleeE)) {
@@ -186,7 +198,7 @@ bool tryToFindPtrOrigin(
auto MemberName = MemberExpr->getMember().getAsString();
bool IsGetter = MemberName == "get" || MemberName == "ptr";
if (Base && isSafePtrType(Base->getType()) && IsGetter)
- return callback(E, true);
+ return callback(E, /*IsSafe=*/true, CrossedShortLivedTemporary);
}
}
@@ -201,7 +213,8 @@ bool tryToFindPtrOrigin(
if (auto *RD = dyn_cast<RecordType>(SubstType)) {
if (auto *CXX = dyn_cast<CXXRecordDecl>(RD->getDecl()))
if (isSafePtr(CXX))
- return callback(E, true);
+ return callback(E, /*IsSafe=*/true,
+ CrossedShortLivedTemporary);
}
}
}
@@ -211,22 +224,23 @@ bool tryToFindPtrOrigin(
if (auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E)) {
if (auto *Method = ObjCMsgExpr->getMethodDecl()) {
if (isSafePtrType(Method->getReturnType()))
- return callback(E, true);
+ return callback(E, /*IsSafe=*/true, CrossedShortLivedTemporary);
}
auto Selector = ObjCMsgExpr->getSelector();
auto NameForFirstSlot = Selector.getNameForSlot(0);
if ((NameForFirstSlot == "class" || NameForFirstSlot == "superclass") &&
!Selector.getNumArgs())
- return callback(E, true);
+ return callback(E, /*IsSafe=*/true, CrossedShortLivedTemporary);
}
if (auto *ObjCProtocol = dyn_cast<ObjCProtocolExpr>(E))
- return callback(ObjCProtocol, true);
+ return callback(ObjCProtocol, /*IsSafe=*/true,
+ CrossedShortLivedTemporary);
if (auto *ObjCDict = dyn_cast<ObjCDictionaryLiteral>(E))
- return callback(ObjCDict, true);
+ return callback(ObjCDict, /*IsSafe=*/true, CrossedShortLivedTemporary);
if (auto *ObjCArray = dyn_cast<ObjCArrayLiteral>(E))
- return callback(ObjCArray, true);
+ return callback(ObjCArray, /*IsSafe=*/true, CrossedShortLivedTemporary);
if (auto *ObjCStr = dyn_cast<ObjCStringLiteral>(E))
- return callback(ObjCStr, true);
+ return callback(ObjCStr, /*IsSafe=*/true, CrossedShortLivedTemporary);
if (auto *unaryOp = dyn_cast<UnaryOperator>(E)) {
// FIXME: Currently accepts ANY unary operator. Is it OK?
E = unaryOp->getSubExpr();
@@ -234,14 +248,29 @@ bool tryToFindPtrOrigin(
}
if (auto *BoxedExpr = dyn_cast<ObjCBoxedExpr>(E)) {
if (StopAtFirstRefCountedObj)
- return callback(BoxedExpr, true);
+ return callback(BoxedExpr, /*IsSafe=*/true, CrossedShortLivedTemporary);
E = BoxedExpr->getSubExpr();
continue;
}
break;
}
// Some other expression.
- return callback(E, false);
+ return callback(E, /*IsSafe=*/false, CrossedShortLivedTemporary);
+}
+
+bool tryToFindPtrOrigin(
+ const Expr *E, bool StopAtFirstRefCountedObj,
+ std::function<bool(const clang::CXXRecordDecl *)> isSafePtr,
+ std::function<bool(const clang::QualType)> isSafePtrType,
+ std::function<bool(const clang::Decl *)> isSafeGlobalDecl,
+ std::function<bool(const clang::Expr *, bool /*IsSafe*/,
+ bool /*CrossedShortLivedTemporary*/)>
+ callback) {
+ return tryToFindPtrOriginImpl(E, StopAtFirstRefCountedObj,
+ std::move(isSafePtr), std::move(isSafePtrType),
+ std::move(isSafeGlobalDecl),
+ std::move(callback),
+ /*CrossedShortLivedTemporary=*/false);
}
bool isASafeCallArg(const Expr *E) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
index fc2c43f33037e..47a86adb8453b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
@@ -49,15 +49,19 @@ class Expr;
/// represents ref-counted object during the traversal we return relevant
/// sub-expression and true.
///
-/// Calls \p callback with the subexpression that we traversed to and if \p
-/// StopAtFirstRefCountedObj is true we also specify whether we stopped early.
-/// Returns false if any of calls to callbacks returned false. Otherwise true.
+/// Calls \p callback for each origin the traversal reaches, passing the
+/// subexpression, whether the traversal recognized it as a safe origin, and
+/// whether the path to it crossed a temporary that dies at the end of the
+/// full-expression. Returns false if any of calls to callbacks returned false.
+/// Otherwise true.
bool tryToFindPtrOrigin(
const clang::Expr *E, bool StopAtFirstRefCountedObj,
std::function<bool(const clang::CXXRecordDecl *)> isSafePtr,
std::function<bool(const clang::QualType)> isSafePtrType,
std::function<bool(const clang::Decl *)> isSafeGlobalDecl,
- std::function<bool(const clang::Expr *, bool)> callback);
+ std::function<bool(const clang::Expr *, bool /*IsSafe*/,
+ bool /*CrossedShortLivedTemporary*/)>
+ callback);
/// For \p E referring to a ref-countable/-counted pointer/reference we return
/// whether it's a safe call argument. Examples: function parameter or
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
index 7e5261723014a..654621665bde6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
@@ -252,7 +252,10 @@ class RawPtrRefCallArgsChecker
[&](const clang::Decl *D) {
return Model->isSafeDecl(D, BR->getSourceManager());
},
- [&](const clang::Expr *ArgOrigin, bool IsSafe) {
+ // A temporary on the path to an argument's origin is safe: the full
+ // expression does not end until the call returns.
+ [&](const clang::Expr *ArgOrigin, bool IsSafe,
+ bool /*CrossedShortLivedTemporary*/) {
if (IsSafe)
return true;
if (isNullPtr(ArgOrigin))
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
index b83f1e3ea00b7..8884cee77a9ad 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
@@ -351,10 +351,14 @@ class RawPtrRefLocalVarsChecker
[&](const clang::Decl *D) {
return Model->isSafeDecl(D, BR->getSourceManager());
},
- [&](const clang::Expr *InitArgOrigin, bool IsSafe) {
- if (!InitArgOrigin || IsSafe)
+ [&](const clang::Expr *InitArgOrigin, bool IsSafe,
+ bool CrossedShortLivedTemporary) {
+ if (!InitArgOrigin)
return true;
+ if (IsSafe)
+ return !CrossedShortLivedTemporary;
+
if (isa<CXXThisExpr>(InitArgOrigin))
return true;
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
index c6c75968ae924..2a3d9f2fefab8 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
@@ -799,3 +799,27 @@ namespace using_reexported_ref_deref {
}
}
+
+namespace short_lived_temporaries {
+
+Ref<RefCountable> provide_ref();
+bool condition(const Ref<RefCountable> &);
+
+void dying_ref_temporary() {
+ RefCountable *bar = provide_ref().ptr();
+ // expected-warning at -1{{Local variable 'bar' is a raw pointer to RefPtr-capable type 'RefCountable' [alpha.webkit.UncountedLocalVarsChecker]}}
+ someFunction();
+ bar->method();
+}
+
+void unrelated_temporary_traces_to_guardian(RefCountable &obj) {
+ Ref<RefCountable> guardian(obj);
+ {
+ RefCountable *bar = condition(provide_ref()) ? guardian.ptr() : nullptr;
+ someFunction();
+ if (bar)
+ bar->method();
+ }
+}
+
+} // namespace short_lived_temporaries
More information about the cfe-commits
mailing list