balazske updated this revision to Diff 250588.
balazske marked 2 inline comments as done.
balazske added a comment.
Addressed some comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75682/new/
https://reviews.llvm.org/D75682
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,17 +19,21 @@
#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;
+#define ASSERT_STREAMSTATE_OPENED \
+ assert(SS->isOpened() && \
+ "Create of error node failed (only way to get this state)?");
+
namespace {
/// Full state information about a stream pointer.
struct StreamState {
/// State of a stream symbol.
+ /// FIXME: We need maybe an "escaped" state later.
enum KindTy {
Opened, /// Stream is opened.
Closed, /// Closed stream (an invalid stream pointer after it was closed).
@@ -37,37 +41,44 @@
} State;
/// The error state of a stream.
+ /// Valid only if the stream is opened.
enum ErrorKindTy {
- NoError, /// No error flag is set or stream is not open.
- EofError, /// EOF condition (`feof` is true).
- OtherError, /// Other (non-EOF) error (`ferror` is true).
- AnyError /// EofError or OtherError, used if exact error is unknown.
+ /// 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,
} ErrorState = NoError;
bool isOpened() const { return State == Opened; }
bool isClosed() const { return State == Closed; }
bool isOpenFailed() const { return State == OpenFailed; }
- bool isNoError() const { return ErrorState == NoError; }
- bool isEofError() const { return ErrorState == EofError; }
- bool isOtherError() const { return ErrorState == OtherError; }
- bool isAnyError() const { return ErrorState == AnyError; }
+ 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 operator==(const StreamState &X) const {
+ // In not opened state error should always NoError.
return 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 getOpenedWithAnyError() {
- return StreamState{Opened, AnyError};
- }
- static StreamState getOpenedWithEofError() {
- return StreamState{Opened, EofError};
- }
- static StreamState getOpenedWithOtherError() {
- return StreamState{Opened, OtherError};
+ static StreamState getOpenedWithFEof() { return StreamState{Opened, FEof}; }
+ static StreamState getOpenedWithFError() {
+ return StreamState{Opened, FError};
}
void Profile(llvm::FoldingSetNodeID &ID) const {
@@ -102,7 +113,7 @@
DefinedSVal makeRetVal(CheckerContext &C, const CallExpr *CE) {
assert(CE && "Expecting a call expression.");
- const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
+ const LocationContext *LCtx = C.getLocationContext();
return C.getSValBuilder()
.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount())
.castAs<DefinedSVal>();
@@ -135,9 +146,12 @@
{{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
{{"clearerr", 1},
{&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0}},
- {{"feof", 1}, {&StreamChecker::preDefault, &StreamChecker::evalFeof, 0}},
+ {{"feof", 1},
+ {&StreamChecker::preDefault,
+ &StreamChecker::evalFeofFerror<&StreamState::isFEof>, 0}},
{{"ferror", 1},
- {&StreamChecker::preDefault, &StreamChecker::evalFerror, 0}},
+ {&StreamChecker::preDefault,
+ &StreamChecker::evalFeofFerror<&StreamState::isFError>, 0}},
{{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0}},
};
@@ -161,11 +175,9 @@
void evalClearerr(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
- void evalFeof(const FnDescription *Desc, const CallEvent &Call,
- CheckerContext &C) const;
-
- void evalFerror(const FnDescription *Desc, const CallEvent &Call,
- CheckerContext &C) const;
+ template <bool (StreamState::*IsOfError)() const>
+ void evalFeofFerror(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
/// Check that the stream (in StreamVal) is not NULL.
/// If it can only be NULL a fatal error is emitted and nullptr returned.
@@ -315,6 +327,8 @@
if (!SS)
return;
+ ASSERT_STREAMSTATE_OPENED;
+
// Close the File Descriptor.
// Regardless if the close fails or not, stream becomes "closed"
// and can not be used any more.
@@ -352,6 +366,8 @@
if (!SS)
return;
+ ASSERT_STREAMSTATE_OPENED;
+
if (SS->isNoError())
return;
@@ -359,50 +375,10 @@
C.addTransition(State);
}
-void StreamChecker::evalFeof(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;
-
- const StreamState *SS = State->get<StreamMap>(StreamSym);
- // Ignore the call if the stream is not tracked.
- if (!SS)
- return;
-
- DefinedSVal RetVal = makeRetVal(C, CE);
-
- // Make expression result.
- State = State->BindExpr(CE, C.getLocationContext(), RetVal);
-
- if (SS->isAnyError()) {
- // We do not know yet what kind of error is set.
- // Differentiate between EOF and other error.
- ProgramStateRef StateEof, StateOther;
- std::tie(StateEof, StateOther) = State->assume(RetVal);
- assert(StateEof && StateOther &&
- "Return value should not be constrained already.");
- StateEof = StateEof->set<StreamMap>(StreamSym,
- StreamState::getOpenedWithEofError());
- StateOther = StateOther->set<StreamMap>(
- StreamSym, StreamState::getOpenedWithOtherError());
- C.addTransition(StateEof);
- C.addTransition(StateOther);
- } else {
- // We know what error is set, make the return value accordingly.
- State = State->assume(RetVal, SS->isEofError());
- assert(State && "Return value should not be constrained already.");
- C.addTransition(State);
- }
-}
-
-void StreamChecker::evalFerror(const FnDescription *Desc, const CallEvent &Call,
- CheckerContext &C) const {
+template <bool (StreamState::*IsOfError)() const>
+void StreamChecker::evalFeofFerror(const FnDescription *Desc,
+ const CallEvent &Call,
+ CheckerContext &C) const {
ProgramStateRef State = C.getState();
SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
if (!StreamSym)
@@ -417,29 +393,20 @@
if (!SS)
return;
- // Make expression result.
- DefinedSVal RetVal = makeRetVal(C, CE);
- State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+ ASSERT_STREAMSTATE_OPENED;
- if (SS->isAnyError()) {
- // We do not know yet what kind of error is set.
- // Differentiate between EOF and other error.
- ProgramStateRef StateEof, StateOther;
- std::tie(StateOther, StateEof) = State->assume(RetVal);
- assert(StateEof && StateOther &&
- "Return value should not be constrained already.");
- StateEof = StateEof->set<StreamMap>(StreamSym,
- StreamState::getOpenedWithEofError());
- StateOther = StateOther->set<StreamMap>(
- StreamSym, StreamState::getOpenedWithOtherError());
- C.addTransition(StateEof);
- C.addTransition(StateOther);
+ 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.");
} else {
- // We know what error is set, make the return value accordingly.
- State = State->assume(RetVal, SS->isOtherError());
- assert(State && "Return value should not be constrained already.");
- C.addTransition(State);
+ // Return zero.
+ State = State->BindExpr(CE, C.getLocationContext(),
+ C.getSValBuilder().makeIntVal(0, false));
}
+ C.addTransition(State);
}
void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
