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.

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
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -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;
@@ -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);
@@ -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;
}
};
@@ -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;

@@ -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();
@@ -119,7 +131,7 @@ class UncountedCallArgsChecker
if (isPtrOriginSafe(Arg))
continue;

reportBug(Arg, *P);
reportBug(Arg, *P, D);
}
}
}
@@ -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;
@@ -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();
@@ -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));
}
};
Original file line number Diff line number Diff line change
@@ -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;

@@ -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;
@@ -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;

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

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

@@ -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);
@@ -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));
}
}
Loading