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@-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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits