Skip to content

Commit 180d162

Browse files
authored
[analyzer] Remove impossible BugType from CStringChecker (#152163)
CStringChecker had an AdditionOverflow bug type which was intended for a situation where the analyzer concludes that the addition of two size/length values overflows `size_t`. I strongly suspect that the analyzer could emit bugs of this type in certain complex corner cases (e.g. due to inaccurate cast modeling), but these reports would be all false positives because in the real world the sum of two size/length values is always far below SIZE_MAX. (Although note that there was no test where the analyzer emitted a bug with this type.) To simplify the code (and perhaps eliminate false positives), I eliminated this bug type and replaced code that emits it by a simple `addSink()` call (because we still want to get rid of the execution paths where the analyzer has invalid assumptions).
1 parent ab40909 commit 180d162

File tree

1 file changed

+11
-29
lines changed

1 file changed

+11
-29
lines changed

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

Lines changed: 11 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,6 @@ class CStringChecker
9797
CheckerFrontendWithBugType UninitializedRead{
9898
"Accessing unitialized/garbage values"};
9999

100-
// FIXME: This bug type should be removed because it is only emitted in a
101-
// situation that is practically impossible.
102-
const BugType AdditionOverflow{&OutOfBounds, "API"};
103-
104100
StringRef getDebugTag() const override { return "MallocChecker"; }
105101

106102
static void *getTag() { static int tag; return &tag; }
@@ -330,7 +326,6 @@ class CStringChecker
330326
const Stmt *S, StringRef WarningMsg) const;
331327
void emitNotCStringBug(CheckerContext &C, ProgramStateRef State,
332328
const Stmt *S, StringRef WarningMsg) const;
333-
void emitAdditionOverflowBug(CheckerContext &C, ProgramStateRef State) const;
334329
void emitUninitializedReadBug(CheckerContext &C, ProgramStateRef State,
335330
const Expr *E, const MemRegion *R,
336331
StringRef Msg) const;
@@ -843,22 +838,6 @@ void CStringChecker::emitNotCStringBug(CheckerContext &C, ProgramStateRef State,
843838
}
844839
}
845840

846-
void CStringChecker::emitAdditionOverflowBug(CheckerContext &C,
847-
ProgramStateRef State) const {
848-
if (ExplodedNode *N = C.generateErrorNode(State)) {
849-
// This isn't a great error message, but this should never occur in real
850-
// code anyway -- you'd have to create a buffer longer than a size_t can
851-
// represent, which is sort of a contradiction.
852-
const char *WarningMsg =
853-
"This expression will create a string whose length is too big to "
854-
"be represented as a size_t";
855-
856-
auto Report = std::make_unique<PathSensitiveBugReport>(AdditionOverflow,
857-
WarningMsg, N);
858-
C.emitReport(std::move(Report));
859-
}
860-
}
861-
862841
ProgramStateRef CStringChecker::checkAdditionOverflow(CheckerContext &C,
863842
ProgramStateRef state,
864843
NonLoc left,
@@ -896,19 +875,22 @@ ProgramStateRef CStringChecker::checkAdditionOverflow(CheckerContext &C,
896875
SVal willOverflow = svalBuilder.evalBinOpNN(state, BO_GT, left,
897876
*maxMinusRightNL, cmpTy);
898877

899-
ProgramStateRef stateOverflow, stateOkay;
900-
std::tie(stateOverflow, stateOkay) =
901-
state->assume(willOverflow.castAs<DefinedOrUnknownSVal>());
878+
auto [StateOverflow, StateOkay] =
879+
state->assume(willOverflow.castAs<DefinedOrUnknownSVal>());
902880

903-
if (stateOverflow && !stateOkay) {
904-
// We have an overflow. Emit a bug report.
905-
emitAdditionOverflowBug(C, stateOverflow);
881+
if (StateOverflow && !StateOkay) {
882+
// On this path the analyzer is convinced that the addition of these two
883+
// values would overflow `size_t` which must be caused by the inaccuracy
884+
// of our modeling because this method is called in situations where the
885+
// summands are size/length values which are much less than SIZE_MAX. To
886+
// avoid false positives let's just sink this invalid path.
887+
C.addSink(StateOverflow);
906888
return nullptr;
907889
}
908890

909891
// From now on, assume an overflow didn't occur.
910-
assert(stateOkay);
911-
state = stateOkay;
892+
assert(StateOkay);
893+
state = StateOkay;
912894
}
913895

914896
return state;

0 commit comments

Comments
 (0)