xazax.hun created this revision.
xazax.hun added reviewers: NoQ, dcoughlin, george.karpenkov.
Herald added subscribers: dkrupp, a.sidorin, rnkovacs, szepet,
baloghadamsoftware, whisperity.
Unfortunately, currently, the analyzer core sets the checker name after the
constructor was already run. So if we set the BugType in the constructor, the
output plist will not contain a checker name. Right now the idiomatic solution
is to create the BugType lazily. This patch moves the lazy initialization from
the checker side to the analyzer core. I also found some bugs, when some
checkers were triggered even if they were not enabled or wrong checker name is
emitted to the plist.
In the future probably it would be better to alter the signature of the
checkers' constructor to set the name in the constructor so it is possible to
create the BugType eagerly.
This patch is an alternative approach to https://reviews.llvm.org/D37437.
Repository:
rC Clang
https://reviews.llvm.org/D41538
Files:
include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
lib/StaticAnalyzer/Checkers/ValistChecker.cpp
test/Analysis/malloc.c
Index: test/Analysis/malloc.c
===================================================================
--- test/Analysis/malloc.c
+++ test/Analysis/malloc.c
@@ -1720,13 +1720,6 @@
}
}
-char *dupstrWarn(const char *s) {
- const int len = strlen(s);
- char *p = (char*) smallocWarn(len + 1);
- strcpy(p, s); // expected-warning{{String copy function overflows destination buffer}}
- return p;
-}
-
int *radar15580979() {
int *data = (int *)malloc(32);
int *p = data ?: (int*)malloc(32); // no warning
Index: lib/StaticAnalyzer/Checkers/ValistChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/ValistChecker.cpp
+++ lib/StaticAnalyzer/Checkers/ValistChecker.cpp
@@ -64,7 +64,7 @@
CheckerContext &C) const;
void reportLeakedVALists(const RegionVector &LeakedVALists, StringRef Msg1,
StringRef Msg2, CheckerContext &C, ExplodedNode *N,
- bool ForceReport = false) const;
+ bool ReportUninit = false) const;
void checkVAListStartCall(const CallEvent &Call, CheckerContext &C,
bool IsCopy) const;
@@ -267,15 +267,15 @@
void ValistChecker::reportLeakedVALists(const RegionVector &LeakedVALists,
StringRef Msg1, StringRef Msg2,
CheckerContext &C, ExplodedNode *N,
- bool ForceReport) const {
+ bool ReportUninit) const {
if (!(ChecksEnabled[CK_Unterminated] ||
- (ChecksEnabled[CK_Uninitialized] && ForceReport)))
+ (ChecksEnabled[CK_Uninitialized] && ReportUninit)))
return;
for (auto Reg : LeakedVALists) {
if (!BT_leakedvalist) {
- BT_leakedvalist.reset(new BugType(CheckNames[CK_Unterminated],
- "Leaked va_list",
- categories::MemoryError));
+ BT_leakedvalist.reset(new BugType(
+ CheckNames[ReportUninit ? CK_Uninitialized : CK_Unterminated],
+ "Leaked va_list", categories::MemoryError));
BT_leakedvalist->setSuppressOnSink(true);
}
@@ -375,7 +375,7 @@
std::shared_ptr<PathDiagnosticPiece> ValistChecker::ValistBugVisitor::VisitNode(
const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext &BRC,
- BugReport &BR) {
+ BugReport &) {
ProgramStateRef State = N->getState();
ProgramStateRef StatePrev = PrevN->getState();
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2900,8 +2900,13 @@
mgr.getCurrentCheckName();
// We currently treat NewDeleteLeaks checker as a subchecker of NewDelete
// checker.
- if (!checker->ChecksEnabled[MallocChecker::CK_NewDeleteChecker])
+ if (!checker->ChecksEnabled[MallocChecker::CK_NewDeleteChecker]) {
checker->ChecksEnabled[MallocChecker::CK_NewDeleteChecker] = true;
+ // FIXME: This does not set the correct name, but without this workaround
+ // no name will be set at all.
+ checker->CheckNames[MallocChecker::CK_NewDeleteChecker] =
+ mgr.getCurrentCheckName();
+ }
}
#define REGISTER_CHECKER(name) \
Index: include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
===================================================================
--- include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
+++ include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
@@ -29,30 +29,42 @@
class BugType {
private:
- const CheckName Check;
+ mutable CheckName Check;
const std::string Name;
const std::string Category;
- bool SuppressonSink;
+ const CheckerBase *Checker;
+ bool SuppressOnSink;
virtual void anchor();
public:
- BugType(class CheckName check, StringRef name, StringRef cat)
- : Check(check), Name(name), Category(cat), SuppressonSink(false) {}
- BugType(const CheckerBase *checker, StringRef name, StringRef cat)
- : Check(checker->getCheckName()), Name(name), Category(cat),
- SuppressonSink(false) {}
- virtual ~BugType() {}
-
- // FIXME: Should these be made strings as well?
- StringRef getName() const { return Name; }
+ BugType(class CheckName Check, StringRef Name, StringRef Cat)
+ : Check(Check), Name(Name), Category(Cat), Checker(nullptr),
+ SuppressOnSink(false) {}
+ BugType(const CheckerBase *Checker, StringRef Name, StringRef Cat)
+ : Check(Checker->getCheckName()), Name(Name), Category(Cat),
+ Checker(Checker), SuppressOnSink(false) {}
+ virtual ~BugType() = default;
+
+ StringRef getName() const {
+ // FIXME: This is a workaround to ensure that Check is initialized
+ // correctly. The name of the checks are set after checker the constructor
+ // is run. In case the BugType object is initialized in the checker's ctor
+ // the check name will be empty. In this case we fall back to lazy
+ // initialization.
+ if (getCheckName().empty() && Checker) {
+ Check = Checker->getCheckName();
+ }
+ assert(!getCheckName().empty() && "Check name is not set properly.");
+ return Name;
+ }
StringRef getCategory() const { return Category; }
StringRef getCheckName() const { return Check.getName(); }
/// isSuppressOnSink - Returns true if bug reports associated with this bug
/// type should be suppressed if the end node of the report is post-dominated
/// by a sink node.
- bool isSuppressOnSink() const { return SuppressonSink; }
- void setSuppressOnSink(bool x) { SuppressonSink = x; }
+ bool isSuppressOnSink() const { return SuppressOnSink; }
+ void setSuppressOnSink(bool x) { SuppressOnSink = x; }
virtual void FlushReports(BugReporter& BR);
};
@@ -74,7 +86,7 @@
StringRef getDescription() const { return desc; }
};
-} // end GR namespace
+} // end ento namespace
} // end clang namespace
#endif
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
