https://github.com/pskrgag updated
https://github.com/llvm/llvm-project/pull/136345
>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 1/3] [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;
}
}
>From 1acd9727b1a076467cdfcb2761a02e7bb2784eef Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskrip...@gmail.com>
Date: Fri, 18 Apr 2025 22:06:59 +0300
Subject: [PATCH 2/3] fix formatting
---
.../Checkers/BuiltinFunctionChecker.cpp | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index a2f6172ca4056..4dd692b5f95e9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -121,10 +121,9 @@ class BuiltinFunctionChecker : public Checker<eval::Call> {
} // namespace
const NoteTag *BuiltinFunctionChecker::createBuiltinOverflowNoteTag(
- CheckerContext &C, bool overflow, SVal Arg1, SVal Arg2,
- SVal Result) const {
- return C.getNoteTag([Result, Arg1, Arg2, overflow](
- PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
+ CheckerContext &C, bool overflow, SVal Arg1, SVal Arg2, SVal Result) const
{
+ return C.getNoteTag([Result, Arg1, Arg2, overflow](PathSensitiveBugReport
&BR,
+ llvm::raw_ostream &OS) {
if (!BR.isInteresting(Result))
return;
@@ -200,10 +199,9 @@ void BuiltinFunctionChecker::handleOverflowBuiltin(const
CallEvent &Call,
NewState = addTaint(NewState, *L);
}
- C.addTransition(
- NewState,
- createBuiltinOverflowNoteTag(
- C, /*overflow=*/isOverflow, Arg1, Arg2, RetVal));
+ C.addTransition(NewState,
+ createBuiltinOverflowNoteTag(C, /*overflow=*/isOverflow,
+ Arg1, Arg2, RetVal));
};
if (NotOverflow)
>From a9ba9d387a8a37259233ec5c30be6f829bb72669 Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskrip...@gmail.com>
Date: Sun, 20 Apr 2025 16:10:17 +0300
Subject: [PATCH 3/3] address review comments
---
.../Checkers/BuiltinFunctionChecker.cpp | 59 ++++++++++++-------
1 file changed, 39 insertions(+), 20 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index 4dd692b5f95e9..8eb68b1fa638e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -99,6 +99,11 @@ class BuiltinFunctionChecker : public Checker<eval::Call> {
const NoteTag *createBuiltinOverflowNoteTag(CheckerContext &C,
bool BothFeasible, SVal Arg1,
SVal Arg2, SVal Result) const;
+ ProgramStateRef initStateAftetBuiltinOverflow(CheckerContext &C,
+ ProgramStateRef State,
+ const CallEvent &Call,
+ SVal RetCal,
+ bool IsOverflow) const;
std::pair<bool, bool> checkOverflow(CheckerContext &C, SVal RetVal,
QualType Res) const;
@@ -167,6 +172,29 @@ BuiltinFunctionChecker::checkOverflow(CheckerContext &C,
SVal RetVal,
return {MayOverflow || MayUnderflow, MayNotOverflow && MayNotUnderflow};
}
+ProgramStateRef BuiltinFunctionChecker::initStateAftetBuiltinOverflow(
+ CheckerContext &C, ProgramStateRef State, const CallEvent &Call,
+ SVal RetVal, bool IsOverflow) const {
+ SValBuilder &SVB = C.getSValBuilder();
+ SVal Arg1 = Call.getArgSVal(0);
+ SVal Arg2 = Call.getArgSVal(1);
+ auto BoolTy = C.getASTContext().BoolTy;
+
+ ProgramStateRef NewState =
+ State->BindExpr(Call.getOriginExpr(), C.getLocationContext(),
+ SVB.makeTruthVal(IsOverflow, BoolTy));
+
+ if (auto L = Call.getArgSVal(2).getAs<Loc>()) {
+ NewState = NewState->bindLoc(*L, RetVal, C.getLocationContext());
+
+ // Propagate taint if any of the arguments were tainted
+ if (isTainted(State, Arg1) || isTainted(State, Arg2))
+ NewState = addTaint(NewState, *L);
+ }
+
+ return NewState;
+}
+
void BuiltinFunctionChecker::handleOverflowBuiltin(const CallEvent &Call,
CheckerContext &C,
BinaryOperator::Opcode Op,
@@ -176,8 +204,6 @@ void BuiltinFunctionChecker::handleOverflowBuiltin(const
CallEvent &Call,
ProgramStateRef State = C.getState();
SValBuilder &SVB = C.getSValBuilder();
- const Expr *CE = Call.getOriginExpr();
- auto BoolTy = C.getASTContext().BoolTy;
SVal Arg1 = Call.getArgSVal(0);
SVal Arg2 = Call.getArgSVal(1);
@@ -187,28 +213,21 @@ void BuiltinFunctionChecker::handleOverflowBuiltin(const
CallEvent &Call,
SVal RetVal = SVB.evalBinOp(State, Op, Arg1, Arg2, ResultType);
auto [Overflow, NotOverflow] = checkOverflow(C, RetValMax, ResultType);
- auto initializeState = [&](bool isOverflow) {
- ProgramStateRef NewState = State->BindExpr(
- CE, C.getLocationContext(), SVB.makeTruthVal(isOverflow, BoolTy));
- if (auto L = Call.getArgSVal(2).getAs<Loc>()) {
- NewState = NewState->bindLoc(*L, RetVal, C.getLocationContext());
+ if (NotOverflow) {
+ auto NewState =
+ initStateAftetBuiltinOverflow(C, State, Call, RetVal, false);
- // Propagate taint if any of the arguments were tainted
- if (isTainted(State, Arg1) || isTainted(State, Arg2))
- NewState = addTaint(NewState, *L);
- }
-
- C.addTransition(NewState,
- createBuiltinOverflowNoteTag(C, /*overflow=*/isOverflow,
- Arg1, Arg2, RetVal));
- };
+ C.addTransition(NewState, createBuiltinOverflowNoteTag(
+ C, /*overflow=*/false, Arg1, Arg2, RetVal));
+ }
- if (NotOverflow)
- initializeState(false);
+ if (Overflow) {
+ auto NewState = initStateAftetBuiltinOverflow(C, State, Call, RetVal,
true);
- if (Overflow)
- initializeState(true);
+ C.addTransition(NewState, createBuiltinOverflowNoteTag(C,
/*overflow=*/true,
+ Arg1, Arg2,
RetVal));
+ }
}
bool BuiltinFunctionChecker::isBuiltinLikeFunction(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
