This revision was automatically updated to reflect the committed changes.
Closed by commit rG22d40cc3a724: [Analyzer][StreamChecker] Changed
representation of stream error state - NFC. (authored by balazske).
Changed prior to commit:
https://reviews.llvm.org/D80009?vs=264230&id=264618#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80009/new/
https://reviews.llvm.org/D80009
Files:
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -19,14 +19,62 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include <functional>
using namespace clang;
using namespace ento;
+using namespace std::placeholders;
namespace {
struct FnDescription;
+/// State of the stream error flags.
+/// Sometimes it is not known to the checker what error flags are set.
+/// This is indicated by setting more than one flag to true.
+/// This is an optimization to avoid state splits.
+/// A stream can either be in FEOF or FERROR but not both at the same time.
+/// Multiple flags are set to handle the corresponding states together.
+struct StreamErrorState {
+ /// The stream can be in state where none of the error flags set.
+ bool NoError = true;
+ /// The stream can be in state where the EOF indicator is set.
+ bool FEof = false;
+ /// The stream can be in state where the error indicator is set.
+ bool FError = false;
+
+ bool isNoError() const { return NoError && !FEof && !FError; }
+ bool isFEof() const { return !NoError && FEof && !FError; }
+ bool isFError() const { return !NoError && !FEof && FError; }
+
+ bool operator==(const StreamErrorState &ES) const {
+ return NoError == ES.NoError && FEof == ES.FEof && FError == ES.FError;
+ }
+
+ StreamErrorState operator|(const StreamErrorState &E) const {
+ return {NoError || E.NoError, FEof || E.FEof, FError || E.FError};
+ }
+
+ StreamErrorState operator&(const StreamErrorState &E) const {
+ return {NoError && E.NoError, FEof && E.FEof, FError && E.FError};
+ }
+
+ StreamErrorState operator~() const { return {!NoError, !FEof, !FError}; }
+
+ /// Returns if the StreamErrorState is a valid object.
+ operator bool() const { return NoError || FEof || FError; }
+
+ void Profile(llvm::FoldingSetNodeID &ID) const {
+ ID.AddBoolean(NoError);
+ ID.AddBoolean(FEof);
+ ID.AddBoolean(FError);
+ }
+};
+
+const StreamErrorState ErrorNone{true, false, false};
+const StreamErrorState ErrorFEof{false, true, false};
+const StreamErrorState ErrorFError{false, false, true};
+
/// Full state information about a stream pointer.
struct StreamState {
/// The last file operation called in the stream.
@@ -40,53 +88,24 @@
OpenFailed /// The last open operation has failed.
} State;
- /// The error state of a stream.
- /// Valid only if the stream is opened.
- /// It is assumed that feof and ferror flags are never true at the same time.
- enum ErrorKindTy {
- /// No error flag is set (or stream is not open).
- NoError,
- /// EOF condition (`feof` is true).
- 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;
+ /// State of the error flags.
+ /// Ignored in non-opened stream state but must be NoError.
+ StreamErrorState ErrorState;
bool isOpened() const { return State == Opened; }
bool isClosed() const { return State == Closed; }
bool isOpenFailed() const { return State == OpenFailed; }
- bool isNoError() const {
- assert(State == Opened && "Error undefined for closed stream.");
- return ErrorState == NoError;
- }
- bool isFEof() const {
- assert(State == Opened && "Error undefined for closed stream.");
- return ErrorState == FEof;
- }
- bool isFError() const {
- 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.
+ // 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;
}
- 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 getOpened(const FnDescription *L,
+ const StreamErrorState &ES = {}) {
+ return StreamState{L, Opened, ES};
}
static StreamState getClosed(const FnDescription *L) {
return StreamState{L, Closed};
@@ -95,15 +114,6 @@
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);
@@ -122,28 +132,8 @@
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) {
@@ -193,49 +183,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,
- &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, {}}},
+ {&StreamChecker::preDefault, &StreamChecker::evalFclose, 0}},
+ {{"fread", 4}, {&StreamChecker::preDefault, nullptr, 3}},
+ {{"fwrite", 4}, {&StreamChecker::preDefault, nullptr, 3}},
+ {{"fseek", 3}, {&StreamChecker::preFseek, &StreamChecker::evalFseek, 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}},
{{"clearerr", 1},
- {&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.
+ {&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0}},
{{"feof", 1},
{&StreamChecker::preDefault,
- &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.
+ std::bind(&StreamChecker::evalFeofFerror, _1, _2, _3, _4, ErrorFEof),
+ 0}},
{{"ferror", 1},
{&StreamChecker::preDefault,
- &StreamChecker::evalFeofFerror<StreamState::FError>,
- 0,
- {StreamState::FEof, StreamState::NoError}}},
- {{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0, {}}},
+ std::bind(&StreamChecker::evalFeofFerror, _1, _2, _3, _4, ErrorFError),
+ 0}},
+ {{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0}},
};
CallDescriptionMap<FnDescription> FnTestDescriptions = {
{{"StreamTesterChecker_make_feof_stream", 1},
- {nullptr, &StreamChecker::evalSetFeofFerror<StreamState::FEof>, 0}},
+ {nullptr,
+ std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4, ErrorFEof),
+ 0}},
{{"StreamTesterChecker_make_ferror_stream", 1},
- {nullptr, &StreamChecker::evalSetFeofFerror<StreamState::FError>, 0}},
+ {nullptr,
+ std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4,
+ ErrorFError),
+ 0}},
};
void evalFopen(const FnDescription *Desc, const CallEvent &Call,
@@ -260,13 +243,13 @@
void evalClearerr(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
- template <StreamState::ErrorKindTy ErrorKind>
void evalFeofFerror(const FnDescription *Desc, const CallEvent &Call,
- CheckerContext &C) const;
+ CheckerContext &C,
+ const StreamErrorState &ErrorKind) const;
- template <StreamState::ErrorKindTy EK>
void evalSetFeofFerror(const FnDescription *Desc, const CallEvent &Call,
- CheckerContext &C) const;
+ CheckerContext &C,
+ const StreamErrorState &ErrorKind) const;
/// Check that the stream (in StreamVal) is not NULL.
/// If it can only be NULL a fatal error is emitted and nullptr returned.
@@ -482,8 +465,10 @@
StateNotFailed =
StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
// We get error.
+ // It is possible that fseek fails but sets none of the error flags.
StateFailed = StateFailed->set<StreamMap>(
- StreamSym, StreamState::getOpened(Desc, StreamState::Unknown));
+ StreamSym,
+ StreamState::getOpened(Desc, ErrorNone | ErrorFEof | ErrorFError));
C.addTransition(StateNotFailed);
C.addTransition(StateFailed);
@@ -507,10 +492,9 @@
C.addTransition(State);
}
-template <StreamState::ErrorKindTy ErrorKind>
void StreamChecker::evalFeofFerror(const FnDescription *Desc,
- const CallEvent &Call,
- CheckerContext &C) const {
+ const CallEvent &Call, CheckerContext &C,
+ const StreamErrorState &ErrorKind) const {
ProgramStateRef State = C.getState();
SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
if (!StreamSym)
@@ -520,42 +504,27 @@
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);
if (!SS)
return;
assertStreamStateOpened(SS);
- 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 {
- // 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);
- }
+ if (SS->ErrorState & ErrorKind) {
+ // Execution path with error of ErrorKind.
+ // Function returns true.
+ // 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)));
+ }
+ if (StreamErrorState NewES = SS->ErrorState & (~ErrorKind)) {
+ // Execution path(s) with ErrorKind not set.
+ // Function returns false.
+ // 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)));
}
}
@@ -573,17 +542,16 @@
C.addTransition(State);
}
-template <StreamState::ErrorKindTy EK>
void StreamChecker::evalSetFeofFerror(const FnDescription *Desc,
- const CallEvent &Call,
- CheckerContext &C) const {
+ const CallEvent &Call, CheckerContext &C,
+ const StreamErrorState &ErrorKind) const {
ProgramStateRef State = C.getState();
SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
assert(StreamSym && "Operation not permitted on non-symbolic stream value.");
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));
+ State = State->set<StreamMap>(
+ StreamSym, StreamState::getOpened(SS->LastOperation, ErrorKind));
C.addTransition(State);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
