[clang] [alpha.webkit.UncountedLocalVarsChecker] Don't treat a const operator call on a guardian as a mutation (PR #230900)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)

<details>
<summary>Changes</summary>

GuardianVisitor treated `guardian->method()` as a mutation of the guardian because the implicit object argument of a member operator call had no corresponding parameter, and the check fell back to the non-const type of the guardian itself. Skip the implicit object argument when the operator is a const member function, and only apply the argument offset for implicit object member operators so that arguments of non-member operators map to the right parameters.

Also treat passing a guardian to a const reference parameter as non-mutating by checking the constness of the referenced type instead of the reference type.

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


3 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp (+10-2) 
- (modified) clang/test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp (+9) 
- (modified) clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp (+22) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
index 232cca1c7a2c4..b3ddbb858044c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
@@ -96,14 +96,22 @@ struct GuardianVisitor : DynamicRecursiveASTVisitor {
       if (isGetterOfSafePtr(Method).value_or(false))
         return true;
     }
+    // For a member operator call, the first argument is the implicit object.
+    auto *OperatorMethod = dyn_cast<CXXMethodDecl>(Callee);
+    if (!isa<CXXOperatorCallExpr>(CE) || !OperatorMethod ||
+        !OperatorMethod->isImplicitObjectMemberFunction())
+      OperatorMethod = nullptr;
     unsigned ArgIndex = 0;
-    unsigned ArgOffset = isa<CXXOperatorCallExpr>(CE);
+    unsigned ArgOffset = OperatorMethod ? 1 : 0;
     for (auto *Arg : CE->arguments()) {
       ParmVarDecl *Parm = nullptr;
       if (ArgIndex >= ArgOffset) {
         unsigned ParmIndex = ArgIndex - ArgOffset;
         if (ParmIndex < Callee->getNumParams())
           Parm = Callee->getParamDecl(ParmIndex);
+      } else if (OperatorMethod->isConst()) {
+        ArgIndex++;
+        continue;
       }
       if (mutatesGuardian(Arg, Parm))
         return false;
@@ -152,7 +160,7 @@ struct GuardianVisitor : DynamicRecursiveASTVisitor {
         return false;
       return !ArgType.getNonReferenceType().isConstQualified();
     }
-    return !ArgType.isConstQualified();
+    return !ArgType.getNonReferenceType().isConstQualified();
   }
 };
 
diff --git a/clang/test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp
index a0ce0ffbff90d..a3f7d23328e52 100644
--- a/clang/test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp
@@ -360,6 +360,15 @@ void local_var_with_guardian_checked_ref() {
   }
 }
 
+void local_var_with_guardian_used_via_arrow() {
+  Ref<RefCountableAndCheckable> a = *provide_obj();
+  {
+    RefCountableAndCheckable* b = a.ptr();
+    a->method();
+    b->method();
+  }
+}
+
 void static_var() {
   static RefCountableAndCheckable* a = nullptr;
   // expected-warning at -1{{Static local variable 'a' is a raw pointer to CheckedPtr-capable type 'RefCountableAndCheckable' [alpha.webkit.UncheckedLocalVarsChecker]}}
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
index 7f086f13f68e4..a7a5e9ae19746 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
@@ -214,6 +214,28 @@ void foo9(RefCountable& o) {
   }
 }
 
+bool operator==(const Ref<RefCountable>&, const Ref<RefCountable>&);
+void consumeConstRef(const Ref<RefCountable>&);
+
+void guardian_const_operator_call(RefCountable& o, Ref<RefCountable>& other) {
+  Ref<RefCountable> guardian(o);
+  {
+    RefCountable *bar = guardian.ptr();
+    guardian->method();
+    bar->method();
+  }
+  {
+    RefCountable *bar = guardian.ptr();
+    if (guardian == other)
+      bar->method();
+  }
+  {
+    RefCountable *bar = guardian.ptr();
+    consumeConstRef(guardian);
+    bar->method();
+  }
+}
+
 RefCountable* provide();
 
 struct Derived : public RefCountable {

``````````

</details>


https://github.com/llvm/llvm-project/pull/230900