llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Ryosuke Niwa (rniwa) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/230809.diff 3 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+59-1) - (modified) clang/test/Analysis/Checkers/WebKit/mock-types.h (+3) - (modified) clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp (+45) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index 1cd2822575845..dda64bea9fec6 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -1227,9 +1227,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()); @@ -1300,6 +1353,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 7f086f13f68e4..239b11ef90fbc 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp @@ -945,3 +945,48 @@ void unrelated_temporary_traces_to_guardian(RefCountable &obj) { } } // namespace short_lived_temporaries + +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@-1{{Local variable 'bar' is a raw pointer to RefPtr-capable type 'RefCountable' [alpha.webkit.UncountedLocalVarsChecker]}} + bar->trivial(); + } +} + +} // namespace hash_table_empty_value `````````` </details> https://github.com/llvm/llvm-project/pull/230809 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
