[clang] f2b5e60 - [Analyzer][StreamChecker] Added evaluation of fseek.
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 14 03:36:06 PDT 2020
Author: Balázs Kéri
Date: 2020-04-14T12:35:28+02:00
New Revision: f2b5e60dfd09f99d036197c078179c774f8b4582
URL: https://github.com/llvm/llvm-project/commit/f2b5e60dfd09f99d036197c078179c774f8b4582
DIFF: https://github.com/llvm/llvm-project/commit/f2b5e60dfd09f99d036197c078179c774f8b4582.diff
LOG: [Analyzer][StreamChecker] Added evaluation of fseek.
Summary:
Function `fseek` is now evaluated with setting error return value
and error flags.
Reviewers: Szelethus, NoQ, xazax.hun, rnkovacs, dcoughlin, baloghadamsoftware, martong
Reviewed By: Szelethus
Subscribers: ASDenysPetrov, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D75851
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream-error.c
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index b38a645432f4..76dd62d30ddf 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -25,8 +25,13 @@ using namespace ento;
namespace {
+struct FnDescription;
+
/// Full state information about a stream pointer.
struct StreamState {
+ /// The last file operation called in the stream.
+ const FnDescription *LastOperation;
+
/// State of a stream symbol.
/// FIXME: We need maybe an "escaped" state later.
enum KindTy {
@@ -45,6 +50,9 @@ struct StreamState {
FEof,
/// Other generic (non-EOF) error (`ferror` is true).
FError,
+ /// Unknown error flag is set (or none), the meaning depends on the last
+ /// operation.
+ Unknown
} ErrorState = NoError;
bool isOpened() const { return State == Opened; }
@@ -63,28 +71,47 @@ struct StreamState {
assert(State == Opened && "Error undefined for closed stream.");
return ErrorState == FError;
}
+ bool isUnknown() const {
+ assert(State == Opened && "Error undefined for closed stream.");
+ return ErrorState == Unknown;
+ }
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 getOpened(const FnDescription *L, ErrorKindTy E) {
+ return StreamState{L, Opened, E};
+ }
+ static StreamState getClosed(const FnDescription *L) {
+ return StreamState{L, Closed};
}
+ static StreamState getOpenFailed(const FnDescription *L) {
+ return StreamState{L, OpenFailed};
+ }
+
+ /// Return if the specified error kind is possible on the stream in the
+ /// current state.
+ /// This depends on the stored `LastOperation` value.
+ /// If the error is not possible returns empty value.
+ /// If the error is possible returns the remaining possible error type
+ /// (after taking out `ErrorKind`). If a single error is possible it will
+ /// return that value, otherwise unknown error.
+ Optional<ErrorKindTy> getRemainingPossibleError(ErrorKindTy ErrorKind) const;
void Profile(llvm::FoldingSetNodeID &ID) const {
+ ID.AddPointer(LastOperation);
ID.AddInteger(State);
ID.AddInteger(ErrorState);
}
};
class StreamChecker;
-struct FnDescription;
using FnCheck = std::function<void(const StreamChecker *, const FnDescription *,
const CallEvent &, CheckerContext &)>;
@@ -95,8 +122,28 @@ struct FnDescription {
FnCheck PreFn;
FnCheck EvalFn;
ArgNoTy StreamArgNo;
+ // What errors are possible after this operation.
+ // Used only if this operation resulted in Unknown state
+ // (otherwise there is a known single error).
+ // Must contain 2 or 3 elements, or zero.
+ llvm::SmallVector<StreamState::ErrorKindTy, 3> PossibleErrors = {};
};
+Optional<StreamState::ErrorKindTy>
+StreamState::getRemainingPossibleError(ErrorKindTy ErrorKind) const {
+ assert(ErrorState == Unknown &&
+ "Function to be used only if error is unknown.");
+ llvm::SmallVector<StreamState::ErrorKindTy, 3> NewPossibleErrors;
+ for (StreamState::ErrorKindTy E : LastOperation->PossibleErrors)
+ if (E != ErrorKind)
+ NewPossibleErrors.push_back(E);
+ if (NewPossibleErrors.size() == LastOperation->PossibleErrors.size())
+ return {};
+ if (NewPossibleErrors.size() == 1)
+ return NewPossibleErrors.front();
+ return Unknown;
+}
+
/// Get the value of the stream argument out of the passed call event.
/// The call should contain a function that is described by Desc.
SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
@@ -115,6 +162,22 @@ DefinedSVal makeRetVal(CheckerContext &C, const CallExpr *CE) {
.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,
@@ -130,38 +193,49 @@ class StreamChecker
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 Unknown 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 Unknown 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, {}}},
};
CallDescriptionMap<FnDescription> FnTestDescriptions = {
{{"StreamTesterChecker_make_feof_stream", 1},
- {nullptr,
- &StreamChecker::evalSetFeofFerror<&StreamState::getOpenedWithFEof>, 0}},
+ {nullptr, &StreamChecker::evalSetFeofFerror<StreamState::FEof>, 0}},
{{"StreamTesterChecker_make_ferror_stream", 1},
- {nullptr,
- &StreamChecker::evalSetFeofFerror<&StreamState::getOpenedWithFError>,
- 0}},
+ {nullptr, &StreamChecker::evalSetFeofFerror<StreamState::FError>, 0}},
};
void evalFopen(const FnDescription *Desc, const CallEvent &Call,
@@ -177,6 +251,8 @@ class StreamChecker
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;
@@ -184,11 +260,11 @@ class StreamChecker
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;
- template <StreamState (*GetState)()>
+ template <StreamState::ErrorKindTy EK>
void evalSetFeofFerror(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
@@ -197,7 +273,7 @@ class StreamChecker
/// Otherwise the return value is a new state where the stream is constrained
/// to be non-null.
ProgramStateRef ensureStreamNonNull(SVal StreamVal, CheckerContext &C,
- ProgramStateRef State) const;
+ ProgramStateRef State) const;
/// Check that the stream is the opened state.
/// If the stream is known to be not opened an error is generated
@@ -210,7 +286,7 @@ class StreamChecker
/// Otherwise returns the state.
/// (State is not changed here because the "whence" value is already known.)
ProgramStateRef ensureFseekWhenceCorrect(SVal WhenceVal, CheckerContext &C,
- ProgramStateRef State) const;
+ ProgramStateRef State) const;
/// Find the description data of the function called by a call event.
/// Returns nullptr if no function is recognized.
@@ -226,7 +302,7 @@ class StreamChecker
}
return FnDescriptions.lookup(Call);
- }
+ }
};
} // end anonymous namespace
@@ -278,8 +354,10 @@ void StreamChecker::evalFopen(const FnDescription *Desc, const CallEvent &Call,
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);
@@ -328,9 +406,9 @@ void StreamChecker::evalFreopen(const FnDescription *Desc,
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);
@@ -352,7 +430,7 @@ void StreamChecker::evalFclose(const FnDescription *Desc, const CallEvent &Call,
// 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);
}
@@ -374,6 +452,43 @@ void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
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::getOpened(Desc, StreamState::Unknown));
+
+ C.addTransition(StateNotFailed);
+ C.addTransition(StateFailed);
+}
+
void StreamChecker::evalClearerr(const FnDescription *Desc,
const CallEvent &Call,
CheckerContext &C) const {
@@ -388,14 +503,11 @@ void StreamChecker::evalClearerr(const FnDescription *Desc,
assertStreamStateOpened(SS);
- 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 {
@@ -408,25 +520,43 @@ void StreamChecker::evalFeofFerror(const FnDescription *Desc,
if (!CE)
return;
+ auto AddTransition = [&C, Desc, StreamSym](ProgramStateRef State,
+ StreamState::ErrorKindTy SSError) {
+ StreamState SSNew = StreamState::getOpened(Desc, SSError);
+ State = State->set<StreamMap>(StreamSym, SSNew);
+ C.addTransition(State);
+ };
+
const StreamState *SS = State->get<StreamMap>(StreamSym);
- // Ignore the call if the stream is not tracked.
if (!SS)
return;
assertStreamStateOpened(SS);
- 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->isUnknown()) {
+ // The stream is in exactly known state (error or not).
+ // Check if it is in error of kind `ErrorKind`.
+ if (SS->ErrorState == ErrorKind)
+ State = bindAndAssumeTrue(State, C, CE);
+ else
+ State = bindInt(0, State, C, CE);
+ // Error state is not changed in the new state.
+ AddTransition(State, SS->ErrorState);
} else {
- // Return zero.
- State = State->BindExpr(CE, C.getLocationContext(),
- C.getSValBuilder().makeIntVal(0, false));
+ // Stream is in unknown state, check if error `ErrorKind` is possible.
+ Optional<StreamState::ErrorKindTy> NewError =
+ SS->getRemainingPossibleError(ErrorKind);
+ if (!NewError) {
+ // This kind of error is not possible, function returns zero.
+ // Error state remains unknown.
+ AddTransition(bindInt(0, State, C, CE), StreamState::Unknown);
+ } else {
+ // Add state with true returned.
+ AddTransition(bindAndAssumeTrue(State, C, CE), ErrorKind);
+ // Add state with false returned and the new remaining error type.
+ AddTransition(bindInt(0, State, C, CE), *NewError);
+ }
}
- C.addTransition(State);
}
void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call,
@@ -443,14 +573,17 @@ void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call,
C.addTransition(State);
}
-template <StreamState (*GetState)()>
+template <StreamState::ErrorKindTy EK>
void StreamChecker::evalSetFeofFerror(const FnDescription *Desc,
const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
assert(StreamSym && "Operation not permitted on non-symbolic stream value.");
- State = State->set<StreamMap>(StreamSym, (*GetState)());
+ const StreamState *SS = State->get<StreamMap>(StreamSym);
+ assert(SS && "Stream should be tracked by the checker.");
+ State = State->set<StreamMap>(StreamSym,
+ StreamState::getOpened(SS->LastOperation, EK));
C.addTransition(State);
}
@@ -597,4 +730,3 @@ void ento::registerStreamTesterChecker(CheckerManager &Mgr) {
bool ento::shouldRegisterStreamTesterChecker(const CheckerManager &Mgr) {
return true;
}
-
diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c
index e9fb8fdebfab..2bd25ca30fa3 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -52,3 +52,34 @@ void stream_error_ferror() {
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
fclose(F);
}
+
+void error_fseek() {
+ FILE *F = fopen("file", "r");
+ if (!F)
+ return;
+ int rc = fseek(F, 0, SEEK_SET);
+ if (rc) {
+ int IsFEof = feof(F), IsFError = ferror(F);
+ // Get feof or ferror or no error.
+ clang_analyzer_eval(IsFEof || IsFError);
+ // expected-warning at -1 {{FALSE}}
+ // expected-warning at -2 {{TRUE}}
+ clang_analyzer_eval(IsFEof && IsFError); // expected-warning {{FALSE}}
+ // Error flags should not change.
+ if (IsFEof)
+ clang_analyzer_eval(feof(F)); // expected-warning {{TRUE}}
+ else
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ if (IsFError)
+ 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);
+}
More information about the cfe-commits
mailing list