r330589 - [analyzer] CStringChecker.cpp - Code refactoring on bug report.
Henry Wong via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 23 06:36:51 PDT 2018
Author: henrywong
Date: Mon Apr 23 06:36:51 2018
New Revision: 330589
URL: http://llvm.org/viewvc/llvm-project?rev=330589&view=rev
Log:
[analyzer] CStringChecker.cpp - Code refactoring on bug report.
Reviewers: NoQ, george.karpenkov, xazax.hun
Reviewed By: george.karpenkov
Differential Revision: https://reviews.llvm.org/D44557
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp?rev=330589&r1=330588&r2=330589&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Mon Apr 23 06:36:51 2018
@@ -194,6 +194,14 @@ public:
const Stmt *First,
const Stmt *Second) const;
+ void emitNullArgBug(CheckerContext &C, ProgramStateRef State, const Stmt *S,
+ StringRef WarningMsg) const;
+ void emitOutOfBoundsBug(CheckerContext &C, ProgramStateRef State,
+ const Stmt *S, StringRef WarningMsg) const;
+ void emitNotCStringBug(CheckerContext &C, ProgramStateRef State,
+ const Stmt *S, StringRef WarningMsg) const;
+ void emitAdditionOverflowBug(CheckerContext &C, ProgramStateRef State) const;
+
ProgramStateRef checkAdditionOverflow(CheckerContext &C,
ProgramStateRef state,
NonLoc left,
@@ -239,30 +247,14 @@ ProgramStateRef CStringChecker::checkNon
std::tie(stateNull, stateNonNull) = assumeZero(C, state, l, S->getType());
if (stateNull && !stateNonNull) {
- if (!Filter.CheckCStringNullArg)
- return nullptr;
-
- ExplodedNode *N = C.generateErrorNode(stateNull);
- if (!N)
- return nullptr;
-
- if (!BT_Null)
- BT_Null.reset(new BuiltinBug(
- Filter.CheckNameCStringNullArg, categories::UnixAPI,
- "Null pointer argument in call to byte string function"));
+ if (Filter.CheckCStringNullArg) {
+ SmallString<80> buf;
+ llvm::raw_svector_ostream os(buf);
+ assert(CurrentFunctionDescription);
+ os << "Null pointer argument in call to " << CurrentFunctionDescription;
- SmallString<80> buf;
- llvm::raw_svector_ostream os(buf);
- assert(CurrentFunctionDescription);
- os << "Null pointer argument in call to " << CurrentFunctionDescription;
-
- // Generate a report for this bug.
- BuiltinBug *BT = static_cast<BuiltinBug*>(BT_Null.get());
- auto report = llvm::make_unique<BugReport>(*BT, os.str(), N);
-
- report->addRange(S->getSourceRange());
- bugreporter::trackNullOrUndefValue(N, S, *report);
- C.emitReport(std::move(report));
+ emitNullArgBug(C, stateNull, S, os.str());
+ }
return nullptr;
}
@@ -305,31 +297,14 @@ ProgramStateRef CStringChecker::CheckLoc
ProgramStateRef StInBound = state->assumeInBound(Idx, Size, true);
ProgramStateRef StOutBound = state->assumeInBound(Idx, Size, false);
if (StOutBound && !StInBound) {
- ExplodedNode *N = C.generateErrorNode(StOutBound);
- if (!N)
- return nullptr;
-
- CheckName Name;
// These checks are either enabled by the CString out-of-bounds checker
// explicitly or the "basic" CStringNullArg checker support that Malloc
// checker enables.
assert(Filter.CheckCStringOutOfBounds || Filter.CheckCStringNullArg);
- if (Filter.CheckCStringOutOfBounds)
- Name = Filter.CheckNameCStringOutOfBounds;
- else
- Name = Filter.CheckNameCStringNullArg;
- if (!BT_Bounds) {
- BT_Bounds.reset(new BuiltinBug(
- Name, "Out-of-bound array access",
- "Byte string function accesses out-of-bound array element"));
- }
- BuiltinBug *BT = static_cast<BuiltinBug*>(BT_Bounds.get());
-
- // Generate a report for this bug.
- std::unique_ptr<BugReport> report;
+ // Emit a bug report.
if (warningMsg) {
- report = llvm::make_unique<BugReport>(*BT, warningMsg, N);
+ emitOutOfBoundsBug(C, StOutBound, S, warningMsg);
} else {
assert(CurrentFunctionDescription);
assert(CurrentFunctionDescription[0] != '\0');
@@ -339,15 +314,8 @@ ProgramStateRef CStringChecker::CheckLoc
os << toUppercase(CurrentFunctionDescription[0])
<< &CurrentFunctionDescription[1]
<< " accesses out-of-bound array element";
- report = llvm::make_unique<BugReport>(*BT, os.str(), N);
+ emitOutOfBoundsBug(C, StOutBound, S, os.str());
}
-
- // FIXME: It would be nice to eventually make this diagnostic more clear,
- // e.g., by referencing the original declaration or by saying *why* this
- // reference is outside the range.
-
- report->addRange(S->getSourceRange());
- C.emitReport(std::move(report));
return nullptr;
}
@@ -567,6 +535,79 @@ void CStringChecker::emitOverlapBug(Chec
C.emitReport(std::move(report));
}
+void CStringChecker::emitNullArgBug(CheckerContext &C, ProgramStateRef State,
+ const Stmt *S, StringRef WarningMsg) const {
+ if (ExplodedNode *N = C.generateErrorNode(State)) {
+ if (!BT_Null)
+ BT_Null.reset(new BuiltinBug(
+ Filter.CheckNameCStringNullArg, categories::UnixAPI,
+ "Null pointer argument in call to byte string function"));
+
+ BuiltinBug *BT = static_cast<BuiltinBug *>(BT_Null.get());
+ auto Report = llvm::make_unique<BugReport>(*BT, WarningMsg, N);
+ bugreporter::trackNullOrUndefValue(N, S, *Report);
+ C.emitReport(std::move(Report));
+ }
+}
+
+void CStringChecker::emitOutOfBoundsBug(CheckerContext &C,
+ ProgramStateRef State, const Stmt *S,
+ StringRef WarningMsg) const {
+ if (ExplodedNode *N = C.generateErrorNode(State)) {
+ if (!BT_Bounds)
+ BT_Bounds.reset(new BuiltinBug(
+ Filter.CheckCStringOutOfBounds ? Filter.CheckNameCStringOutOfBounds
+ : Filter.CheckNameCStringNullArg,
+ "Out-of-bound array access",
+ "Byte string function accesses out-of-bound array element"));
+
+ BuiltinBug *BT = static_cast<BuiltinBug *>(BT_Bounds.get());
+
+ // FIXME: It would be nice to eventually make this diagnostic more clear,
+ // e.g., by referencing the original declaration or by saying *why* this
+ // reference is outside the range.
+ auto Report = llvm::make_unique<BugReport>(*BT, WarningMsg, N);
+ Report->addRange(S->getSourceRange());
+ C.emitReport(std::move(Report));
+ }
+}
+
+void CStringChecker::emitNotCStringBug(CheckerContext &C, ProgramStateRef State,
+ const Stmt *S,
+ StringRef WarningMsg) const {
+ if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) {
+ if (!BT_NotCString)
+ BT_NotCString.reset(new BuiltinBug(
+ Filter.CheckNameCStringNotNullTerm, categories::UnixAPI,
+ "Argument is not a null-terminated string."));
+
+ auto Report = llvm::make_unique<BugReport>(*BT_NotCString, WarningMsg, N);
+
+ Report->addRange(S->getSourceRange());
+ C.emitReport(std::move(Report));
+ }
+}
+
+void CStringChecker::emitAdditionOverflowBug(CheckerContext &C,
+ ProgramStateRef State) const {
+ if (ExplodedNode *N = C.generateErrorNode(State)) {
+ if (!BT_NotCString)
+ BT_NotCString.reset(
+ new BuiltinBug(Filter.CheckNameCStringOutOfBounds, "API",
+ "Sum of expressions causes overflow."));
+
+ // This isn't a great error message, but this should never occur in real
+ // code anyway -- you'd have to create a buffer longer than a size_t can
+ // represent, which is sort of a contradiction.
+ const char *WarningMsg =
+ "This expression will create a string whose length is too big to "
+ "be represented as a size_t";
+
+ auto Report = llvm::make_unique<BugReport>(*BT_NotCString, WarningMsg, N);
+ C.emitReport(std::move(Report));
+ }
+}
+
ProgramStateRef CStringChecker::checkAdditionOverflow(CheckerContext &C,
ProgramStateRef state,
NonLoc left,
@@ -610,26 +651,7 @@ ProgramStateRef CStringChecker::checkAdd
if (stateOverflow && !stateOkay) {
// We have an overflow. Emit a bug report.
- ExplodedNode *N = C.generateErrorNode(stateOverflow);
- if (!N)
- return nullptr;
-
- if (!BT_AdditionOverflow)
- BT_AdditionOverflow.reset(
- new BuiltinBug(Filter.CheckNameCStringOutOfBounds, "API",
- "Sum of expressions causes overflow"));
-
- // This isn't a great error message, but this should never occur in real
- // code anyway -- you'd have to create a buffer longer than a size_t can
- // represent, which is sort of a contradiction.
- const char *warning =
- "This expression will create a string whose length is too big to "
- "be represented as a size_t";
-
- // Generate a report for this bug.
- C.emitReport(
- llvm::make_unique<BugReport>(*BT_AdditionOverflow, warning, N));
-
+ emitAdditionOverflowBug(C, stateOverflow);
return nullptr;
}
@@ -729,15 +751,7 @@ SVal CStringChecker::getCStringLength(Ch
// C string. In the context of locations, the only time we can issue such
// a warning is for labels.
if (Optional<loc::GotoLabel> Label = Buf.getAs<loc::GotoLabel>()) {
- if (!Filter.CheckCStringNotNullTerm)
- return UndefinedVal();
-
- if (ExplodedNode *N = C.generateNonFatalErrorNode(state)) {
- if (!BT_NotCString)
- BT_NotCString.reset(new BuiltinBug(
- Filter.CheckNameCStringNotNullTerm, categories::UnixAPI,
- "Argument is not a null-terminated string."));
-
+ if (Filter.CheckCStringNotNullTerm) {
SmallString<120> buf;
llvm::raw_svector_ostream os(buf);
assert(CurrentFunctionDescription);
@@ -745,14 +759,9 @@ SVal CStringChecker::getCStringLength(Ch
<< " is the address of the label '" << Label->getLabel()->getName()
<< "', which is not a null-terminated string";
- // Generate a report for this bug.
- auto report = llvm::make_unique<BugReport>(*BT_NotCString, os.str(), N);
-
- report->addRange(Ex->getSourceRange());
- C.emitReport(std::move(report));
+ emitNotCStringBug(C, state, Ex, os.str());
}
return UndefinedVal();
-
}
// If it's not a region and not a label, give up.
@@ -789,15 +798,7 @@ SVal CStringChecker::getCStringLength(Ch
// Other regions (mostly non-data) can't have a reliable C string length.
// In this case, an error is emitted and UndefinedVal is returned.
// The caller should always be prepared to handle this case.
- if (!Filter.CheckCStringNotNullTerm)
- return UndefinedVal();
-
- if (ExplodedNode *N = C.generateNonFatalErrorNode(state)) {
- if (!BT_NotCString)
- BT_NotCString.reset(new BuiltinBug(
- Filter.CheckNameCStringNotNullTerm, categories::UnixAPI,
- "Argument is not a null-terminated string."));
-
+ if (Filter.CheckCStringNotNullTerm) {
SmallString<120> buf;
llvm::raw_svector_ostream os(buf);
@@ -809,13 +810,8 @@ SVal CStringChecker::getCStringLength(Ch
else
os << "not a null-terminated string";
- // Generate a report for this bug.
- auto report = llvm::make_unique<BugReport>(*BT_NotCString, os.str(), N);
-
- report->addRange(Ex->getSourceRange());
- C.emitReport(std::move(report));
+ emitNotCStringBug(C, state, Ex, os.str());
}
-
return UndefinedVal();
}
}
More information about the cfe-commits
mailing list