[clang] [alpha.webkit.UncountedLocalVarsChecker] Detect assignments to uncounted local variable and parameters. (PR #92639)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: Ryosuke Niwa (rniwa)

<details>
<summary>Changes</summary>

This PR updates alpha.webkit.UncountedLocalVarsChecker to emit warnings for assignments to uncounted local variable and parameters instead of just the initialization during the declaration.

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


2 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp (+37-24) 
- (modified) clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp (+42) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
index 0d9710a5e2d83..6c0d56303d5ad 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
@@ -135,7 +135,19 @@ class UncountedLocalVarsChecker
       bool shouldVisitImplicitCode() const { return false; }
 
       bool VisitVarDecl(VarDecl *V) {
-        Checker->visitVarDecl(V);
+        auto *Init = V->getInit();
+        if (Init && V->isLocalVarDecl())
+          Checker->visitVarDecl(V, Init);
+        return true;
+      }
+
+      bool VisitBinaryOperator(const BinaryOperator *BO) {
+        if (BO->isAssignmentOp()) {
+          if (auto *VarRef = dyn_cast<DeclRefExpr>(BO->getLHS())) {
+            if (auto *V = dyn_cast<VarDecl>(VarRef->getDecl()))
+              Checker->visitVarDecl(V, BO->getRHS());
+          }
+        }
         return true;
       }
 
@@ -174,7 +186,7 @@ class UncountedLocalVarsChecker
     visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
   }
 
-  void visitVarDecl(const VarDecl *V) const {
+  void visitVarDecl(const VarDecl *V, const Expr *Value) const {
     if (shouldSkipVarDecl(V))
       return;
 
@@ -184,12 +196,8 @@ class UncountedLocalVarsChecker
 
     std::optional<bool> IsUncountedPtr = isUncountedPtr(ArgType);
     if (IsUncountedPtr && *IsUncountedPtr) {
-      const Expr *const InitExpr = V->getInit();
-      if (!InitExpr)
-        return; // FIXME: later on we might warn on uninitialized vars too
-
       if (tryToFindPtrOrigin(
-              InitExpr, /*StopAtFirstRefCountedObj=*/false,
+              Value, /*StopAtFirstRefCountedObj=*/false,
               [&](const clang::Expr *InitArgOrigin, bool IsSafe) {
                 if (!InitArgOrigin)
                   return true;
@@ -232,34 +240,39 @@ class UncountedLocalVarsChecker
               }))
         return;
 
-      reportBug(V);
+      reportBug(V, Value);
     }
   }
 
   bool shouldSkipVarDecl(const VarDecl *V) const {
     assert(V);
-    if (!V->isLocalVarDecl())
-      return true;
-
-    if (BR->getSourceManager().isInSystemHeader(V->getLocation()))
-      return true;
-
-    return false;
+    return BR->getSourceManager().isInSystemHeader(V->getLocation());
   }
 
-  void reportBug(const VarDecl *V) const {
+  void reportBug(const VarDecl *V, const Expr *Value) const {
     assert(V);
     SmallString<100> Buf;
     llvm::raw_svector_ostream Os(Buf);
 
-    Os << "Local variable ";
-    printQuotedQualifiedName(Os, V);
-    Os << " is uncounted and unsafe.";
-
-    PathDiagnosticLocation BSLoc(V->getLocation(), BR->getSourceManager());
-    auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
-    Report->addRange(V->getSourceRange());
-    BR->emitReport(std::move(Report));
+    if (dyn_cast<ParmVarDecl>(V)) {
+      Os << "Assignment to an uncounted parameter ";
+      printQuotedQualifiedName(Os, V);
+      Os << " is unsafe.";
+
+      PathDiagnosticLocation BSLoc(Value->getExprLoc(), BR->getSourceManager());
+      auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
+      Report->addRange(Value->getSourceRange());
+      BR->emitReport(std::move(Report));
+    } else {
+      Os << "Local variable ";
+      printQuotedQualifiedName(Os, V);
+      Os << " is uncounted and unsafe.";
+
+      PathDiagnosticLocation BSLoc(V->getLocation(), BR->getSourceManager());
+      auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
+      Report->addRange(V->getSourceRange());
+      BR->emitReport(std::move(Report));
+    }
   }
 };
 } // namespace
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
index 632a82eb0d8d1..abcb9c5122f70 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
@@ -216,3 +216,45 @@ void foo() {
 }
 
 } // namespace conditional_op
+
+namespace local_assignment_basic {
+
+RefCountable *provide_ref_ctnbl();
+
+void foo(RefCountable* a) {
+  RefCountable* b = a;
+  // expected-warning at -1{{Local variable 'b' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+  if (b->trivial())
+    b = provide_ref_ctnbl();
+}
+
+void bar(RefCountable* a) {
+  RefCountable* b;
+  // expected-warning at -1{{Local variable 'b' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+  b = provide_ref_ctnbl();
+}
+
+void baz() {
+  RefPtr a = provide_ref_ctnbl();
+  {
+    RefCountable* b = a.get();
+    // expected-warning at -1{{Local variable 'b' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+    b = provide_ref_ctnbl();
+  }
+}
+
+} // namespace local_assignment_basic
+
+namespace local_assignment_to_parameter {
+
+RefCountable *provide_ref_ctnbl();
+void someFunction();
+
+void foo(RefCountable* a) {
+  a = provide_ref_ctnbl();
+  // expected-warning at -1{{Assignment to an uncounted parameter 'a' is unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+  someFunction();
+  a->method();
+}
+
+} // namespace local_assignment_to_parameter

``````````

</details>


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