Skip to content

[webkit.UncountedLambdaCapturesChecker] Fix a bug that the checker didn't take the object pointer into account. #125662

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 5, 2025

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

Verified

This commit was signed with the committer’s verified signature.
Icohedron Deric C.
…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.

None yet

3 participants