[clang][Analyzer] Fix error path of builtin overflow #136345
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-static-analyzer-1 @llvm/pr-subscribers-clang Author: Pavel Skripkin (pskrgag) ChangesAccording 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 Full diff: https://github.com/llvm/llvm-project/pull/136345.diff 3 Files Affected:
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;
}
}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
steakhal
requested changes
Apr 20, 2025
I just feel bad when introduce bugs and want to fix them asap. Thanks for review! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/174/builds/16498 Here is the relevant piece of the build log for the reference |
pskrgag
added a commit
to pskrgag/llvm-project
that referenced
this pull request
Apr 21, 2025
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 llvm#136292
steakhal
pushed a commit
to pskrgag/llvm-project
that referenced
this pull request
Apr 23, 2025
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 llvm#136292 (cherry picked from commit 060f955)
swift-ci
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Apr 25, 2025
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 llvm#136292 (cherry picked from commit 060f955)
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
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 llvm#136292
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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