https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/92639
>From 5ae3b193a6ec3617c99297da5b8d276b0e5bbc01 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@apple.com>
Date: Sat, 18 May 2024 02:17:30 -0700
Subject: [PATCH 1/2] [alpha.webkit.UncountedLocalVarsChecker] Detect
assignments to uncounted local variable and parameters.
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.
---
.../WebKit/UncountedLocalVarsChecker.cpp | 61 +++++++++++--------
.../Checkers/WebKit/uncounted-local-vars.cpp | 42 +++++++++++++
2 files changed, 79 insertions(+), 24 deletions(-)
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@-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@-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@-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@-1{{Assignment to an uncounted parameter 'a' is unsafe
[alpha.webkit.UncountedLocalVarsChecker]}}
+ someFunction();
+ a->method();
+}
+
+} // namespace local_assignment_to_parameter
>From fa69297aaa801bbc0c721fa3a2a8ead5b1b7ee79 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@apple.com>
Date: Thu, 23 May 2024 22:16:13 -0700
Subject: [PATCH 2/2] Add tests for assigning to a static local variable and a
global variable, and correct the warning to refer to those variables
appropriately instead of erroneously calling all of them as local variable.
---
.../WebKit/UncountedLocalVarsChecker.cpp | 9 +++-
.../Checkers/WebKit/uncounted-local-vars.cpp | 45 ++++++++++++++++---
2 files changed, 46 insertions(+), 8 deletions(-)
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
index 6c0d56303d5ad..274da0baf2ce5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
@@ -264,7 +264,14 @@ class UncountedLocalVarsChecker
Report->addRange(Value->getSourceRange());
BR->emitReport(std::move(Report));
} else {
- Os << "Local variable ";
+ 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.";
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
index abcb9c5122f70..25776870dd3ae 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
@@ -219,27 +219,27 @@ void foo() {
namespace local_assignment_basic {
-RefCountable *provide_ref_ctnbl();
+RefCountable *provide_ref_cntbl();
void foo(RefCountable* a) {
RefCountable* b = a;
// expected-warning@-1{{Local variable 'b' is uncounted and unsafe
[alpha.webkit.UncountedLocalVarsChecker]}}
if (b->trivial())
- b = provide_ref_ctnbl();
+ b = provide_ref_cntbl();
}
void bar(RefCountable* a) {
RefCountable* b;
// expected-warning@-1{{Local variable 'b' is uncounted and unsafe
[alpha.webkit.UncountedLocalVarsChecker]}}
- b = provide_ref_ctnbl();
+ b = provide_ref_cntbl();
}
void baz() {
- RefPtr a = provide_ref_ctnbl();
+ RefPtr a = provide_ref_cntbl();
{
RefCountable* b = a.get();
// expected-warning@-1{{Local variable 'b' is uncounted and unsafe
[alpha.webkit.UncountedLocalVarsChecker]}}
- b = provide_ref_ctnbl();
+ b = provide_ref_cntbl();
}
}
@@ -247,14 +247,45 @@ void baz() {
namespace local_assignment_to_parameter {
-RefCountable *provide_ref_ctnbl();
+RefCountable *provide_ref_cntbl();
void someFunction();
void foo(RefCountable* a) {
- a = provide_ref_ctnbl();
+ a = provide_ref_cntbl();
// expected-warning@-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@-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@-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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
