Author: Ryosuke Niwa Date: 2026-09-23T14:01:31-07:00 New Revision: d28d490e113147bed0061a1bde79ebc982cbdb52
URL: https://github.com/llvm/llvm-project/commit/d28d490e113147bed0061a1bde79ebc982cbdb52 DIFF: https://github.com/llvm/llvm-project/commit/d28d490e113147bed0061a1bde79ebc982cbdb52.diff LOG: [alpha.webkit.UncountedLocalVarsChecker] Wait for the instantiation to check a dependent local variable (#224566) RawPtrRefLocalVarsChecker had the same false positive as the call arguments checker: a local variable whose declared type is a concrete raw pointer was reported when its initializer was still type-dependent. In template <typename T> void f(T& guard) { Voice* v = guard.ptr(); } the declared type Voice* makes isUnsafePtr fire, while guard.ptr() is a CXXDependentScopeMemberExpr, so tryToFindPtrOrigin bails out before it can reach the guardian analysis which makes the resolved form safe. The instantiation is character for character the concrete form, which is not reported. Skip type-dependent initializers and traverse the instantiated call operators of a generic lambda, as in the call arguments checker. Unlike there, each specialization also has to be installed as DeclWithIssue while its body is traversed: the guardian analysis runs GuardianVisitor over the body of DeclWithIssue, and pointed at the function enclosing the lambda it walks the dependent pattern instead, where VisitCallExpr gives up on a null getDirectCallee() and every guarded local is reported again. Added: clang/test/Analysis/Checkers/WebKit/local-vars-dependent-init.cpp Modified: clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp index dfb2349b390f0..d0191bd0ccc62 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp @@ -273,12 +273,42 @@ class RawPtrRefLocalVarsChecker } bool TraverseDecl(Decl *D) override { + // A template pattern is checked through its instantiations, which are + // traversed from the TemplateDecl itself. In the pattern the callee of + // a call may still be an unresolved overload set and the type of an + // expression may still be dependent, neither of which can be reasoned + // about, so don't enter it at all. + if (D && !isa<TemplateDecl>(D) && D->isTemplated()) + return true; llvm::SaveAndRestore SavedDecl(DeclWithIssue); if (D && (isa<FunctionDecl>(D) || isa<ObjCMethodDecl>(D))) DeclWithIssue = D; return DynamicRecursiveASTVisitor::TraverseDecl(D); } + bool TraverseLambdaExpr(LambdaExpr *L) override { + auto *FTD = L->getLambdaClass()->getDependentLambdaCallOperator(); + if (!FTD) + return DynamicRecursiveASTVisitor::TraverseLambdaExpr(L); + // The body of a generic lambda is the pattern of its call operator, + // but it is reached from the LambdaExpr as a statement, so TraverseDecl + // never gets to skip it. Traverse the capture initializers, which are + // evaluated in the enclosing scope, and then the call operator itself, + // of which the pattern is skipped like any other and the instantiations + // are traversed. Going through TraverseDecl also makes an instantiation + // the decl with the issue, so that a guardian is looked up in it rather + // than in the pattern. The initializers are traversed as expressions + // because the variable of an init capture is declared in the pattern. + for (unsigned I = 0, N = L->capture_size(); I != N; ++I) { + if (!(L->capture_begin() + I)->isExplicit()) + continue; + if (auto *Init = L->capture_init_begin()[I]; + Init && !TraverseStmt(Init)) + return false; + } + return TraverseDecl(FTD); + } + bool VisitTypedefDecl(TypedefDecl *TD) override { if (auto *RTC = Checker->Model->retainTypeChecker()) RTC->visitTypedef(TD); diff --git a/clang/test/Analysis/Checkers/WebKit/local-vars-dependent-init.cpp b/clang/test/Analysis/Checkers/WebKit/local-vars-dependent-init.cpp new file mode 100644 index 0000000000000..2984c89fc6d33 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/local-vars-dependent-init.cpp @@ -0,0 +1,82 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedLocalVarsChecker -verify %s + +#include "mock-types.h" + +class Voice : public RefCountable { }; + +template <typename T> T* unsafeCast(T& t) { return &t; } + +Voice* provide(); +void consume(Voice&); + +template <typename F> void map(F f) { + Ref<Voice> voice = adoptRef(*provide()); + f(voice); +} + +// The Ref parameter guards the raw pointer for the duration of the call. +void concreteGuarded(Ref<Voice>& guard) { + Voice* v = guard.ptr(); + consume(*v); +} + +// Same code, but the receiver of ptr() is only resolved in the instantiation. +template <typename T> void tmplGuarded(T& guard) { + Voice* v = guard.ptr(); + consume(*v); +} + +void lambdaGuarded() { + map([](auto& guard) { + Voice* v = guard.ptr(); + consume(*v); + }); +} + +void concreteLambdaGuarded() { + map([](Ref<Voice>& guard) { + Voice* v = guard.ptr(); + consume(*v); + }); +} + +// The initializer is only known to be unsafe once unsafeCast() is resolved. +void lambdaUnsafe() { + map([](auto& guard) { + Voice* v = unsafeCast(guard.get()); + // expected-warning@-1{{Local variable 'v' is a raw pointer to RefPtr-capable type 'Voice'}} + consume(*v); + }); +} + +// Mutating the guardian invalidates the raw pointer. The assignment is only +// visible in the instantiated body, so the instantiation has to be the decl +// the guardian is looked up in. +void concreteLambdaMutatedGuardian() { + map([](Ref<Voice>& guard) { + Voice* v = guard.ptr(); + // expected-warning@-1{{Local variable 'v' is a raw pointer to RefPtr-capable type 'Voice'}} + guard = adoptRef(*provide()); + consume(*v); + }); +} + +void lambdaMutatedGuardian() { + map([](auto& guard) { + Voice* v = guard.ptr(); + // expected-warning@-1{{Local variable 'v' is a raw pointer to RefPtr-capable type 'Voice'}} + guard = adoptRef(*provide()); + consume(*v); + }); +} + +// A pattern which is never instantiated generates no code to diagnose. +template <typename T> void neverInstantiated(T& guard) { + Voice* v = unsafeCast(guard.get()); + consume(*v); +} + +void instantiate() { + Ref<Voice> voice = adoptRef(*provide()); + tmplGuarded(voice); +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
