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. // [email protected]:37 + {{Calling '~RefPtr'}} // [email protected]:299 + {{Calling 'deref'}} -// [email protected]:313 + {{Calling 'derefIfNotNull'}} -// [email protected]:380 + {{Calling 'deref'}} -// [email protected]:399 + {{Calling '~RefPtr'}} -// [email protected]:440 + {{Could destruct an object}} +// [email protected]:316 + {{Calling 'derefIfNotNull'}} +// [email protected]:383 + {{Calling 'deref'}} +// [email protected]:402 + {{Calling '~RefPtr'}} +// [email protected]: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" // [email protected]:299 + {{Calling 'deref'}} -// [email protected]:313 + {{Calling 'derefIfNotNull'}} +// [email protected]: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@-1{{Local variable 'bar' is a raw pointer to RefPtr-capable type 'RefCountable' [alpha.webkit.UncountedLocalVarsChecker]}} + bar->trivial(); + } +} + +} // namespace hash_table_empty_value _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
