Author: Ryosuke Niwa Date: 2024-12-19T18:11:28-08:00 New Revision: a71f9e6986b80fa90c453219795a1369b8ea7b6e
URL: https://github.com/llvm/llvm-project/commit/a71f9e6986b80fa90c453219795a1369b8ea7b6e DIFF: https://github.com/llvm/llvm-project/commit/a71f9e6986b80fa90c453219795a1369b8ea7b6e.diff LOG: [webkit.UncountedLambdaCapturesChecker] Detect protectedThis pattern. (#120528) In WebKit, we often capture this as Ref or RefPtr in addition to this itself so that the object lives as long as a capturing lambda stays alive. Detect this pattern and treat it as safe. This PR also makes the check for a lambda being passed as a function argument more robust by handling CXXBindTemporaryExpr, CXXConstructExpr, and DeclRefExpr referring to the lambda. Added: Modified: clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp index d786b02e2d7f3a..da9698e327562e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp @@ -115,7 +115,7 @@ class UncountedLambdaCapturesChecker if (ArgIndex >= CE->getNumArgs()) return true; auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts(); - if (auto *L = dyn_cast_or_null<LambdaExpr>(Arg)) { + if (auto *L = findLambdaInArg(Arg)) { LambdasToIgnore.insert(L); if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape) Checker->visitLambdaExpr(L, shouldCheckThis()); @@ -126,6 +126,38 @@ class UncountedLambdaCapturesChecker return true; } + LambdaExpr *findLambdaInArg(Expr *E) { + if (auto *Lambda = dyn_cast_or_null<LambdaExpr>(E)) + return Lambda; + auto *TempExpr = dyn_cast_or_null<CXXBindTemporaryExpr>(E); + if (!TempExpr) + return nullptr; + E = TempExpr->getSubExpr()->IgnoreParenCasts(); + if (!E) + return nullptr; + if (auto *Lambda = dyn_cast<LambdaExpr>(E)) + return Lambda; + auto *CE = dyn_cast_or_null<CXXConstructExpr>(E); + if (!CE || !CE->getNumArgs()) + return nullptr; + auto *CtorArg = CE->getArg(0)->IgnoreParenCasts(); + if (!CtorArg) + return nullptr; + if (auto *Lambda = dyn_cast<LambdaExpr>(CtorArg)) + return Lambda; + auto *DRE = dyn_cast<DeclRefExpr>(CtorArg); + if (!DRE) + return nullptr; + auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl()); + if (!VD) + return nullptr; + auto *Init = VD->getInit(); + if (!Init) + return nullptr; + TempExpr = dyn_cast<CXXBindTemporaryExpr>(Init->IgnoreParenCasts()); + return dyn_cast_or_null<LambdaExpr>(TempExpr->getSubExpr()); + } + void checkCalleeLambda(CallExpr *CE) { auto *Callee = CE->getCallee(); if (!Callee) @@ -180,11 +212,53 @@ class UncountedLambdaCapturesChecker } else if (C.capturesThis() && shouldCheckThis) { if (ignoreParamVarDecl) // this is always a parameter to this function. continue; - reportBugOnThisPtr(C); + bool hasProtectThis = false; + for (const LambdaCapture &OtherCapture : L->captures()) { + if (!OtherCapture.capturesVariable()) + continue; + if (auto *ValueDecl = OtherCapture.getCapturedVar()) { + if (protectThis(ValueDecl)) { + hasProtectThis = true; + break; + } + } + } + if (!hasProtectThis) + reportBugOnThisPtr(C); } } } + bool protectThis(const ValueDecl *ValueDecl) const { + auto *VD = dyn_cast<VarDecl>(ValueDecl); + if (!VD) + return false; + auto *Init = VD->getInit()->IgnoreParenCasts(); + if (!Init) + return false; + auto *BTE = dyn_cast<CXXBindTemporaryExpr>(Init); + if (!BTE) + return false; + auto *CE = dyn_cast_or_null<CXXConstructExpr>(BTE->getSubExpr()); + if (!CE) + return false; + auto *Ctor = CE->getConstructor(); + if (!Ctor) + return false; + auto clsName = safeGetName(Ctor->getParent()); + if (!isRefType(clsName) || !CE->getNumArgs()) + return false; + auto *Arg = CE->getArg(0)->IgnoreParenCasts(); + while (auto *UO = dyn_cast<UnaryOperator>(Arg)) { + auto OpCode = UO->getOpcode(); + if (OpCode == UO_Deref || OpCode == UO_AddrOf) + Arg = UO->getSubExpr(); + else + break; + } + return isa<CXXThisExpr>(Arg); + } + void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar, const QualType T) const { assert(CapturedVar); diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp index daff32e9940c83..2173245bc7af3e 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp @@ -207,6 +207,58 @@ struct RefCountableWithLambdaCapturingThis { }; call(lambda); } + + void method_captures_this_unsafe_capture_local_var_explicitly() { + RefCountable* x = make_obj(); + call([this, protectedThis = RefPtr { this }, x]() { + // expected-warning@-1{{Captured raw-pointer 'x' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + nonTrivial(); + x->method(); + }); + } + + void method_captures_this_with_other_protected_var() { + RefCountable* x = make_obj(); + call([this, protectedX = RefPtr { x }]() { + // expected-warning@-1{{Captured raw-pointer 'this' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + nonTrivial(); + protectedX->method(); + }); + } + + void method_captures_this_unsafe_capture_local_var_explicitly_with_deref() { + RefCountable* x = make_obj(); + call([this, protectedThis = Ref { *this }, x]() { + // expected-warning@-1{{Captured raw-pointer 'x' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + nonTrivial(); + x->method(); + }); + } + + void method_captures_this_unsafe_local_var_via_vardecl() { + RefCountable* x = make_obj(); + auto lambda = [this, protectedThis = Ref { *this }, x]() { + // expected-warning@-1{{Captured raw-pointer 'x' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + nonTrivial(); + x->method(); + }; + call(lambda); + } + + void method_captures_this_with_guardian() { + auto lambda = [this, protectedThis = Ref { *this }]() { + nonTrivial(); + }; + call(lambda); + } + + void method_captures_this_with_guardian_refPtr() { + auto lambda = [this, protectedThis = RefPtr { &*this }]() { + nonTrivial(); + }; + call(lambda); + } + }; struct NonRefCountableWithLambdaCapturingThis { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
