Skip to content

WebKit Checkers should set DeclWithIssue. #109389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 27, 2024
Merged

WebKit Checkers should set DeclWithIssue. #109389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b8f95b5
WebKit Checkers should set DeclWithIssue.
rniwa Sep 20, 2024
8402527
Fix formatting.
rniwa Sep 20, 2024
e7aa34b
Fix more formatting.
rniwa Sep 20, 2024
b4fe97e
Also handle ObjCMethodDecl
rniwa Sep 27, 2024
6030365
Fix formatting.
rniwa Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 23 additions & 8 deletions clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/Support/SaveAndRestore.h"
#include <optional>

using namespace clang;
Expand All @@ -44,7 +46,11 @@ class UncountedCallArgsChecker
// visit template instantiations or lambda classes. We
// want to visit those, so we make our own RecursiveASTVisitor.
struct LocalVisitor : public RecursiveASTVisitor<LocalVisitor> {
using Base = RecursiveASTVisitor<LocalVisitor>;

const UncountedCallArgsChecker *Checker;
Decl *DeclWithIssue{nullptr};

explicit LocalVisitor(const UncountedCallArgsChecker *Checker)
: Checker(Checker) {
assert(Checker);
Expand All @@ -56,12 +62,18 @@ class UncountedCallArgsChecker
bool TraverseClassTemplateDecl(ClassTemplateDecl *Decl) {
if (isRefType(safeGetName(Decl)))
return true;
return RecursiveASTVisitor<LocalVisitor>::TraverseClassTemplateDecl(
Decl);
return Base::TraverseClassTemplateDecl(Decl);
}

bool TraverseDecl(Decl *D) {
llvm::SaveAndRestore SavedDecl(DeclWithIssue);
if (D && (isa<FunctionDecl>(D) || isa<ObjCMethodDecl>(D)))
DeclWithIssue = D;
return Base::TraverseDecl(D);
}

bool VisitCallExpr(const CallExpr *CE) {
Checker->visitCallExpr(CE);
Checker->visitCallExpr(CE, DeclWithIssue);
return true;
}
};
Expand All @@ -70,7 +82,7 @@ class UncountedCallArgsChecker
visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
}

void visitCallExpr(const CallExpr *CE) const {
void visitCallExpr(const CallExpr *CE, const Decl *D) const {
if (shouldSkipCall(CE))
return;

Expand All @@ -89,7 +101,7 @@ class UncountedCallArgsChecker
QualType ArgType = MemberCallExpr->getObjectType();
std::optional<bool> IsUncounted = isUncounted(ArgType);
if (IsUncounted && *IsUncounted && !isPtrOriginSafe(E))
reportBugOnThis(E);
reportBugOnThis(E, D);
}

for (auto P = F->param_begin();
Expand Down Expand Up @@ -119,7 +131,7 @@ class UncountedCallArgsChecker
if (isPtrOriginSafe(Arg))
continue;

reportBug(Arg, *P);
reportBug(Arg, *P, D);
}
}
}
Expand Down Expand Up @@ -240,7 +252,8 @@ class UncountedCallArgsChecker
ClsName.ends_with("String"));
}

void reportBug(const Expr *CallArg, const ParmVarDecl *Param) const {
void reportBug(const Expr *CallArg, const ParmVarDecl *Param,
const Decl *DeclWithIssue) const {
assert(CallArg);

SmallString<100> Buf;
Expand All @@ -261,10 +274,11 @@ class UncountedCallArgsChecker
PathDiagnosticLocation BSLoc(SrcLocToReport, BR->getSourceManager());
auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
Report->addRange(CallArg->getSourceRange());
Report->setDeclWithIssue(DeclWithIssue);
BR->emitReport(std::move(Report));
}

void reportBugOnThis(const Expr *CallArg) const {
void reportBugOnThis(const Expr *CallArg, const Decl *DeclWithIssue) const {
assert(CallArg);

const SourceLocation SrcLocToReport = CallArg->getSourceRange().getBegin();
Expand All @@ -274,6 +288,7 @@ class UncountedCallArgsChecker
Bug, "Call argument for 'this' parameter is uncounted and unsafe.",
BSLoc);
Report->addRange(CallArg->getSourceRange());
Report->setDeclWithIssue(DeclWithIssue);
BR->emitReport(std::move(Report));
}
};
Expand Down
21 changes: 16 additions & 5 deletions clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ class UncountedLocalVarsChecker
// want to visit those, so we make our own RecursiveASTVisitor.
struct LocalVisitor : public RecursiveASTVisitor<LocalVisitor> {
const UncountedLocalVarsChecker *Checker;
Decl *DeclWithIssue{nullptr};

TrivialFunctionAnalysis TFA;

Expand All @@ -134,18 +135,25 @@ class UncountedLocalVarsChecker
bool shouldVisitTemplateInstantiations() const { return true; }
bool shouldVisitImplicitCode() const { return false; }

bool TraverseDecl(Decl *D) {
llvm::SaveAndRestore SavedDecl(DeclWithIssue);
if (D && (isa<FunctionDecl>(D) || isa<ObjCMethodDecl>(D)))
DeclWithIssue = D;
return Base::TraverseDecl(D);
}

bool VisitVarDecl(VarDecl *V) {
auto *Init = V->getInit();
if (Init && V->isLocalVarDecl())
Checker->visitVarDecl(V, Init);
Checker->visitVarDecl(V, Init, DeclWithIssue);
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());
Checker->visitVarDecl(V, BO->getRHS(), DeclWithIssue);
}
}
return true;
Expand Down Expand Up @@ -186,7 +194,8 @@ class UncountedLocalVarsChecker
visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
}

void visitVarDecl(const VarDecl *V, const Expr *Value) const {
void visitVarDecl(const VarDecl *V, const Expr *Value,
const Decl *DeclWithIssue) const {
if (shouldSkipVarDecl(V))
return;

Expand Down Expand Up @@ -240,7 +249,7 @@ class UncountedLocalVarsChecker
}))
return;

reportBug(V, Value);
reportBug(V, Value, DeclWithIssue);
}
}

Expand All @@ -249,7 +258,8 @@ class UncountedLocalVarsChecker
return BR->getSourceManager().isInSystemHeader(V->getLocation());
}

void reportBug(const VarDecl *V, const Expr *Value) const {
void reportBug(const VarDecl *V, const Expr *Value,
const Decl *DeclWithIssue) const {
assert(V);
SmallString<100> Buf;
llvm::raw_svector_ostream Os(Buf);
Expand Down Expand Up @@ -278,6 +288,7 @@ class UncountedLocalVarsChecker
PathDiagnosticLocation BSLoc(V->getLocation(), BR->getSourceManager());
auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
Report->addRange(V->getSourceRange());
Report->setDeclWithIssue(DeclWithIssue);
BR->emitReport(std::move(Report));
}
}
Expand Down
Loading