balazske updated this revision to Diff 255360.
balazske added a comment.
- Adding comment about ferror, feof.
- Enabled core checker in test file.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75682/new/
https://reviews.llvm.org/D75682
Files:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
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
@@ -1,8 +1,10 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-checker=debug.ExprInspection -analyzer-store region -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.StreamTester -analyzer-checker=debug.ExprInspection -analyzer-store region -verify %s
#include "Inputs/system-header-simulator.h"
void clang_analyzer_eval(int);
+void StreamTesterChecker_make_feof_stream(FILE *);
+void StreamTesterChecker_make_ferror_stream(FILE *);
void error_fopen() {
FILE *F = fopen("file", "r");
@@ -25,16 +27,28 @@
fclose(F);
}
-void error_clearerr() {
+void stream_error_feof() {
FILE *F = fopen("file", "r");
if (!F)
return;
- int ch = fputc('a', F);
- if (ch == EOF) {
- // FIXME: fputc not handled by checker yet, should expect TRUE
- clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
- clearerr(F);
- clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
- }
+ StreamTesterChecker_make_feof_stream(F);
+ clang_analyzer_eval(feof(F)); // expected-warning {{TRUE}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+ clearerr(F);
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+ fclose(F);
+}
+
+void stream_error_ferror() {
+ FILE *F = fopen("file", "r");
+ if (!F)
+ return;
+ StreamTesterChecker_make_ferror_stream(F);
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+ clearerr(F);
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ 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
@@ -19,17 +19,20 @@
#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 +40,45 @@
} 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 {
- 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>();
@@ -118,6 +129,9 @@
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
+ /// If true, evaluate special testing stream functions.
+ bool TestMode = false;
+
private:
CallDescriptionMap<FnDescription> FnDescriptions = {
{{"fopen"}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
@@ -135,12 +149,25 @@
{{"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}},
};
+ CallDescriptionMap<FnDescription> FnTestDescriptions = {
+ {{"StreamTesterChecker_make_feof_stream", 1},
+ {nullptr,
+ &StreamChecker::evalSetFeofFerror<&StreamState::getOpenedWithFEof>, 0}},
+ {{"StreamTesterChecker_make_ferror_stream", 1},
+ {nullptr,
+ &StreamChecker::evalSetFeofFerror<&StreamState::getOpenedWithFError>,
+ 0}},
+ };
+
void evalFopen(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
@@ -161,11 +188,13 @@
void evalClearerr(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
- void evalFeof(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;
- void evalFerror(const FnDescription *Desc, const CallEvent &Call,
- CheckerContext &C) const;
+ template <StreamState (*GetState)()>
+ void evalSetFeofFerror(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.
@@ -219,6 +248,8 @@
bool StreamChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
const FnDescription *Desc = lookupFn(Call);
+ if (!Desc && TestMode)
+ Desc = FnTestDescriptions.lookup(Call);
if (!Desc || !Desc->EvalFn)
return false;
@@ -315,6 +346,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 +385,8 @@
if (!SS)
return;
+ ASSERT_STREAMSTATE_OPENED;
+
if (SS->isNoError())
return;
@@ -359,8 +394,10 @@
C.addTransition(State);
}
-void StreamChecker::evalFeof(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)
@@ -375,71 +412,20 @@
if (!SS)
return;
- DefinedSVal RetVal = makeRetVal(C, CE);
-
- // Make expression result.
- 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(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);
+ 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->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 {
- 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;
-
- // Make expression result.
- DefinedSVal RetVal = makeRetVal(C, CE);
- 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(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);
- } 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,
@@ -456,6 +442,17 @@
C.addTransition(State);
}
+template <StreamState (*GetState)()>
+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)());
+ C.addTransition(State);
+}
+
ProgramStateRef
StreamChecker::ensureStreamNonNull(SVal StreamVal, CheckerContext &C,
ProgramStateRef State) const {
@@ -583,8 +580,17 @@
}
}
-void ento::registerStreamChecker(CheckerManager &mgr) {
- mgr.registerChecker<StreamChecker>();
+void ento::registerStreamChecker(CheckerManager &Mgr) {
+ Mgr.registerChecker<StreamChecker>();
}
bool ento::shouldRegisterStreamChecker(const LangOptions &LO) { return true; }
+
+void ento::registerStreamTesterChecker(CheckerManager &Mgr) {
+ auto *Checker = Mgr.getChecker<StreamChecker>();
+ Checker->TestMode = true;
+}
+
+bool ento::shouldRegisterStreamTesterChecker(const LangOptions &LO) {
+ return true;
+}
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -452,6 +452,11 @@
HelpText<"Check stream handling functions">,
Documentation<HasAlphaDocumentation>;
+def StreamTesterChecker : Checker<"StreamTester">,
+ HelpText<"Add test functions to StreamChecker for test and debugging purposes.">,
+ Dependencies<[StreamChecker]>,
+ Documentation<NotDocumented>;
+
def SimpleStreamChecker : Checker<"SimpleStream">,
HelpText<"Check for misuses of stream APIs">,
Documentation<HasAlphaDocumentation>;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
