https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/152163
From 6b9489e246333eda92b3aa67f87096ad0a17bfd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com> Date: Tue, 5 Aug 2025 16:46:49 +0200 Subject: [PATCH] [analyzer] Remove impossible BugType from CStringChecker 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). --- .../Checkers/CStringChecker.cpp | 40 +++++-------------- 1 file changed, 11 insertions(+), 29 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index fd0a3982bf461..0e5fc0a074938 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -97,10 +97,6 @@ class CStringChecker CheckerFrontendWithBugType UninitializedRead{ "Accessing unitialized/garbage values"}; - // FIXME: This bug type should be removed because it is only emitted in a - // situation that is practically impossible. - const BugType AdditionOverflow{&OutOfBounds, "API"}; - StringRef getDebugTag() const override { return "MallocChecker"; } static void *getTag() { static int tag; return &tag; } @@ -330,7 +326,6 @@ class CStringChecker 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; void emitUninitializedReadBug(CheckerContext &C, ProgramStateRef State, const Expr *E, const MemRegion *R, StringRef Msg) const; @@ -843,22 +838,6 @@ void CStringChecker::emitNotCStringBug(CheckerContext &C, ProgramStateRef State, } } -void CStringChecker::emitAdditionOverflowBug(CheckerContext &C, - ProgramStateRef State) const { - if (ExplodedNode *N = C.generateErrorNode(State)) { - // 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 = std::make_unique<PathSensitiveBugReport>(AdditionOverflow, - WarningMsg, N); - C.emitReport(std::move(Report)); - } -} - ProgramStateRef CStringChecker::checkAdditionOverflow(CheckerContext &C, ProgramStateRef state, NonLoc left, @@ -896,19 +875,22 @@ ProgramStateRef CStringChecker::checkAdditionOverflow(CheckerContext &C, SVal willOverflow = svalBuilder.evalBinOpNN(state, BO_GT, left, *maxMinusRightNL, cmpTy); - ProgramStateRef stateOverflow, stateOkay; - std::tie(stateOverflow, stateOkay) = - state->assume(willOverflow.castAs<DefinedOrUnknownSVal>()); + auto [StateOverflow, StateOkay] = + state->assume(willOverflow.castAs<DefinedOrUnknownSVal>()); - if (stateOverflow && !stateOkay) { - // We have an overflow. Emit a bug report. - emitAdditionOverflowBug(C, stateOverflow); + if (StateOverflow && !StateOkay) { + // On this path the analyzer is convinced that the addition of these two + // values would overflow `size_t` which must be caused by the inaccuracy + // of our modeling because this method is called in situations where the + // summands are size/length values which are much less than SIZE_MAX. To + // avoid false positives let's just sink this invalid path. + C.addSink(StateOverflow); return nullptr; } // From now on, assume an overflow didn't occur. - assert(stateOkay); - state = stateOkay; + assert(StateOkay); + state = StateOkay; } return state; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits