Skip to content

Commit 2eefd19

Browse files
committed
[clang][analyzer] No end-of-file when seek to file begin.
If `fseek` is used with 0 position and SEEK_SET it sets the position to the start of the file. This should not cause FEOF (end of file) error. The case of an empty file is not handled for simplification. It is not exactly defined in what cases `fseek` produces the different error states. Normally feof should not happen at all because it is possible to set the position after the end of file, but previous tests showed that still feof (and any other error cases) can happen. Reviewed By: donat.nagy Differential Revision: https://reviews.llvm.org/D153363
1 parent db15ace commit 2eefd19

File tree

2 files changed

+59
-6
lines changed

2 files changed

+59
-6
lines changed

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,14 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
285285
0}},
286286
};
287287

288+
/// Expanded value of EOF, empty before initialization.
288289
mutable std::optional<int> EofVal;
290+
/// Expanded value of SEEK_SET, 0 if not found.
291+
mutable int SeekSetVal = 0;
292+
/// Expanded value of SEEK_CUR, 1 if not found.
293+
mutable int SeekCurVal = 1;
294+
/// Expanded value of SEEK_END, 2 if not found.
295+
mutable int SeekEndVal = 2;
289296

290297
void evalFopen(const FnDescription *Desc, const CallEvent &Call,
291298
CheckerContext &C) const;
@@ -432,7 +439,7 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
432439
});
433440
}
434441

435-
void initEof(CheckerContext &C) const {
442+
void initMacroValues(CheckerContext &C) const {
436443
if (EofVal)
437444
return;
438445

@@ -441,6 +448,15 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
441448
EofVal = *OptInt;
442449
else
443450
EofVal = -1;
451+
if (const std::optional<int> OptInt =
452+
tryExpandAsInteger("SEEK_SET", C.getPreprocessor()))
453+
SeekSetVal = *OptInt;
454+
if (const std::optional<int> OptInt =
455+
tryExpandAsInteger("SEEK_END", C.getPreprocessor()))
456+
SeekEndVal = *OptInt;
457+
if (const std::optional<int> OptInt =
458+
tryExpandAsInteger("SEEK_CUR", C.getPreprocessor()))
459+
SeekCurVal = *OptInt;
444460
}
445461

446462
/// Searches for the ExplodedNode where the file descriptor was acquired for
@@ -488,7 +504,7 @@ const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N,
488504

489505
void StreamChecker::checkPreCall(const CallEvent &Call,
490506
CheckerContext &C) const {
491-
initEof(C);
507+
initMacroValues(C);
492508

493509
const FnDescription *Desc = lookupFn(Call);
494510
if (!Desc || !Desc->PreFn)
@@ -786,6 +802,11 @@ void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
786802
if (!State->get<StreamMap>(StreamSym))
787803
return;
788804

805+
const llvm::APSInt *PosV =
806+
C.getSValBuilder().getKnownValue(State, Call.getArgSVal(1));
807+
const llvm::APSInt *WhenceV =
808+
C.getSValBuilder().getKnownValue(State, Call.getArgSVal(2));
809+
789810
DefinedSVal RetVal = makeRetVal(C, CE);
790811

791812
// Make expression result.
@@ -804,9 +825,12 @@ void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
804825
// It is possible that fseek fails but sets none of the error flags.
805826
// If fseek failed, assume that the file position becomes indeterminate in any
806827
// case.
828+
StreamErrorState NewErrS = ErrorNone | ErrorFError;
829+
// Setting the position to start of file never produces EOF error.
830+
if (!(PosV && *PosV == 0 && WhenceV && *WhenceV == SeekSetVal))
831+
NewErrS = NewErrS | ErrorFEof;
807832
StateFailed = StateFailed->set<StreamMap>(
808-
StreamSym,
809-
StreamState::getOpened(Desc, ErrorNone | ErrorFEof | ErrorFError, true));
833+
StreamSym, StreamState::getOpened(Desc, NewErrS, true));
810834

811835
C.addTransition(StateNotFailed);
812836
C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
@@ -1153,7 +1177,7 @@ StreamChecker::ensureFseekWhenceCorrect(SVal WhenceVal, CheckerContext &C,
11531177
return State;
11541178

11551179
int64_t X = CI->getValue().getSExtValue();
1156-
if (X >= 0 && X <= 2)
1180+
if (X == SeekSetVal || X == SeekCurVal || X == SeekEndVal)
11571181
return State;
11581182

11591183
if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) {

clang/test/Analysis/stream-error.c

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ void error_fseek(void) {
146146
FILE *F = fopen("file", "r");
147147
if (!F)
148148
return;
149-
int rc = fseek(F, 0, SEEK_SET);
149+
int rc = fseek(F, 1, SEEK_SET);
150150
if (rc) {
151151
int IsFEof = feof(F), IsFError = ferror(F);
152152
// Get feof or ferror or no error.
@@ -173,6 +173,35 @@ void error_fseek(void) {
173173
fclose(F);
174174
}
175175

176+
void error_fseek_0(void) {
177+
FILE *F = fopen("file", "r");
178+
if (!F)
179+
return;
180+
int rc = fseek(F, 0, SEEK_SET);
181+
if (rc) {
182+
int IsFEof = feof(F), IsFError = ferror(F);
183+
// Get ferror or no error, but not feof.
184+
clang_analyzer_eval(IsFError);
185+
// expected-warning@-1 {{FALSE}}
186+
// expected-warning@-2 {{TRUE}}
187+
clang_analyzer_eval(IsFEof);
188+
// expected-warning@-1 {{FALSE}}
189+
// Error flags should not change.
190+
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
191+
if (IsFError)
192+
clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
193+
else
194+
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
195+
} else {
196+
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
197+
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
198+
// Error flags should not change.
199+
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
200+
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
201+
}
202+
fclose(F);
203+
}
204+
176205
void error_indeterminate(void) {
177206
FILE *F = fopen("file", "r+");
178207
if (!F)

0 commit comments

Comments
 (0)