Skip to content

[analyzer] Unbreak [[clang::suppress]] on checkers without decl-with-issue. #79398

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
Show file tree
Hide file tree
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
Expand Up @@ -19,6 +19,7 @@
#include "llvm/ADT/SmallVector.h"

namespace clang {
class ASTContext;
class Decl;

namespace ento {
Expand All @@ -27,6 +28,8 @@ class PathDiagnosticLocation;

class BugSuppression {
public:
explicit BugSuppression(const ASTContext &ACtx) : ACtx(ACtx) {}

using DiagnosticIdentifierList = llvm::ArrayRef<llvm::StringRef>;

/// Return true if the given bug report was explicitly suppressed by the user.
Expand All @@ -45,6 +48,8 @@ class BugSuppression {
llvm::SmallVector<SourceRange, EXPECTED_NUMBER_OF_SUPPRESSIONS>;

llvm::DenseMap<const Decl *, CachedRanges> CachedSuppressionLocations;

const ASTContext &ACtx;
};

} // end namespace ento
Expand Down
4 changes: 3 additions & 1 deletion clang/lib/StaticAnalyzer/Core/BugReporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2467,7 +2467,9 @@ ProgramStateManager &PathSensitiveBugReporter::getStateManager() const {
return Eng.getStateManager();
}

BugReporter::BugReporter(BugReporterData &d) : D(d) {}
BugReporter::BugReporter(BugReporterData &D)
: D(D), UserSuppressions(D.getASTContext()) {}

BugReporter::~BugReporter() {
// Make sure reports are flushed.
assert(StrBugTypes.empty() &&
Expand Down
10 changes: 9 additions & 1 deletion clang/lib/StaticAnalyzer/Core/BugSuppression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,17 @@ bool BugSuppression::isSuppressed(const BugReport &R) {
bool BugSuppression::isSuppressed(const PathDiagnosticLocation &Location,
const Decl *DeclWithIssue,
DiagnosticIdentifierList Hashtags) {
if (!Location.isValid() || DeclWithIssue == nullptr)
if (!Location.isValid())
return false;

if (!DeclWithIssue) {
// FIXME: This defeats the purpose of passing DeclWithIssue to begin with.
// If this branch is ever hit, we're re-doing all the work we've already
// done as well as perform a lot of work we'll never need.
// Gladly, none of our on-by-default checkers currently need it.
DeclWithIssue = ACtx.getTranslationUnitDecl();
}

// While some warnings are attached to AST nodes (mostly path-sensitive
// checks), others are simply associated with a plain source location
// or range. Figuring out the node based on locations can be tricky,
Expand Down
6 changes: 6 additions & 0 deletions clang/test/Analysis/Checkers/WebKit/call-args.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ namespace simple {
consume_refcntbl(provide());
// expected-warning@-1{{Call argument is uncounted and unsafe}}
}

// Test that the checker works with [[clang::suppress]].
void foo_suppressed() {
[[clang::suppress]]
consume_refcntbl(provide()); // no-warning
}
}

namespace multi_arg {
Expand Down
24 changes: 24 additions & 0 deletions clang/test/Analysis/copypaste/suspicious-clones.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,30 @@ int maxClone(int x, int y, int z) {
return z; // expected-warning{{Potential copy-paste error; did you really mean to use 'z' here?}}
}

// Test that the checker works with [[clang::suppress]].
int max_suppressed(int a, int b) {
log();
if (a > b)
return a;

// This [[clang::suppress]] doesn't suppress anything but we need it here
// because otherwise the other function won't count as a perfect clone.
// FIXME: The checker should probably skip the attribute entirely
// when detecting clones. Otherwise warnings will still get suppressed,
// but for a completely wrong reason.
[[clang::suppress]]
return b; // no-note
}

int maxClone_suppressed(int x, int y, int z) {
log();
if (x > y)
return x;
[[clang::suppress]]
return z; // no-warning
}


// Tests finding a suspicious clone that references global variables.

struct mutex {
Expand Down