maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[Analyzer][StreamChecker] Added check for "indeterminate file position". 

Summary:
According to the standard, after a `wread` or `fwrite` call the file position
becomes "indeterminate". It is assumable that a next read or write causes
undefined behavior, so a (fatal error) warning is added for this case.
The indeterminate position can be cleared by some operations, for example
`fseek` or `freopen`, not with `clearerr`.

Reviewers: Szelethus, baloghadamsoftware, martong, NoQ, xazax.hun, dcoughlin

Reviewed By: Szelethus

Subscribers: rnkovacs, NoQ, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, ASDenysPetrov, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D80018
This commit is contained in:
Balázs Kéri 2020-05-27 16:23:42 +02:00
parent c4990a03c6
commit 9081fa2099
2 changed files with 181 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

124
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
View File

@ -92,7 +92,27 @@ struct StreamState {
/// State of the error flags.
/// Ignored in non-opened stream state but must be NoError.
StreamErrorState ErrorState;
StreamErrorState const ErrorState;
/// Indicate if the file has an "indeterminate file position indicator".
/// This can be set at a failing read or write or seek operation.
/// If it is set no more read or write is allowed.
/// This value is not dependent on the stream error flags:
/// The error flag may be cleared with `clearerr` but the file position
/// remains still indeterminate.
/// This value applies to all error states in ErrorState except FEOF.
/// An EOF+indeterminate state is the same as EOF state.
bool const FilePositionIndeterminate = false;
StreamState(const FnDescription *L, KindTy S, const StreamErrorState &ES,
bool IsFilePositionIndeterminate)
: LastOperation(L), State(S), ErrorState(ES),
FilePositionIndeterminate(IsFilePositionIndeterminate) {
assert((!ES.isFEof() || !IsFilePositionIndeterminate) &&
"FilePositionIndeterminate should be false in FEof case.");
assert((State == Opened || ErrorState.isNoError()) &&
"ErrorState should be None in non-opened stream state.");
}
bool isOpened() const { return State == Opened; }
bool isClosed() const { return State == Closed; }
@ -102,24 +122,27 @@ struct StreamState {
// In not opened state error state should always NoError, so comparison
// here is no problem.
return LastOperation == X.LastOperation && State == X.State &&
ErrorState == X.ErrorState;
ErrorState == X.ErrorState &&
FilePositionIndeterminate == X.FilePositionIndeterminate;
}
static StreamState getOpened(const FnDescription *L,
const StreamErrorState &ES = {}) {
return StreamState{L, Opened, ES};
const StreamErrorState &ES = ErrorNone,
bool IsFilePositionIndeterminate = false) {
return StreamState{L, Opened, ES, IsFilePositionIndeterminate};
}
static StreamState getClosed(const FnDescription *L) {
return StreamState{L, Closed, {}};
return StreamState{L, Closed, {}, false};
}
static StreamState getOpenFailed(const FnDescription *L) {
return StreamState{L, OpenFailed, {}};
return StreamState{L, OpenFailed, {}, false};
}
void Profile(llvm::FoldingSetNodeID &ID) const {
ID.AddPointer(LastOperation);
ID.AddInteger(State);
ID.AddInteger(ErrorState);
ID.AddBoolean(FilePositionIndeterminate);
}
};
@ -173,7 +196,8 @@ ProgramStateRef bindInt(uint64_t Value, ProgramStateRef State,
class StreamChecker
: public Checker<check::PreCall, eval::Call, check::DeadSymbols> {
mutable std::unique_ptr<BuiltinBug> BT_nullfp, BT_illegalwhence,
BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak, BT_StreamEof;
BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak, BT_StreamEof,
BT_IndeterminatePosition;
public:
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
@ -279,6 +303,16 @@ private:
ProgramStateRef ensureStreamOpened(SVal StreamVal, CheckerContext &C,
ProgramStateRef State) const;
/// Check that the stream has not an invalid ("indeterminate") file position,
/// generate warning for it.
/// (EOF is not an invalid position.)
/// The returned state can be nullptr if a fatal error was generated.
/// It can return non-null state if the stream has not an invalid position or
/// there is execution path with non-invalid position.
ProgramStateRef
ensureNoFilePositionIndeterminate(SVal StreamVal, CheckerContext &C,
ProgramStateRef State) const;
/// Check the legality of the 'whence' argument of 'fseek'.
/// Generate error and return nullptr if it is found to be illegal.
/// Otherwise returns the state.
@ -447,6 +481,9 @@ void StreamChecker::preFread(const FnDescription *Desc, const CallEvent &Call,
if (!State)
return;
State = ensureStreamOpened(StreamVal, C, State);
if (!State)
return;
State = ensureNoFilePositionIndeterminate(StreamVal, C, State);
if (!State)
return;
@ -468,6 +505,9 @@ void StreamChecker::preFwrite(const FnDescription *Desc, const CallEvent &Call,
if (!State)
return;
State = ensureStreamOpened(StreamVal, C, State);
if (!State)
return;
State = ensureNoFilePositionIndeterminate(StreamVal, C, State);
if (!State)
return;
@ -548,7 +588,9 @@ void StreamChecker::evalFreadFwrite(const FnDescription *Desc,
NewES = (SS->ErrorState == ErrorFEof) ? ErrorFEof : ErrorFEof | ErrorFError;
else
NewES = ErrorFError;
StreamState NewState = StreamState::getOpened(Desc, NewES);
// If a (non-EOF) error occurs, the resulting value of the file position
// indicator for the stream is indeterminate.
StreamState NewState = StreamState::getOpened(Desc, NewES, !NewES.isFEof());
StateFailed = StateFailed->set<StreamMap>(StreamSym, NewState);
C.addTransition(StateFailed);
}
@ -601,9 +643,11 @@ void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
// We get error.
// It is possible that fseek fails but sets none of the error flags.
// If fseek failed, assume that the file position becomes indeterminate in any
// case.
StateFailed = StateFailed->set<StreamMap>(
StreamSym,
StreamState::getOpened(Desc, ErrorNone | ErrorFEof | ErrorFError));
StreamState::getOpened(Desc, ErrorNone | ErrorFEof | ErrorFError, true));
C.addTransition(StateNotFailed);
C.addTransition(StateFailed);
@ -623,7 +667,10 @@ void StreamChecker::evalClearerr(const FnDescription *Desc,
assertStreamStateOpened(SS);
State = State->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
// FilePositionIndeterminate is not cleared.
State = State->set<StreamMap>(
StreamSym,
StreamState::getOpened(Desc, ErrorNone, SS->FilePositionIndeterminate));
C.addTransition(State);
}
@ -651,7 +698,9 @@ void StreamChecker::evalFeofFerror(const FnDescription *Desc,
// From now on it is the only one error state.
ProgramStateRef TrueState = bindAndAssumeTrue(State, C, CE);
C.addTransition(TrueState->set<StreamMap>(
StreamSym, StreamState::getOpened(Desc, ErrorKind)));
StreamSym, StreamState::getOpened(Desc, ErrorKind,
SS->FilePositionIndeterminate &&
!ErrorKind.isFEof())));
}
if (StreamErrorState NewES = SS->ErrorState & (~ErrorKind)) {
// Execution path(s) with ErrorKind not set.
@ -659,7 +708,9 @@ void StreamChecker::evalFeofFerror(const FnDescription *Desc,
// New error state is everything before minus ErrorKind.
ProgramStateRef FalseState = bindInt(0, State, C, CE);
C.addTransition(FalseState->set<StreamMap>(
StreamSym, StreamState::getOpened(Desc, NewES)));
StreamSym,
StreamState::getOpened(
Desc, NewES, SS->FilePositionIndeterminate && !NewES.isFEof())));
}
}
@ -767,6 +818,55 @@ ProgramStateRef StreamChecker::ensureStreamOpened(SVal StreamVal,
return State;
}
ProgramStateRef StreamChecker::ensureNoFilePositionIndeterminate(
SVal StreamVal, CheckerContext &C, ProgramStateRef State) const {
SymbolRef Sym = StreamVal.getAsSymbol();
if (!Sym)
return State;
const StreamState *SS = State->get<StreamMap>(Sym);
if (!SS)
return State;
assert(SS->isOpened() && "First ensure that stream is opened.");
if (SS->FilePositionIndeterminate) {
if (!BT_IndeterminatePosition)
BT_IndeterminatePosition.reset(
new BuiltinBug(this, "Invalid stream state",
"File position of the stream might be 'indeterminate' "
"after a failed operation. "
"Can cause undefined behavior."));
if (SS->ErrorState & ErrorFEof) {
// The error is unknown but may be FEOF.
// Continue analysis with the FEOF error state.
// Report warning because the other possible error states.
ExplodedNode *N = C.generateNonFatalErrorNode(State);
if (!N)
return nullptr;
C.emitReport(std::make_unique<PathSensitiveBugReport>(
*BT_IndeterminatePosition, BT_IndeterminatePosition->getDescription(),
N));
return State->set<StreamMap>(
Sym, StreamState::getOpened(SS->LastOperation, ErrorFEof, false));
}
// Known or unknown error state without FEOF possible.
// Stop analysis, report error.
ExplodedNode *N = C.generateErrorNode(State);
if (N)
C.emitReport(std::make_unique<PathSensitiveBugReport>(
*BT_IndeterminatePosition, BT_IndeterminatePosition->getDescription(),
N));
return nullptr;
}
return State;
}
ProgramStateRef
StreamChecker::ensureFseekWhenceCorrect(SVal WhenceVal, CheckerContext &C,
ProgramStateRef State) const {

71
clang/test/Analysis/stream-error.c
View File

@ -76,7 +76,7 @@ void error_fread() {
}
if (ferror(F)) {
clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
fread(Buf, 1, 10, F); // no warning
fread(Buf, 1, 10, F); // expected-warning {{might be 'indeterminate'}}
}
}
fclose(F);
@ -94,7 +94,7 @@ void error_fwrite() {
} else {
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
fwrite(0, 1, 10, F); // no warning
fwrite(0, 1, 10, F); // expected-warning {{might be 'indeterminate'}}
}
fclose(F);
Ret = fwrite(0, 1, 10, F); // expected-warning {{Stream might be already closed}}
@ -166,3 +166,70 @@ void error_fseek() {
}
fclose(F);
}
void error_indeterminate() {
FILE *F = fopen("file", "r+");
if (!F)
return;
const char *Buf = "123456789";
int rc = fseek(F, 0, SEEK_SET);
if (rc) {
if (feof(F)) {
fwrite(Buf, 1, 10, F); // no warning
} else if (ferror(F)) {
fwrite(Buf, 1, 10, F); // expected-warning {{might be 'indeterminate'}}
} else {
fwrite(Buf, 1, 10, F); // expected-warning {{might be 'indeterminate'}}
}
}
fclose(F);
}
void error_indeterminate_clearerr() {
FILE *F = fopen("file", "r+");
if (!F)
return;
const char *Buf = "123456789";
int rc = fseek(F, 0, SEEK_SET);
if (rc) {
if (feof(F)) {
clearerr(F);
fwrite(Buf, 1, 10, F); // no warning
} else if (ferror(F)) {
clearerr(F);
fwrite(Buf, 1, 10, F); // expected-warning {{might be 'indeterminate'}}
} else {
clearerr(F);
fwrite(Buf, 1, 10, F); // expected-warning {{might be 'indeterminate'}}
}
}
fclose(F);
}
void error_indeterminate_feof1() {
FILE *F = fopen("file", "r+");
if (!F)
return;
char Buf[10];
if (fread(Buf, 1, 10, F) < 10) {
if (feof(F)) {
// error is feof, should be non-indeterminate
fwrite("1", 1, 1, F); // no warning
}
}
fclose(F);
}
void error_indeterminate_feof2() {
FILE *F = fopen("file", "r+");
if (!F)
return;
char Buf[10];
if (fread(Buf, 1, 10, F) < 10) {
if (ferror(F) == 0) {
// error is feof, should be non-indeterminate
fwrite("1", 1, 1, F); // no warning
}
}
fclose(F);
}