https://github.com/pskrgag created https://github.com/llvm/llvm-project/pull/136345
According to https://clang.llvm.org/docs/LanguageExtensions.html#checked-arithmetic-builtins, result of builtin_*_overflow functions will be initialized even in case of overflow. Align analyzer logic to docs and always initialize 3rd argument of such builtins. Closes #136292 >From bcc3a8d0654d7a92183fb5b010892740c593e012 Mon Sep 17 00:00:00 2001 From: Pavel Skripkin <paskrip...@gmail.com> Date: Fri, 18 Apr 2025 22:01:02 +0300 Subject: [PATCH] [clang][Analyzer] Fix error path of builtin overflow --- .../Checkers/BuiltinFunctionChecker.cpp | 57 ++++++++----------- clang/test/Analysis/builtin_overflow.c | 6 +- clang/test/Analysis/builtin_overflow_notes.c | 10 +++- 3 files changed, 35 insertions(+), 38 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp index b1a11442dec53..a2f6172ca4056 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp @@ -96,10 +96,9 @@ class BuiltinFunctionChecker : public Checker<eval::Call> { void handleOverflowBuiltin(const CallEvent &Call, CheckerContext &C, BinaryOperator::Opcode Op, QualType ResultType) const; - const NoteTag *createBuiltinNoOverflowNoteTag(CheckerContext &C, - bool BothFeasible, SVal Arg1, - SVal Arg2, SVal Result) const; - const NoteTag *createBuiltinOverflowNoteTag(CheckerContext &C) const; + const NoteTag *createBuiltinOverflowNoteTag(CheckerContext &C, + bool BothFeasible, SVal Arg1, + SVal Arg2, SVal Result) const; std::pair<bool, bool> checkOverflow(CheckerContext &C, SVal RetVal, QualType Res) const; @@ -121,30 +120,25 @@ class BuiltinFunctionChecker : public Checker<eval::Call> { } // namespace -const NoteTag *BuiltinFunctionChecker::createBuiltinNoOverflowNoteTag( - CheckerContext &C, bool BothFeasible, SVal Arg1, SVal Arg2, +const NoteTag *BuiltinFunctionChecker::createBuiltinOverflowNoteTag( + CheckerContext &C, bool overflow, SVal Arg1, SVal Arg2, SVal Result) const { - return C.getNoteTag([Result, Arg1, Arg2, BothFeasible]( + return C.getNoteTag([Result, Arg1, Arg2, overflow]( PathSensitiveBugReport &BR, llvm::raw_ostream &OS) { if (!BR.isInteresting(Result)) return; - // Propagate interestingness to input argumets if result is interesting. + // Propagate interestingness to input arguments if result is interesting. BR.markInteresting(Arg1); BR.markInteresting(Arg2); - if (BothFeasible) + if (overflow) + OS << "Assuming overflow"; + else OS << "Assuming no overflow"; }); } -const NoteTag * -BuiltinFunctionChecker::createBuiltinOverflowNoteTag(CheckerContext &C) const { - return C.getNoteTag([](PathSensitiveBugReport &BR, - llvm::raw_ostream &OS) { OS << "Assuming overflow"; }, - /*isPrunable=*/true); -} - std::pair<bool, bool> BuiltinFunctionChecker::checkOverflow(CheckerContext &C, SVal RetVal, QualType Res) const { @@ -194,30 +188,29 @@ void BuiltinFunctionChecker::handleOverflowBuiltin(const CallEvent &Call, SVal RetVal = SVB.evalBinOp(State, Op, Arg1, Arg2, ResultType); auto [Overflow, NotOverflow] = checkOverflow(C, RetValMax, ResultType); - if (NotOverflow) { - ProgramStateRef StateNoOverflow = State->BindExpr( - CE, C.getLocationContext(), SVB.makeTruthVal(false, BoolTy)); + auto initializeState = [&](bool isOverflow) { + ProgramStateRef NewState = State->BindExpr( + CE, C.getLocationContext(), SVB.makeTruthVal(isOverflow, BoolTy)); if (auto L = Call.getArgSVal(2).getAs<Loc>()) { - StateNoOverflow = - StateNoOverflow->bindLoc(*L, RetVal, C.getLocationContext()); + NewState = NewState->bindLoc(*L, RetVal, C.getLocationContext()); - // Propagate taint if any of the argumets were tainted + // Propagate taint if any of the arguments were tainted if (isTainted(State, Arg1) || isTainted(State, Arg2)) - StateNoOverflow = addTaint(StateNoOverflow, *L); + NewState = addTaint(NewState, *L); } C.addTransition( - StateNoOverflow, - createBuiltinNoOverflowNoteTag( - C, /*BothFeasible=*/NotOverflow && Overflow, Arg1, Arg2, RetVal)); - } + NewState, + createBuiltinOverflowNoteTag( + C, /*overflow=*/isOverflow, Arg1, Arg2, RetVal)); + }; - if (Overflow) { - C.addTransition(State->BindExpr(CE, C.getLocationContext(), - SVB.makeTruthVal(true, BoolTy)), - createBuiltinOverflowNoteTag(C)); - } + if (NotOverflow) + initializeState(false); + + if (Overflow) + initializeState(true); } bool BuiltinFunctionChecker::isBuiltinLikeFunction( diff --git a/clang/test/Analysis/builtin_overflow.c b/clang/test/Analysis/builtin_overflow.c index 9d98ce7a1af45..d290333071dc9 100644 --- a/clang/test/Analysis/builtin_overflow.c +++ b/clang/test/Analysis/builtin_overflow.c @@ -26,7 +26,7 @@ void test_add_overflow(void) int res; if (__builtin_add_overflow(__INT_MAX__, 1, &res)) { - clang_analyzer_dump_int(res); //expected-warning{{1st function call argument is an uninitialized value}} + clang_analyzer_dump_int(res); //expected-warning{{-2147483648 S32b}} return; } @@ -38,7 +38,7 @@ void test_add_underoverflow(void) int res; if (__builtin_add_overflow(__INT_MIN__, -1, &res)) { - clang_analyzer_dump_int(res); //expected-warning{{1st function call argument is an uninitialized value}} + clang_analyzer_dump_int(res); //expected-warning{{2147483647 S32b}} return; } @@ -160,7 +160,7 @@ void test_bool_assign(void) { int res; - // Reproduce issue from GH#111147. __builtin_*_overflow funcions + // Reproduce issue from GH#111147. __builtin_*_overflow functions // should return _Bool, but not int. _Bool ret = __builtin_mul_overflow(10, 20, &res); // no crash } diff --git a/clang/test/Analysis/builtin_overflow_notes.c b/clang/test/Analysis/builtin_overflow_notes.c index 5fa2156df925c..94c79b5ed334a 100644 --- a/clang/test/Analysis/builtin_overflow_notes.c +++ b/clang/test/Analysis/builtin_overflow_notes.c @@ -19,12 +19,16 @@ void test_no_overflow_note(int a, int b) void test_overflow_note(int a, int b) { - int res; // expected-note{{'res' declared without an initial value}} + int res; if (__builtin_add_overflow(a, b, &res)) { // expected-note {{Assuming overflow}} // expected-note@-1 {{Taking true branch}} - int var = res; // expected-warning{{Assigned value is uninitialized}} - // expected-note@-1 {{Assigned value is uninitialized}} + if (res) { // expected-note {{Assuming 'res' is not equal to 0}} + // expected-note@-1 {{Taking true branch}} + int *ptr = 0; // expected-note {{'ptr' initialized to a null pointer value}} + int var = *(int *) ptr; //expected-warning {{Dereference of null pointer}} + //expected-note@-1 {{Dereference of null pointer}} + } return; } } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
