Skip to content

Commit

Permalink
[alpha.webkit.UncountedLocalVarsChecker] Detect assignments to uncoun…
Browse files Browse the repository at this point in the history
…ted 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.
  • Loading branch information
rniwa authored May 24, 2024
1 parent 04c94ba commit b80d982
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down
73 changes: 73 additions & 0 deletions clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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@-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@-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@-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@-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

0 comments on commit b80d982

Please sign in to comment.