Skip to content

Commit 8f78dd4

Browse files
authored
[clang][analyzer] Add function 'ungetc' to StreamChecker. (#77331)
`StdLibraryFunctionsChecker` is updated too with `ungetc`.
1 parent b788692 commit 8f78dd4

File tree

7 files changed

+117
-3
lines changed

7 files changed

+117
-3
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1202,16 +1202,18 @@ Improvements
12021202
(`c3a87ddad62a <https://github.com/llvm/llvm-project/commit/c3a87ddad62a6cc01acaccc76592bc6730c8ac3c>`_,
12031203
`0954dc3fb921 <https://github.com/llvm/llvm-project/commit/0954dc3fb9214b994623f5306473de075f8e3593>`_)
12041204

1205-
- Improved the ``alpha.unix.Stream`` checker by modeling more functions like,
1206-
``fflush``, ``fputs``, ``fgetc``, ``fputc``, ``fopen``, ``fdopen``, ``fgets``, ``tmpfile``.
1205+
- Improved the ``alpha.unix.Stream`` checker by modeling more functions
1206+
``fputs``, ``fputc``, ``fgets``, ``fgetc``, ``fdopen``, ``ungetc``, ``fflush``
1207+
and no not recognize alternative ``fopen`` and ``tmpfile`` implementations.
12071208
(`#76776 <https://github.com/llvm/llvm-project/pull/76776>`_,
12081209
`#74296 <https://github.com/llvm/llvm-project/pull/74296>`_,
12091210
`#73335 <https://github.com/llvm/llvm-project/pull/73335>`_,
12101211
`#72627 <https://github.com/llvm/llvm-project/pull/72627>`_,
12111212
`#71518 <https://github.com/llvm/llvm-project/pull/71518>`_,
12121213
`#72016 <https://github.com/llvm/llvm-project/pull/72016>`_,
12131214
`#70540 <https://github.com/llvm/llvm-project/pull/70540>`_,
1214-
`#73638 <https://github.com/llvm/llvm-project/pull/73638>`_)
1215+
`#73638 <https://github.com/llvm/llvm-project/pull/73638>`_,
1216+
`#77331 <https://github.com/llvm/llvm-project/pull/77331>`_)
12151217

12161218
- The ``alpha.security.taint.TaintPropagation`` checker no longer propagates
12171219
taint on ``strlen`` and ``strnlen`` calls, unless these are marked

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2201,6 +2201,25 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
22012201
ErrnoNEZeroIrrelevant, GenericFailureMsg)
22022202
.ArgConstraint(NotNull(ArgNo(0))));
22032203

2204+
// int ungetc(int c, FILE *stream);
2205+
addToFunctionSummaryMap(
2206+
"ungetc", Signature(ArgTypes{IntTy, FilePtrTy}, RetType{IntTy}),
2207+
Summary(NoEvalCall)
2208+
.Case({ReturnValueCondition(BO_EQ, ArgNo(0)),
2209+
ArgumentCondition(0, WithinRange, {{0, UCharRangeMax}})},
2210+
ErrnoMustNotBeChecked, GenericSuccessMsg)
2211+
.Case({ReturnValueCondition(WithinRange, SingleValue(EOFv)),
2212+
ArgumentCondition(0, WithinRange, {{EOFv, EOFv}})},
2213+
ErrnoNEZeroIrrelevant,
2214+
"Assuming that 'ungetc' fails because EOF was passed as "
2215+
"character")
2216+
.Case({ReturnValueCondition(WithinRange, SingleValue(EOFv)),
2217+
ArgumentCondition(0, WithinRange, {{0, UCharRangeMax}})},
2218+
ErrnoNEZeroIrrelevant, GenericFailureMsg)
2219+
.ArgConstraint(ArgumentCondition(
2220+
0, WithinRange, {{EOFv, EOFv}, {0, UCharRangeMax}}))
2221+
.ArgConstraint(NotNull(ArgNo(1))));
2222+
22042223
// int fseek(FILE *stream, long offset, int whence);
22052224
// FIXME: It can be possible to get the 'SEEK_' values (like EOFv) and use
22062225
// these for condition of arg 2.

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
263263
{{{"fputs"}, 2},
264264
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
265265
std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, false), 1}},
266+
{{{"ungetc"}, 2},
267+
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
268+
std::bind(&StreamChecker::evalUngetc, _1, _2, _3, _4), 1}},
266269
{{{"fseek"}, 3},
267270
{&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
268271
{{{"ftell"}, 1},
@@ -332,6 +335,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
332335
void evalFputx(const FnDescription *Desc, const CallEvent &Call,
333336
CheckerContext &C, bool IsSingleChar) const;
334337

338+
void evalUngetc(const FnDescription *Desc, const CallEvent &Call,
339+
CheckerContext &C) const;
340+
335341
void preFseek(const FnDescription *Desc, const CallEvent &Call,
336342
CheckerContext &C) const;
337343
void evalFseek(const FnDescription *Desc, const CallEvent &Call,
@@ -916,6 +922,45 @@ void StreamChecker::evalFputx(const FnDescription *Desc, const CallEvent &Call,
916922
C.addTransition(StateFailed);
917923
}
918924

925+
void StreamChecker::evalUngetc(const FnDescription *Desc, const CallEvent &Call,
926+
CheckerContext &C) const {
927+
ProgramStateRef State = C.getState();
928+
SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
929+
if (!StreamSym)
930+
return;
931+
932+
const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
933+
if (!CE)
934+
return;
935+
936+
const StreamState *OldSS = State->get<StreamMap>(StreamSym);
937+
if (!OldSS)
938+
return;
939+
940+
assertStreamStateOpened(OldSS);
941+
942+
// Generate a transition for the success state.
943+
std::optional<NonLoc> PutVal = Call.getArgSVal(0).getAs<NonLoc>();
944+
if (!PutVal)
945+
return;
946+
ProgramStateRef StateNotFailed =
947+
State->BindExpr(CE, C.getLocationContext(), *PutVal);
948+
StateNotFailed =
949+
StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
950+
C.addTransition(StateNotFailed);
951+
952+
// Add transition for the failed state.
953+
// Failure of 'ungetc' does not result in feof or ferror state.
954+
// If the PutVal has value of EofVal the function should "fail", but this is
955+
// the same transition as the success state.
956+
// In this case only one state transition is added by the analyzer (the two
957+
// new states may be similar).
958+
ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
959+
StateFailed =
960+
StateFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
961+
C.addTransition(StateFailed);
962+
}
963+
919964
void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
920965
CheckerContext &C) const {
921966
ProgramStateRef State = C.getState();

clang/test/Analysis/Inputs/system-header-simulator.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ int fgetc(FILE *stream);
5353
char *fgets(char *restrict str, int count, FILE *restrict stream);
5454
int fputc(int ch, FILE *stream);
5555
int fputs(const char *restrict s, FILE *restrict stream);
56+
int ungetc(int c, FILE *stream);
5657
int fseek(FILE *__stream, long int __off, int __whence);
5758
long int ftell(FILE *__stream);
5859
void rewind(FILE *__stream);

clang/test/Analysis/stream-error.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,22 @@ void error_fputs(void) {
191191
fputs("ABC", F); // expected-warning {{Stream might be already closed}}
192192
}
193193

194+
void error_ungetc() {
195+
FILE *F = tmpfile();
196+
if (!F)
197+
return;
198+
int Ret = ungetc('X', F);
199+
clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
200+
if (Ret == EOF) {
201+
clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
202+
} else {
203+
clang_analyzer_eval(Ret == 'X'); // expected-warning {{TRUE}}
204+
}
205+
fputc('Y', F); // no-warning
206+
fclose(F);
207+
ungetc('A', F); // expected-warning {{Stream might be already closed}}
208+
}
209+
194210
void write_after_eof_is_allowed(void) {
195211
FILE *F = tmpfile();
196212
if (!F)

clang/test/Analysis/stream-noopen.c

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,31 @@ void test_rewind(FILE *F) {
138138
rewind(F);
139139
}
140140

141+
void test_ungetc(FILE *F) {
142+
int Ret = ungetc('X', F);
143+
clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
144+
if (Ret == 'X') {
145+
if (errno) {} // expected-warning {{undefined}}
146+
} else {
147+
clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}}
148+
clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
149+
}
150+
clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
151+
clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
152+
}
153+
154+
void test_ungetc_EOF(FILE *F, int C) {
155+
int Ret = ungetc(EOF, F);
156+
clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
157+
clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}}
158+
clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
159+
Ret = ungetc(C, F);
160+
if (Ret == EOF) {
161+
clang_analyzer_eval(C == EOF); // expected-warning {{TRUE}}
162+
// expected-warning@-1{{FALSE}}
163+
}
164+
}
165+
141166
void test_feof(FILE *F) {
142167
errno = 0;
143168
feof(F);

clang/test/Analysis/stream.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,12 @@ void check_fputs(void) {
3939
fclose(fp);
4040
}
4141

42+
void check_ungetc(void) {
43+
FILE *fp = tmpfile();
44+
ungetc('A', fp); // expected-warning {{Stream pointer might be NULL}}
45+
fclose(fp);
46+
}
47+
4248
void check_fseek(void) {
4349
FILE *fp = tmpfile();
4450
fseek(fp, 0, 0); // expected-warning {{Stream pointer might be NULL}}

0 commit comments

Comments
 (0)