[clang] 22d40cc - [Analyzer][StreamChecker] Changed representation of stream error state - NFC.
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Mon May 18 07:18:07 PDT 2020
Author: Balázs Kéri
Date: 2020-05-18T16:18:59+02:00
New Revision: 22d40cc3a724fa3df259c52009571a21a3a3a632
URL: https://github.com/llvm/llvm-project/commit/22d40cc3a724fa3df259c52009571a21a3a3a632
DIFF: https://github.com/llvm/llvm-project/commit/22d40cc3a724fa3df259c52009571a21a3a3a632.diff
LOG: [Analyzer][StreamChecker] Changed representation of stream error state - NFC.
Summary:
State of error flags for a stream is handled by having separate flags
that allow combination of multiple error states to be described with one
error state object.
After a failed function the error state is set in the stream state
and must not be determined later based on the last failed function
like before this change. The error state can not always be determined
from the last failed function and it was not the best design.
Reviewers: Szelethus
Reviewed By: Szelethus
Subscribers: 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/D80009
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 76dd62d30ddf..d079951221d0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/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 @@ struct StreamState {
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 @@ struct StreamState {
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 @@ 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) {
@@ -193,49 +183,42 @@ 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,
- &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 @@ class StreamChecker
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 @@ void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
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 @@ void StreamChecker::evalClearerr(const FnDescription *Desc,
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 @@ 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);
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 @@ void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call,
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);
}
More information about the cfe-commits
mailing list