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

[via cfe-commits]

Author: Ryosuke Niwa
Date: 2024-05-24T00:06:06-07:00
New Revision: b80d982a62676314ec93dc8881b9f8957217192a

URL: https://github.com/llvm/llvm-project/commit/b80d982a62676314ec93dc8881b9f8957217192a
DIFF: https://github.com/llvm/llvm-project/commit/b80d982a62676314ec93dc8881b9f8957217192a.diff

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

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.

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
    clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
index 0d9710a5e2d83..274da0baf2ce5 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,46 @@ 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 {
+      if (V->hasLocalStorage())
+        Os << "Local variable ";
+      else if (V->isStaticLocal())
+        Os << "Static local variable ";
+      else if (V->hasGlobalStorage())
+        Os << "Global variable ";
+      else
+        Os << "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..25776870dd3ae 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
@@ -216,3 +216,76 @@ void foo() {
 }
 
 } // namespace conditional_op
+
+namespace local_assignment_basic {
+
+RefCountable *provide_ref_cntbl();
+
+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_cntbl();
+}
+
+void bar(RefCountable* a) {
+  RefCountable* b;
+  // expected-warning at -1{{Local variable 'b' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+  b = provide_ref_cntbl();
+}
+
+void baz() {
+  RefPtr a = provide_ref_cntbl();
+  {
+    RefCountable* b = a.get();
+    // expected-warning at -1{{Local variable 'b' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+    b = provide_ref_cntbl();
+  }
+}
+
+} // namespace local_assignment_basic
+
+namespace local_assignment_to_parameter {
+
+RefCountable *provide_ref_cntbl();
+void someFunction();
+
+void foo(RefCountable* a) {
+  a = provide_ref_cntbl();
+  // expected-warning at -1{{Assignment to an uncounted parameter 'a' is unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+  someFunction();
+  a->method();
+}
+
+} // namespace local_assignment_to_parameter
+
+namespace local_assignment_to_static_local {
+
+RefCountable *provide_ref_cntbl();
+void someFunction();
+
+void foo() {
+  static RefCountable* a = nullptr;
+  // expected-warning at -1{{Static local variable 'a' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+  a = provide_ref_cntbl();
+  someFunction();
+  a->method();
+}
+
+} // namespace local_assignment_to_static_local
+
+namespace local_assignment_to_global {
+
+RefCountable *provide_ref_cntbl();
+void someFunction();
+
+RefCountable* g_a = nullptr;
+// expected-warning at -1{{Global variable 'local_assignment_to_global::g_a' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+
+void foo() {
+  g_a = provide_ref_cntbl();
+  someFunction();
+  g_a->method();
+}
+
+} // namespace local_assignment_to_global