[clang] [WebKit checkers] Treat destruction of a null smart pointer temporary as trivial (PR #230809)
Ryosuke Niwa via cfe-commits
cfe-commits at lists.llvm.org
Sun Oct 11 09:34:07 PDT 2026
https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/230809
>From 97b279c920bba91a42b973b620b75fea45a532ae Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Sat, 10 Oct 2026 09:18:13 -0700
Subject: [PATCH 1/3] [WebKit checkers] Treat destruction of a null smart
pointer temporary as trivial
TrivialFunctionAnalysis treated HashMap<K, Ref<V>>::get as non-trivial
because its miss path, MappedTraits::peek(MappedTraits::emptyValue()),
creates and destroys a temporary Ref<V> constructed from
HashTableEmptyValue. Ref's destructor calls deref(), which may delete, so
any local raw pointer obtained from such a HashMap was reported by
alpha.webkit.UncountedLocalVarsChecker even in otherwise trivial contexts.
Skip the destructor check of a temporary owning smart pointer when it is
known to hold nullptr: one which is default constructed, constructed from
nullptr or HashTableEmptyValue, copied or moved from such a smart pointer,
or returned by a function whose body is a single return statement of such
a smart pointer (e.g. HashTraits<Ref<T>>::emptyValue()).
Also treat CXXScalarValueInitExpr (e.g. T() for a scalar T as in
GenericHashTraits<T>::emptyValue()) as trivial.
Authored with Claude Code.
---
.../Checkers/WebKit/PtrTypesSemantics.cpp | 60 ++++++++++++++++++-
.../Analysis/Checkers/WebKit/mock-types.h | 3 +
.../Checkers/WebKit/uncounted-local-vars.cpp | 45 ++++++++++++++
3 files changed, 107 insertions(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index ed290cd11a692..30762a5db7427 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -1275,9 +1275,62 @@ class TrivialFunctionAnalysisVisitor
return Visit(VMT->getSubExpr());
}
+ // Returns true if E is known to evaluate to a smart pointer which holds
+ // nullptr, e.g. RefPtr<T>(), RefPtr<T>(nullptr), Ref<T>(HashTableEmptyValue),
+ // or a call to a function which simply returns one such as
+ // HashTraits<Ref<T>>::emptyValue(). Destructing such a smart pointer can't
+ // delete anything.
+ bool isNullSmartPtr(const Expr *E, unsigned Depth = 0) {
+ if (!E || Depth > 4)
+ return false;
+ while (true) {
+ E = E->IgnoreParenCasts();
+ if (auto *EWC = dyn_cast<ExprWithCleanups>(E))
+ E = EWC->getSubExpr();
+ else if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(E))
+ E = MTE->getSubExpr();
+ else if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(E))
+ E = BTE->getSubExpr();
+ else
+ break;
+ }
+ if (auto *CE = dyn_cast<CXXConstructExpr>(E)) {
+ auto *Ctor = CE->getConstructor();
+ if (!Ctor || !isOwnerPtr(safeGetName(Ctor->getParent())))
+ return false;
+ if (!CE->getNumArgs())
+ return Ctor->isDefaultConstructor();
+ if (CE->getNumArgs() != 1)
+ return false;
+ auto *Arg = CE->getArg(0)->IgnoreParenImpCasts();
+ auto ArgType = Arg->getType();
+ if (ArgType->isNullPtrType())
+ return true;
+ if (auto *TD = ArgType->getAsTagDecl();
+ TD && safeGetName(TD) == "HashTableEmptyValueType")
+ return true;
+ if (Ctor->isCopyOrMoveConstructor())
+ return isNullSmartPtr(Arg, Depth + 1);
+ return false;
+ }
+ if (auto *CE = dyn_cast<CallExpr>(E)) {
+ auto *Callee = CE->getDirectCallee();
+ if (!Callee || (isa<CXXMethodDecl>(Callee) &&
+ cast<CXXMethodDecl>(Callee)->isVirtual()))
+ return false;
+ auto *Body = dyn_cast_or_null<CompoundStmt>(Callee->getBody());
+ if (!Body || Body->size() != 1)
+ return false;
+ auto *RS = dyn_cast<ReturnStmt>(Body->body_front());
+ return RS && isNullSmartPtr(RS->getRetValue(), Depth + 1);
+ }
+ return false;
+ }
+
bool VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *BTE) {
if (auto *Temp = BTE->getTemporary()) {
- if (!IsFunctionTrivial(Temp->getDestructor()))
+ if (!isNullSmartPtr(BTE->getSubExpr()) &&
+ !IsFunctionTrivial(Temp->getDestructor()))
return false;
}
return Visit(BTE->getSubExpr());
@@ -1359,6 +1412,11 @@ class TrivialFunctionAnalysisVisitor
return true;
}
+ bool VisitCXXScalarValueInitExpr(const CXXScalarValueInitExpr *E) {
+ // A value initialization of a scalar type such as int() is trivial.
+ return true;
+ }
+
private:
CacheTy &Cache;
CacheTy FieldCtorCache;
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h
index 46818a9642ee3..6deac440a06e1 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -300,12 +300,15 @@ template<typename T> struct DefaultRefDerefTraits {
}
};
+enum HashTableEmptyValueType { HashTableEmptyValue };
+
template <typename T, typename PtrTraits = RawPtrTraits<T>, typename RefDerefTraits = DefaultRefDerefTraits<T>> struct Ref {
typename PtrTraits::StorageType t;
enum AdoptTag { Adopt };
Ref() : t{} {};
+ Ref(HashTableEmptyValueType) : t(nullptr) { }
Ref(T &t, AdoptTag) : t(&t) { }
Ref(T &t) : t(&RefDerefTraits::ref(t)) { }
Ref(const Ref& o) : t(RefDerefTraits::refIfNotNull(PtrTraits::unwrap(o.t))) { }
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
index 6c1299998e646..cd40c67749267 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
@@ -977,3 +977,48 @@ class Owner {
};
} // namespace call_returning_reference_to_smart_pointer
+
+namespace hash_table_empty_value {
+
+template <typename T> struct HashTraits {
+ static T emptyValue() { return T(); }
+ static T peek(const T& value) { return value; }
+};
+
+template <typename P> struct HashTraits<Ref<P>> {
+ static Ref<P> emptyValue() { return HashTableEmptyValue; }
+ static P* peek(const Ref<P>& value) { return value.ptr(); }
+};
+
+template <typename KeyType, typename MappedType> struct SingleEntryHashMap {
+ using KeyTraits = HashTraits<KeyType>;
+ using MappedTraits = HashTraits<MappedType>;
+
+ auto get(const KeyType& key) const {
+ if (key == KeyTraits::emptyValue() || key != m_key)
+ return MappedTraits::peek(MappedTraits::emptyValue());
+ return MappedTraits::peek(*m_value);
+ }
+
+ KeyType m_key;
+ MappedType* m_value;
+};
+
+void get_in_trivial_context(const SingleEntryHashMap<int, Ref<RefCountable>>& map) {
+ {
+ auto* obj = map.get(1); // no-warning
+ obj->trivial();
+ }
+}
+
+Ref<RefCountable> makeRef(RefCountable& obj) { return obj; }
+
+void non_null_temporary(RefCountable& obj) {
+ {
+ auto* bar = HashTraits<Ref<RefCountable>>::peek(makeRef(obj));
+ // expected-warning at -1{{Local variable 'bar' is a raw pointer to RefPtr-capable type 'RefCountable' [alpha.webkit.UncountedLocalVarsChecker]}}
+ bar->trivial();
+ }
+}
+
+} // namespace hash_table_empty_value
>From 4ef2735d444c3d723c8b22c29f933482a9ae9059 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Sat, 10 Oct 2026 23:23:44 -0700
Subject: [PATCH 2/3] Incorporate the review feedback and add a test for a free
standing function which returns HashTableEmptyValue.
---
.../StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp | 9 +++++----
.../Analysis/Checkers/WebKit/uncounted-local-vars.cpp | 9 +++++++++
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 30762a5db7427..0a98dc6357bd6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -1298,8 +1298,8 @@ class TrivialFunctionAnalysisVisitor
auto *Ctor = CE->getConstructor();
if (!Ctor || !isOwnerPtr(safeGetName(Ctor->getParent())))
return false;
- if (!CE->getNumArgs())
- return Ctor->isDefaultConstructor();
+ if (Ctor->isDefaultConstructor())
+ return true;
if (CE->getNumArgs() != 1)
return false;
auto *Arg = CE->getArg(0)->IgnoreParenImpCasts();
@@ -1315,8 +1315,9 @@ class TrivialFunctionAnalysisVisitor
}
if (auto *CE = dyn_cast<CallExpr>(E)) {
auto *Callee = CE->getDirectCallee();
- if (!Callee || (isa<CXXMethodDecl>(Callee) &&
- cast<CXXMethodDecl>(Callee)->isVirtual()))
+ if (!Callee)
+ return false;
+ if (auto *MD = dyn_cast<CXXMethodDecl>(Callee); MD && MD->isVirtual())
return false;
auto *Body = dyn_cast_or_null<CompoundStmt>(Callee->getBody());
if (!Body || Body->size() != 1)
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
index cd40c67749267..eb0fb0de07665 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
@@ -1011,6 +1011,15 @@ void get_in_trivial_context(const SingleEntryHashMap<int, Ref<RefCountable>>& ma
}
}
+Ref<RefCountable> makeEmptyRef() { return HashTableEmptyValue; }
+
+void null_temporary_from_free_function() {
+ {
+ auto* foo = HashTraits<Ref<RefCountable>>::peek(makeEmptyRef()); // no-warning
+ foo->trivial();
+ }
+}
+
Ref<RefCountable> makeRef(RefCountable& obj) { return obj; }
void non_null_temporary(RefCountable& obj) {
>From 689223765eec2a033b77e6c79f451c7aa90dd750 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Sun, 11 Oct 2026 09:33:50 -0700
Subject: [PATCH 3/3] Fix tests
---
.../test/Analysis/Checkers/WebKit/nodelete-annotation.cpp | 8 ++++----
.../Analysis/Checkers/WebKit/nodelete-lazy-initialize.cpp | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
index 687eb4c1d62e3..bffc04c764651 100644
--- a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
@@ -8,10 +8,10 @@
// those notes have to be expected by file and line.
// expected-note at mock-types.h:37 + {{Calling '~RefPtr'}}
// expected-note at mock-types.h:299 + {{Calling 'deref'}}
-// expected-note at mock-types.h:313 + {{Calling 'derefIfNotNull'}}
-// expected-note at mock-types.h:380 + {{Calling 'deref'}}
-// expected-note at mock-types.h:399 + {{Calling '~RefPtr'}}
-// expected-note at mock-types.h:440 + {{Could destruct an object}}
+// expected-note at mock-types.h:316 + {{Calling 'derefIfNotNull'}}
+// expected-note at mock-types.h:383 + {{Calling 'deref'}}
+// expected-note at mock-types.h:402 + {{Calling '~RefPtr'}}
+// expected-note at mock-types.h:443 + {{Could destruct an object}}
void *memcpy(void *dst, const void *src, unsigned int size);
void *malloc(unsigned int size);
diff --git a/clang/test/Analysis/Checkers/WebKit/nodelete-lazy-initialize.cpp b/clang/test/Analysis/Checkers/WebKit/nodelete-lazy-initialize.cpp
index e664ab5222238..c804cd7e31e10 100644
--- a/clang/test/Analysis/Checkers/WebKit/nodelete-lazy-initialize.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/nodelete-lazy-initialize.cpp
@@ -3,7 +3,7 @@
#include "mock-types.h"
// expected-note at mock-types.h:299 + {{Calling 'deref'}}
-// expected-note at mock-types.h:313 + {{Calling 'derefIfNotNull'}}
+// expected-note at mock-types.h:316 + {{Calling 'derefIfNotNull'}}
void crash();
More information about the cfe-commits
mailing list