balazske updated this revision to Diff 251112.
balazske added a comment.
Herald added a subscriber: ASDenysPetrov.
Rebased.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75851/new/
https://reviews.llvm.org/D75851
Files:
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream-error.c
Index: clang/test/Analysis/stream-error.c
===================================================================
--- clang/test/Analysis/stream-error.c
+++ clang/test/Analysis/stream-error.c
@@ -38,3 +38,48 @@
}
fclose(F);
}
+
+void error_fseek() {
+ FILE *F = fopen("file", "r");
+ if (!F)
+ return;
+ int rc = fseek(F, 0, SEEK_SET);
+ if (rc) {
+ int Eof = feof(F), Error = ferror(F);
+ // Get feof or ferror or no error.
+ clang_analyzer_eval(Eof || Error); // expected-warning {{FALSE}} \
+ // expected-warning {{TRUE}}
+ clang_analyzer_eval(Eof && Error); // expected-warning {{FALSE}}
+ // Error flags should not change.
+ if (Eof)
+ clang_analyzer_eval(feof(F)); // expected-warning {{TRUE}}
+ else
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ if (Error)
+ clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+ else
+ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+ } else {
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+ // Error flags should not change.
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+ }
+ fclose(F);
+}
+
+void error_fseek_clearerr() {
+ FILE *F = fopen("file", "r");
+ if (!F)
+ return;
+ int rc = fseek(F, 0, SEEK_SET);
+ if (rc && feof(F)) {
+ clearerr(F);
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ } else if (rc && ferror(F)) {
+ clearerr(F);
+ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+ }
+ fclose(F);
+}
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -30,8 +30,12 @@
namespace {
+struct FnDescription;
+
/// Full state information about a stream pointer.
struct StreamState {
+ const FnDescription *LastOperation;
+
/// State of a stream symbol.
/// FIXME: We need maybe an "escaped" state later.
enum KindTy {
@@ -49,6 +53,8 @@
FEof,
/// Other generic (non-EOF) error (`ferror` is true).
FError,
+ /// Unknown error, the meaning depends on the last operation.
+ UnknownError
} ErrorState = NoError;
bool isOpened() const { return State == Opened; }
@@ -67,21 +73,35 @@
assert(State == Opened && "Error undefined for closed stream.");
return ErrorState == FError;
}
+ bool isUnknownError() const {
+ assert(State == Opened && "Error undefined for closed stream.");
+ return ErrorState == UnknownError;
+ }
bool operator==(const StreamState &X) const {
// In not opened state error should always NoError.
- return State == X.State && ErrorState == X.ErrorState;
+ return LastOperation == X.LastOperation && State == X.State &&
+ ErrorState == X.ErrorState;
}
- static StreamState getOpened() { return StreamState{Opened}; }
- static StreamState getClosed() { return StreamState{Closed}; }
- static StreamState getOpenFailed() { return StreamState{OpenFailed}; }
- static StreamState getOpenedWithFEof() { return StreamState{Opened, FEof}; }
- static StreamState getOpenedWithFError() {
- return StreamState{Opened, FError};
+ static StreamState getOpened(const FnDescription *L) {
+ return StreamState{L, Opened};
+ }
+ static StreamState getClosed(const FnDescription *L) {
+ return StreamState{L, Closed};
+ }
+ static StreamState getOpenFailed(const FnDescription *L) {
+ return StreamState{L, OpenFailed};
+ }
+ static StreamState getOpened(const FnDescription *L, ErrorKindTy E) {
+ return StreamState{L, Opened, E};
+ }
+ static StreamState getOpenedWithUnknownError(const FnDescription *L) {
+ return StreamState{L, Opened, UnknownError};
}
void Profile(llvm::FoldingSetNodeID &ID) const {
+ ID.AddPointer(LastOperation);
ID.AddInteger(State);
ID.AddInteger(ErrorState);
}
@@ -99,6 +119,11 @@
FnCheck PreFn;
FnCheck EvalFn;
ArgNoTy StreamArgNo;
+ // What errors are possible after this operation.
+ // Used only if this operation resulted in UnknownError
+ // (otherwise there is a known single error).
+ // Must contain 2 or 3 elements, or zero.
+ llvm::SmallVector<StreamState::ErrorKindTy, 3> PossibleErrors = {};
};
/// Get the value of the stream argument out of the passed call event.
@@ -119,6 +144,20 @@
.castAs<DefinedSVal>();
}
+ProgramStateRef bindAndAssumeTrue(ProgramStateRef State, CheckerContext &C, const CallExpr *CE) {
+ DefinedSVal RetVal = makeRetVal(C, CE);
+ State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+ State = State->assume(RetVal, true);
+ assert(State && "Assumption on new value should not fail.");
+ return State;
+}
+
+ProgramStateRef bindInt(uint64_t Value, ProgramStateRef State, CheckerContext &C, const CallExpr *CE) {
+ State = State->BindExpr(CE, C.getLocationContext(),
+ C.getSValBuilder().makeIntVal(Value, false));
+ return State;
+}
+
class StreamChecker
: public Checker<check::PreCall, eval::Call, check::DeadSymbols> {
mutable std::unique_ptr<BuiltinBug> BT_nullfp, BT_illegalwhence,
@@ -131,28 +170,42 @@
private:
CallDescriptionMap<FnDescription> FnDescriptions = {
- {{"fopen"}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
+ {{"fopen"}, {nullptr, &StreamChecker::evalFopen, ArgNone, {}}},
{{"freopen", 3},
- {&StreamChecker::preFreopen, &StreamChecker::evalFreopen, 2}},
- {{"tmpfile"}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
+ {&StreamChecker::preFreopen, &StreamChecker::evalFreopen, 2, {}}},
+ {{"tmpfile"}, {nullptr, &StreamChecker::evalFopen, ArgNone, {}}},
{{"fclose", 1},
- {&StreamChecker::preDefault, &StreamChecker::evalFclose, 0}},
- {{"fread", 4}, {&StreamChecker::preDefault, nullptr, 3}},
- {{"fwrite", 4}, {&StreamChecker::preDefault, nullptr, 3}},
- {{"fseek", 3}, {&StreamChecker::preFseek, nullptr, 0}},
- {{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0}},
- {{"rewind", 1}, {&StreamChecker::preDefault, nullptr, 0}},
- {{"fgetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
- {{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
+ {&StreamChecker::preDefault, &StreamChecker::evalFclose, 0, {}}},
+ {{"fread", 4}, {&StreamChecker::preDefault, nullptr, 3, {}}},
+ {{"fwrite", 4}, {&StreamChecker::preDefault, nullptr, 3, {}}},
+ {{"fseek", 3},
+ {&StreamChecker::preFseek,
+ &StreamChecker::evalFseek,
+ 0,
+ {StreamState::FEof, StreamState::FError, StreamState::NoError}}},
+ {{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0, {}}},
+ {{"rewind", 1}, {&StreamChecker::preDefault, nullptr, 0, {}}},
+ {{"fgetpos", 2}, {&StreamChecker::preDefault, nullptr, 0, {}}},
+ {{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0, {}}},
{{"clearerr", 1},
- {&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0}},
+ {&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0, {}}},
+ // Note: feof can result in UnknownError if at the call there is a
+ // PossibleErrors with all 3 error states (including NoError).
+ // Then if feof is false the remaining error could be FError or NoError.
{{"feof", 1},
{&StreamChecker::preDefault,
- &StreamChecker::evalFeofFerror<&StreamState::isFEof>, 0}},
+ &StreamChecker::evalFeofFerror<StreamState::FEof>,
+ 0,
+ {StreamState::FError, StreamState::NoError}}},
+ // Note: ferror can result in UnknownError if at the call there is a
+ // PossibleErrors with all 3 error states (including NoError).
+ // Then if ferror is false the remaining error could be FEof or NoError.
{{"ferror", 1},
{&StreamChecker::preDefault,
- &StreamChecker::evalFeofFerror<&StreamState::isFError>, 0}},
- {{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0}},
+ &StreamChecker::evalFeofFerror<StreamState::FError>,
+ 0,
+ {StreamState::FEof, StreamState::NoError}}},
+ {{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0, {}}},
};
void evalFopen(const FnDescription *Desc, const CallEvent &Call,
@@ -168,6 +221,8 @@
void preFseek(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
+ void evalFseek(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
void preDefault(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
@@ -175,7 +230,7 @@
void evalClearerr(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
- template <bool (StreamState::*IsOfError)() const>
+ template <StreamState::ErrorKindTy ErrorKind>
void evalFeofFerror(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
@@ -258,8 +313,10 @@
std::tie(StateNotNull, StateNull) =
C.getConstraintManager().assumeDual(State, RetVal);
- StateNotNull = StateNotNull->set<StreamMap>(RetSym, StreamState::getOpened());
- StateNull = StateNull->set<StreamMap>(RetSym, StreamState::getOpenFailed());
+ StateNotNull =
+ StateNotNull->set<StreamMap>(RetSym, StreamState::getOpened(Desc));
+ StateNull =
+ StateNull->set<StreamMap>(RetSym, StreamState::getOpenFailed(Desc));
C.addTransition(StateNotNull);
C.addTransition(StateNull);
@@ -308,9 +365,9 @@
C.getSValBuilder().makeNull());
StateRetNotNull =
- StateRetNotNull->set<StreamMap>(StreamSym, StreamState::getOpened());
+ StateRetNotNull->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
StateRetNull =
- StateRetNull->set<StreamMap>(StreamSym, StreamState::getOpenFailed());
+ StateRetNull->set<StreamMap>(StreamSym, StreamState::getOpenFailed(Desc));
C.addTransition(StateRetNotNull);
C.addTransition(StateRetNull);
@@ -332,7 +389,7 @@
// Close the File Descriptor.
// Regardless if the close fails or not, stream becomes "closed"
// and can not be used any more.
- State = State->set<StreamMap>(Sym, StreamState::getClosed());
+ State = State->set<StreamMap>(Sym, StreamState::getClosed(Desc));
C.addTransition(State);
}
@@ -354,6 +411,43 @@
C.addTransition(State);
}
+void StreamChecker::evalFseek(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;
+
+ // Ignore the call if the stream is not tracked.
+ if (!State->get<StreamMap>(StreamSym))
+ return;
+
+ DefinedSVal RetVal = makeRetVal(C, CE);
+
+ // Make expression result.
+ State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+
+ // Bifurcate the state into failed and non-failed.
+ // Return zero on success, nonzero on error.
+ ProgramStateRef StateNotFailed, StateFailed;
+ std::tie(StateFailed, StateNotFailed) =
+ C.getConstraintManager().assumeDual(State, RetVal);
+
+ // Reset the state to opened with no error.
+ StateNotFailed =
+ StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+ // We get error.
+ StateFailed = StateFailed->set<StreamMap>(
+ StreamSym, StreamState::getOpenedWithUnknownError(Desc));
+
+ C.addTransition(StateNotFailed);
+ C.addTransition(StateFailed);
+}
+
void StreamChecker::evalClearerr(const FnDescription *Desc,
const CallEvent &Call,
CheckerContext &C) const {
@@ -371,11 +465,11 @@
if (SS->isNoError())
return;
- State = State->set<StreamMap>(StreamSym, StreamState::getOpened());
+ State = State->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
C.addTransition(State);
}
-template <bool (StreamState::*IsOfError)() const>
+template <StreamState::ErrorKindTy ErrorKind>
void StreamChecker::evalFeofFerror(const FnDescription *Desc,
const CallEvent &Call,
CheckerContext &C) const {
@@ -395,18 +489,51 @@
ASSERT_STREAMSTATE_OPENED;
- if ((SS->*IsOfError)()) {
- // Function returns nonzero.
- DefinedSVal RetVal = makeRetVal(C, CE);
- State = State->BindExpr(CE, C.getLocationContext(), RetVal);
- State = State->assume(RetVal, true);
- assert(State && "Assumption on new value should not fail.");
+ if (SS->isUnknownError()) {
+ llvm::SmallVector<StreamState::ErrorKindTy, 3> NewPossibleErrors;
+ for (StreamState::ErrorKindTy E : SS->LastOperation->PossibleErrors)
+ if (E != ErrorKind)
+ NewPossibleErrors.push_back(E);
+ if (NewPossibleErrors.size() == SS->LastOperation->PossibleErrors.size()) {
+ // This kind of error is not possible, function returns zero.
+ // Error state remains unknown (we know only that it is not ErrorKind).
+ State = bindInt(0, State, C, CE);
+ State = State->set<StreamMap>(
+ StreamSym, StreamState::getOpenedWithUnknownError(Desc));
+ C.addTransition(State);
+ } else {
+ // The previously unknown error could be ErrorKind.
+ ProgramStateRef StateTrue = bindAndAssumeTrue(State, C, CE);
+ ProgramStateRef StateFalse = bindInt(0, State, C, CE);
+ // If return true, set error state to ErrorKind.
+ StateTrue = StateTrue->set<StreamMap>(
+ StreamSym, StreamState::getOpened(Desc, ErrorKind));
+ // Return false
+ if (NewPossibleErrors.size() == 1)
+ // One other error is possible, new error kind is fixed to that.
+ StateFalse = StateFalse->set<StreamMap>(
+ StreamSym, StreamState::getOpened(Desc, NewPossibleErrors.front()));
+ else
+ // Multiple other "errors" possible, new error kind is unknown.
+ // In this case we known later from the last operation's (will be feof
+ // or ferror) PossibleErrors list what errors are still possible. This
+ // should be the same as the content of PossibleErrors at this point.
+ StateFalse = StateFalse->set<StreamMap>(
+ StreamSym, StreamState::getOpened(Desc, StreamState::UnknownError));
+ C.addTransition(StateTrue);
+ C.addTransition(StateFalse);
+ }
} else {
- // Return zero.
- State = State->BindExpr(CE, C.getLocationContext(),
- C.getSValBuilder().makeIntVal(0, false));
+ // We know exactly what the error is.
+ // Make the return value accordingly to the error.
+ if (SS->ErrorState == ErrorKind)
+ State = bindAndAssumeTrue(State, C, CE);
+ else
+ State = bindInt(0, State, C, CE);
+ State = State->set<StreamMap>(StreamSym,
+ StreamState::getOpened(Desc, SS->ErrorState));
+ C.addTransition(State);
}
- C.addTransition(State);
}
void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call,
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
