Skip to content

Conversation

@rniwa
Copy link
Contributor

@rniwa rniwa commented Feb 4, 2025

When a callee is a method call (e.g. calling a lambda), we need to skip the object pointer to match the parameter list with the call arguments. This manifests as a bug that the checker erroneously generate a warning for a lambda capture (L1) which is passed to a no-escape argument of another lambda (L2).

@rniwa rniwa changed the title [webkit.UncountedLambdaCapturesChecker] Fix a bug that the checker didn't take the object pointer into acccount. [webkit.UncountedLambdaCapturesChecker] Fix a bug that the checker didn't take the object pointer into account. Feb 4, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Feb 4, 2025
…dn't take the object pointer into account.

When a callee is a method call (e.g. calling a lambda), we need to skip the object pointer
to match the parameter list with the call arguments. This manifests as a bug that the checker
erroneously generate a warning for a lambda capture (L1) which is passed to a no-escape argument of
another lambda (L2).
@llvmbot
Copy link
Member

llvmbot commented Feb 4, 2025

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)

Changes

When a callee is a method call (e.g. calling a lambda), we need to skip the object pointer to match the parameter list with the call arguments. This manifests as a bug that the checker erroneously generate a warning for a lambda capture (L1) which is passed to a no-escape argument of another lambda (L2).


Full diff: https://github.com/llvm/llvm-project/pull/125662.diff

2 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp (+3-1)
  • (modified) clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp (+11-1)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
index a57499d52acd0c..53ef423bd82e7e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -109,8 +109,10 @@ class UncountedLambdaCapturesChecker
       bool VisitCallExpr(CallExpr *CE) override {
         checkCalleeLambda(CE);
         if (auto *Callee = CE->getDirectCallee()) {
-          bool TreatAllArgsAsNoEscape = shouldTreatAllArgAsNoEscape(Callee);
           unsigned ArgIndex = 0;
+          if (auto *CXXCallee = dyn_cast<CXXMethodDecl>(Callee))
+            ArgIndex = CXXCallee->isInstance();
+          bool TreatAllArgsAsNoEscape = shouldTreatAllArgAsNoEscape(Callee);
           for (auto *Param : Callee->parameters()) {
             if (ArgIndex >= CE->getNumArgs())
               return true;
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
index 2173245bc7af3e..0f5ec8d8364325 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
@@ -252,13 +252,23 @@ struct RefCountableWithLambdaCapturingThis {
     call(lambda);
   }
 
-  void method_captures_this_with_guardian_refPtr() {
+  void method_captures_this_with_guardian_refptr() {
     auto lambda = [this, protectedThis = RefPtr { &*this }]() {
       nonTrivial();
     };
     call(lambda);
   }
 
+
+  void forEach(const WTF::Function<void(RefCountable&)>&);
+  void method_captures_this_with_lambda_with_no_escape() {
+    auto run = [&]([[clang::noescape]] const WTF::Function<void(RefCountable&)>& func) {
+      forEach(func);
+    };
+    run([&](RefCountable&) {
+      nonTrivial();
+    });
+  }
 };
 
 struct NonRefCountableWithLambdaCapturingThis {

Copy link
Contributor

@t-rasmud t-rasmud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@rniwa
Copy link
Contributor Author

rniwa commented Feb 4, 2025

Thanks for the review!

@rniwa rniwa merged commit d5a2638 into llvm:main Feb 5, 2025
8 checks passed
@rniwa rniwa deleted the fix-noescape-on-lambda-arg branch February 5, 2025 05:51
rniwa added a commit to rniwa/llvm-project that referenced this pull request Feb 5, 2025
…dn't take the object pointer into account. (llvm#125662)

When a callee is a method call (e.g. calling a lambda), we need to skip
the object pointer to match the parameter list with the call arguments.
This manifests as a bug that the checker erroneously generate a warning
for a lambda capture (L1) which is passed to a no-escape argument of
another lambda (L2).
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
…dn't take the object pointer into account. (llvm#125662)

When a callee is a method call (e.g. calling a lambda), we need to skip
the object pointer to match the parameter list with the call arguments.
This manifests as a bug that the checker erroneously generate a warning
for a lambda capture (L1) which is passed to a no-escape argument of
another lambda (L2).
devincoughlin pushed a commit to swiftlang/llvm-project that referenced this pull request Feb 25, 2025
…dn't take the object pointer into account. (llvm#125662)

When a callee is a method call (e.g. calling a lambda), we need to skip
the object pointer to match the parameter list with the call arguments.
This manifests as a bug that the checker erroneously generate a warning
for a lambda capture (L1) which is passed to a no-escape argument of
another lambda (L2).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:static analyzer clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants