Skip to content

Commit

Permalink
[clang][analyzer] Add function 'ungetc' to StreamChecker. (#77331)
Browse files Browse the repository at this point in the history
`StdLibraryFunctionsChecker` is updated too with `ungetc`.
  • Loading branch information
balazske committed Jan 10, 2024
1 parent b788692 commit 8f78dd4
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 3 deletions.
8 changes: 5 additions & 3 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1202,16 +1202,18 @@ Improvements
(`c3a87ddad62a <https://github.com/llvm/llvm-project/commit/c3a87ddad62a6cc01acaccc76592bc6730c8ac3c>`_,
`0954dc3fb921 <https://github.com/llvm/llvm-project/commit/0954dc3fb9214b994623f5306473de075f8e3593>`_)

- Improved the ``alpha.unix.Stream`` checker by modeling more functions like,
``fflush``, ``fputs``, ``fgetc``, ``fputc``, ``fopen``, ``fdopen``, ``fgets``, ``tmpfile``.
- Improved the ``alpha.unix.Stream`` checker by modeling more functions
``fputs``, ``fputc``, ``fgets``, ``fgetc``, ``fdopen``, ``ungetc``, ``fflush``
and no not recognize alternative ``fopen`` and ``tmpfile`` implementations.
(`#76776 <https://github.com/llvm/llvm-project/pull/76776>`_,
`#74296 <https://github.com/llvm/llvm-project/pull/74296>`_,
`#73335 <https://github.com/llvm/llvm-project/pull/73335>`_,
`#72627 <https://github.com/llvm/llvm-project/pull/72627>`_,
`#71518 <https://github.com/llvm/llvm-project/pull/71518>`_,
`#72016 <https://github.com/llvm/llvm-project/pull/72016>`_,
`#70540 <https://github.com/llvm/llvm-project/pull/70540>`_,
`#73638 <https://github.com/llvm/llvm-project/pull/73638>`_)
`#73638 <https://github.com/llvm/llvm-project/pull/73638>`_,
`#77331 <https://github.com/llvm/llvm-project/pull/77331>`_)

- The ``alpha.security.taint.TaintPropagation`` checker no longer propagates
taint on ``strlen`` and ``strnlen`` calls, unless these are marked
Expand Down
19 changes: 19 additions & 0 deletions clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2201,6 +2201,25 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
ErrnoNEZeroIrrelevant, GenericFailureMsg)
.ArgConstraint(NotNull(ArgNo(0))));

// int ungetc(int c, FILE *stream);
addToFunctionSummaryMap(
"ungetc", Signature(ArgTypes{IntTy, FilePtrTy}, RetType{IntTy}),
Summary(NoEvalCall)
.Case({ReturnValueCondition(BO_EQ, ArgNo(0)),
ArgumentCondition(0, WithinRange, {{0, UCharRangeMax}})},
ErrnoMustNotBeChecked, GenericSuccessMsg)
.Case({ReturnValueCondition(WithinRange, SingleValue(EOFv)),
ArgumentCondition(0, WithinRange, {{EOFv, EOFv}})},
ErrnoNEZeroIrrelevant,
"Assuming that 'ungetc' fails because EOF was passed as "
"character")
.Case({ReturnValueCondition(WithinRange, SingleValue(EOFv)),
ArgumentCondition(0, WithinRange, {{0, UCharRangeMax}})},
ErrnoNEZeroIrrelevant, GenericFailureMsg)
.ArgConstraint(ArgumentCondition(
0, WithinRange, {{EOFv, EOFv}, {0, UCharRangeMax}}))
.ArgConstraint(NotNull(ArgNo(1))));

// int fseek(FILE *stream, long offset, int whence);
// FIXME: It can be possible to get the 'SEEK_' values (like EOFv) and use
// these for condition of arg 2.
Expand Down
45 changes: 45 additions & 0 deletions clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
{{{"fputs"}, 2},
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, false), 1}},
{{{"ungetc"}, 2},
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
std::bind(&StreamChecker::evalUngetc, _1, _2, _3, _4), 1}},
{{{"fseek"}, 3},
{&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
{{{"ftell"}, 1},
Expand Down Expand Up @@ -332,6 +335,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
void evalFputx(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C, bool IsSingleChar) const;

void evalUngetc(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;

void preFseek(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
void evalFseek(const FnDescription *Desc, const CallEvent &Call,
Expand Down Expand Up @@ -916,6 +922,45 @@ void StreamChecker::evalFputx(const FnDescription *Desc, const CallEvent &Call,
C.addTransition(StateFailed);
}

void StreamChecker::evalUngetc(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
if (!StreamSym)
return;

const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
if (!CE)
return;

const StreamState *OldSS = State->get<StreamMap>(StreamSym);
if (!OldSS)
return;

assertStreamStateOpened(OldSS);

// Generate a transition for the success state.
std::optional<NonLoc> PutVal = Call.getArgSVal(0).getAs<NonLoc>();
if (!PutVal)
return;
ProgramStateRef StateNotFailed =
State->BindExpr(CE, C.getLocationContext(), *PutVal);
StateNotFailed =
StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
C.addTransition(StateNotFailed);

// Add transition for the failed state.
// Failure of 'ungetc' does not result in feof or ferror state.
// If the PutVal has value of EofVal the function should "fail", but this is
// the same transition as the success state.
// In this case only one state transition is added by the analyzer (the two
// new states may be similar).
ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
StateFailed =
StateFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
C.addTransition(StateFailed);
}

void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
Expand Down
1 change: 1 addition & 0 deletions clang/test/Analysis/Inputs/system-header-simulator.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ int fgetc(FILE *stream);
char *fgets(char *restrict str, int count, FILE *restrict stream);
int fputc(int ch, FILE *stream);
int fputs(const char *restrict s, FILE *restrict stream);
int ungetc(int c, FILE *stream);
int fseek(FILE *__stream, long int __off, int __whence);
long int ftell(FILE *__stream);
void rewind(FILE *__stream);
Expand Down
16 changes: 16 additions & 0 deletions clang/test/Analysis/stream-error.c
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,22 @@ void error_fputs(void) {
fputs("ABC", F); // expected-warning {{Stream might be already closed}}
}

void error_ungetc() {
FILE *F = tmpfile();
if (!F)
return;
int Ret = ungetc('X', F);
clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
if (Ret == EOF) {
clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
} else {
clang_analyzer_eval(Ret == 'X'); // expected-warning {{TRUE}}
}
fputc('Y', F); // no-warning
fclose(F);
ungetc('A', F); // expected-warning {{Stream might be already closed}}
}

void write_after_eof_is_allowed(void) {
FILE *F = tmpfile();
if (!F)
Expand Down
25 changes: 25 additions & 0 deletions clang/test/Analysis/stream-noopen.c
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,31 @@ void test_rewind(FILE *F) {
rewind(F);
}

void test_ungetc(FILE *F) {
int Ret = ungetc('X', F);
clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
if (Ret == 'X') {
if (errno) {} // expected-warning {{undefined}}
} else {
clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}}
clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
}
clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
}

void test_ungetc_EOF(FILE *F, int C) {
int Ret = ungetc(EOF, F);
clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}}
clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
Ret = ungetc(C, F);
if (Ret == EOF) {
clang_analyzer_eval(C == EOF); // expected-warning {{TRUE}}
// expected-warning@-1{{FALSE}}
}
}

void test_feof(FILE *F) {
errno = 0;
feof(F);
Expand Down
6 changes: 6 additions & 0 deletions clang/test/Analysis/stream.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ void check_fputs(void) {
fclose(fp);
}

void check_ungetc(void) {
FILE *fp = tmpfile();
ungetc('A', fp); // expected-warning {{Stream pointer might be NULL}}
fclose(fp);
}

void check_fseek(void) {
FILE *fp = tmpfile();
fseek(fp, 0, 0); // expected-warning {{Stream pointer might be NULL}}
Expand Down

0 comments on commit 8f78dd4

Please sign in to comment.