Skip to content

[clang][Analyzer] Fix error path of builtin overflow #136345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 20, 2025
Merged

[clang][Analyzer] Fix error path of builtin overflow #136345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bcc3a8d
[clang][Analyzer] Fix error path of builtin overflow
pskrgag Apr 18, 2025
1acd972
fix formatting
pskrgag Apr 18, 2025
a9ba9d3
address review comments
pskrgag Apr 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 48 additions & 38 deletions clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,14 @@ 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;
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;

Expand All @@ -121,30 +125,24 @@ class BuiltinFunctionChecker : public Checker<eval::Call> {

} // namespace

const NoteTag *BuiltinFunctionChecker::createBuiltinNoOverflowNoteTag(
CheckerContext &C, bool BothFeasible, SVal Arg1, SVal Arg2,
SVal Result) const {
return C.getNoteTag([Result, Arg1, Arg2, BothFeasible](
PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
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) {
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 {
Expand Down Expand Up @@ -174,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,
Expand All @@ -183,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);
Expand All @@ -194,29 +213,20 @@ 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));

if (auto L = Call.getArgSVal(2).getAs<Loc>()) {
StateNoOverflow =
StateNoOverflow->bindLoc(*L, RetVal, C.getLocationContext());

// Propagate taint if any of the argumets were tainted
if (isTainted(State, Arg1) || isTainted(State, Arg2))
StateNoOverflow = addTaint(StateNoOverflow, *L);
}
if (NotOverflow) {
auto NewState =
initStateAftetBuiltinOverflow(C, State, Call, RetVal, false);

C.addTransition(
StateNoOverflow,
createBuiltinNoOverflowNoteTag(
C, /*BothFeasible=*/NotOverflow && Overflow, Arg1, Arg2, RetVal));
C.addTransition(NewState, createBuiltinOverflowNoteTag(
C, /*overflow=*/false, Arg1, Arg2, RetVal));
}

if (Overflow) {
C.addTransition(State->BindExpr(CE, C.getLocationContext(),
SVB.makeTruthVal(true, BoolTy)),
createBuiltinOverflowNoteTag(C));
auto NewState = initStateAftetBuiltinOverflow(C, State, Call, RetVal, true);

C.addTransition(NewState, createBuiltinOverflowNoteTag(C, /*overflow=*/true,
Arg1, Arg2, RetVal));
}
}

Expand Down
6 changes: 3 additions & 3 deletions clang/test/Analysis/builtin_overflow.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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
}
10 changes: 7 additions & 3 deletions clang/test/Analysis/builtin_overflow_notes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Loading