[clang] 8952b11 - [WebKit checkers] Treat destruction of a null smart pointer temporary as trivial (#230809)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Oct 11 11:51:13 PDT 2026
Author: Ryosuke Niwa
Date: 2026-10-11T11:51:08-07:00
New Revision: 8952b118683076f26f32ab8aefd834211bf4ab01
URL: https://github.com/llvm/llvm-project/commit/8952b118683076f26f32ab8aefd834211bf4ab01
DIFF: https://github.com/llvm/llvm-project/commit/8952b118683076f26f32ab8aefd834211bf4ab01.diff
LOG: [WebKit checkers] Treat destruction of a null smart pointer temporary as trivial (#230809)
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.
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
clang/test/Analysis/Checkers/WebKit/mock-types.h
clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
clang/test/Analysis/Checkers/WebKit/nodelete-lazy-initialize.cpp
clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index ed290cd11a692..0a98dc6357bd6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -1275,9 +1275,63 @@ 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 (Ctor->isDefaultConstructor())
+ return true;
+ 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)
+ 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)
+ 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 +1413,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/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();
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
index 923bcca5c898e..e26bd87e0d425 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
@@ -999,3 +999,57 @@ 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> 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) {
+ {
+ 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
More information about the cfe-commits
mailing list