This revision was automatically updated to reflect the committed changes.
Closed by commit rGaf473d0e84f1: [Analyzer][StreamChecker] Adding PreCall and
refactoring (NFC). (authored by balazske).
Changed prior to commit:
https://reviews.llvm.org/D75612?vs=248176&id=248665#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75612/new/
https://reviews.llvm.org/D75612
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
@@ -50,98 +50,140 @@
};
class StreamChecker;
+struct FnDescription;
+using FnCheck = std::function<void(const StreamChecker *, const FnDescription *,
+ const CallEvent &, CheckerContext &)>;
-using FnCheck = std::function<void(const StreamChecker *, const CallEvent &,
- CheckerContext &)>;
+using ArgNoTy = unsigned int;
+static const ArgNoTy ArgNone = std::numeric_limits<ArgNoTy>::max();
struct FnDescription {
+ FnCheck PreFn;
FnCheck EvalFn;
+ ArgNoTy StreamArgNo;
};
-class StreamChecker : public Checker<eval::Call,
- check::DeadSymbols > {
+/// 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) {
+ assert(Desc && Desc->StreamArgNo != ArgNone &&
+ "Try to get a non-existing stream argument.");
+ return Call.getArgSVal(Desc->StreamArgNo);
+}
+
+class StreamChecker
+ : public Checker<check::PreCall, eval::Call, check::DeadSymbols> {
mutable std::unique_ptr<BuiltinBug> BT_nullfp, BT_illegalwhence,
BT_doubleclose, BT_ResourceLeak;
public:
+ void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
private:
-
CallDescriptionMap<FnDescription> FnDescriptions = {
- {{"fopen"}, {&StreamChecker::evalFopen}},
- {{"freopen", 3}, {&StreamChecker::evalFreopen}},
- {{"tmpfile"}, {&StreamChecker::evalFopen}},
- {{"fclose", 1}, {&StreamChecker::evalFclose}},
- {{"fread", 4},
- {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 3)}},
- {{"fwrite", 4},
- {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 3)}},
- {{"fseek", 3}, {&StreamChecker::evalFseek}},
- {{"ftell", 1},
- {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
- {{"rewind", 1},
- {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
- {{"fgetpos", 2},
- {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
- {{"fsetpos", 2},
- {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
- {{"clearerr", 1},
- {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
- {{"feof", 1},
- {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
- {{"ferror", 1},
- {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
- {{"fileno", 1},
- {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
+ {{"fopen"}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
+ {{"freopen", 3},
+ {&StreamChecker::preFreopen, &StreamChecker::evalFreopen, 2}},
+ {{"tmpfile"}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
+ {{"fclose", 1},
+ {&StreamChecker::preFclose, &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}},
+ {{"clearerr", 1}, {&StreamChecker::preDefault, nullptr, 0}},
+ {{"feof", 1}, {&StreamChecker::preDefault, nullptr, 0}},
+ {{"ferror", 1}, {&StreamChecker::preDefault, nullptr, 0}},
+ {{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0}},
};
- void evalFopen(const CallEvent &Call, CheckerContext &C) const;
- void evalFreopen(const CallEvent &Call, CheckerContext &C) const;
- void evalFclose(const CallEvent &Call, CheckerContext &C) const;
- void evalFseek(const CallEvent &Call, CheckerContext &C) const;
- void checkArgNullStream(const CallEvent &Call, CheckerContext &C,
- unsigned ArgI) const;
-
- ProgramStateRef checkNullStream(SVal SV, CheckerContext &C,
- ProgramStateRef State) const;
- ProgramStateRef checkFseekWhence(SVal SV, CheckerContext &C,
- ProgramStateRef State) const;
- ProgramStateRef checkDoubleClose(const CallEvent &Call, CheckerContext &C,
- ProgramStateRef State) const;
+ void evalFopen(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+ void preFreopen(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+ void evalFreopen(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+ void preFclose(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+ void evalFclose(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+ void preFseek(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+ void preDefault(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.
+ /// 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;
+
+ /// Check that the stream is not closed.
+ /// Return a state where the stream is guaranteed to not in closed state
+ /// (if data about it exists).
+ /// Generate error if the stream is provable in closed state.
+ ProgramStateRef ensureStreamNotClosed(SVal StreamVal, CheckerContext &C,
+ ProgramStateRef State) const;
+
+ /// Check the legality of the 'whence' argument of 'fseek'.
+ /// Generate error and return nullptr if it is found to be illegal.
+ /// 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;
+
+ /// Find the description data of the function called by a call event.
+ /// Returns nullptr if no function is recognized.
+ const FnDescription *lookupFn(const CallEvent &Call) const {
+ // Recognize "global C functions" with only integral or pointer arguments
+ // (and matching name) as stream functions.
+ if (!Call.isGlobalCFunction())
+ return nullptr;
+ for (auto P : Call.parameters()) {
+ QualType T = P->getType();
+ if (!T->isIntegralOrEnumerationType() && !T->isPointerType())
+ return nullptr;
+ }
+
+ return FnDescriptions.lookup(Call);
+ }
};
} // end anonymous namespace
REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)
+void StreamChecker::checkPreCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ const FnDescription *Desc = lookupFn(Call);
+ if (!Desc || !Desc->PreFn)
+ return;
-bool StreamChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
- const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
- if (!FD || FD->getKind() != Decl::Function)
- return false;
-
- // Recognize "global C functions" with only integral or pointer arguments
- // (and matching name) as stream functions.
- if (!Call.isGlobalCFunction())
- return false;
- for (auto P : Call.parameters()) {
- QualType T = P->getType();
- if (!T->isIntegralOrEnumerationType() && !T->isPointerType())
- return false;
- }
+ Desc->PreFn(this, Desc, Call, C);
+}
- const FnDescription *Description = FnDescriptions.lookup(Call);
- if (!Description)
+bool StreamChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
+ const FnDescription *Desc = lookupFn(Call);
+ if (!Desc || !Desc->EvalFn)
return false;
- (Description->EvalFn)(this, Call, C);
+ Desc->EvalFn(this, Desc, Call, C);
return C.isDifferent();
}
-void StreamChecker::evalFopen(const CallEvent &Call, CheckerContext &C) const {
+void StreamChecker::evalFopen(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
ProgramStateRef State = C.getState();
SValBuilder &SVB = C.getSValBuilder();
const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
@@ -170,7 +212,19 @@
C.addTransition(StateNull);
}
-void StreamChecker::evalFreopen(const CallEvent &Call,
+void StreamChecker::preFreopen(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+ // Do not allow NULL as passed stream pointer but allow a closed stream.
+ ProgramStateRef State = C.getState();
+ State = ensureStreamNonNull(getStreamArg(Desc, Call), C, State);
+ if (!State)
+ return;
+
+ C.addTransition(State);
+}
+
+void StreamChecker::evalFreopen(const FnDescription *Desc,
+ const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
@@ -178,17 +232,14 @@
if (!CE)
return;
- Optional<DefinedSVal> StreamVal = Call.getArgSVal(2).getAs<DefinedSVal>();
+ Optional<DefinedSVal> StreamVal =
+ getStreamArg(Desc, Call).getAs<DefinedSVal>();
if (!StreamVal)
return;
- // Do not allow NULL as passed stream pointer.
- // This is not specified in the man page but may crash on some system.
- State = checkNullStream(*StreamVal, C, State);
- if (!State)
- return;
SymbolRef StreamSym = StreamVal->getAsSymbol();
- // Do not care about special values for stream ("(FILE *)0x12345"?).
+ // Do not care about concrete values for stream ("(FILE *)0x12345"?).
+ // FIXME: Are stdin, stdout, stderr such values?
if (!StreamSym)
return;
@@ -212,49 +263,71 @@
C.addTransition(StateRetNull);
}
-void StreamChecker::evalFclose(const CallEvent &Call, CheckerContext &C) const {
+void StreamChecker::preFclose(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
ProgramStateRef State = C.getState();
- State = checkDoubleClose(Call, C, State);
- if (State)
- C.addTransition(State);
+ State = ensureStreamNotClosed(getStreamArg(Desc, Call), C, State);
+ if (!State)
+ return;
+
+ C.addTransition(State);
}
-void StreamChecker::evalFseek(const CallEvent &Call, CheckerContext &C) const {
- const Expr *AE2 = Call.getArgExpr(2);
- if (!AE2)
+void StreamChecker::evalFclose(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef Sym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!Sym)
return;
- ProgramStateRef State = C.getState();
+ const StreamState *SS = State->get<StreamMap>(Sym);
+ if (!SS)
+ return;
- State = checkNullStream(Call.getArgSVal(0), C, State);
+ // 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());
+
+ C.addTransition(State);
+}
+
+void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ State = ensureStreamNonNull(StreamVal, C, State);
if (!State)
return;
-
- State =
- checkFseekWhence(State->getSVal(AE2, C.getLocationContext()), C, State);
+ State = ensureFseekWhenceCorrect(Call.getArgSVal(2), C, State);
if (!State)
return;
C.addTransition(State);
}
-void StreamChecker::checkArgNullStream(const CallEvent &Call, CheckerContext &C,
- unsigned ArgI) const {
+void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
ProgramStateRef State = C.getState();
- State = checkNullStream(Call.getArgSVal(ArgI), C, State);
- if (State)
- C.addTransition(State);
+ SVal StreamVal = getStreamArg(Desc, Call);
+ State = ensureStreamNonNull(StreamVal, C, State);
+ if (!State)
+ return;
+
+ C.addTransition(State);
}
-ProgramStateRef StreamChecker::checkNullStream(SVal SV, CheckerContext &C,
- ProgramStateRef State) const {
- Optional<DefinedSVal> DV = SV.getAs<DefinedSVal>();
- if (!DV)
+ProgramStateRef
+StreamChecker::ensureStreamNonNull(SVal StreamVal, CheckerContext &C,
+ ProgramStateRef State) const {
+ auto Stream = StreamVal.getAs<DefinedSVal>();
+ if (!Stream)
return State;
ConstraintManager &CM = C.getConstraintManager();
+
ProgramStateRef StateNotNull, StateNull;
- std::tie(StateNotNull, StateNull) = CM.assumeDual(C.getState(), *DV);
+ std::tie(StateNotNull, StateNull) = CM.assumeDual(C.getState(), *Stream);
if (!StateNotNull && StateNull) {
if (ExplodedNode *N = C.generateErrorNode(StateNull)) {
@@ -270,41 +343,14 @@
return StateNotNull;
}
-// Check the legality of the 'whence' argument of 'fseek'.
-ProgramStateRef StreamChecker::checkFseekWhence(SVal SV, CheckerContext &C,
- ProgramStateRef State) const {
- Optional<nonloc::ConcreteInt> CI = SV.getAs<nonloc::ConcreteInt>();
- if (!CI)
- return State;
-
- int64_t X = CI->getValue().getSExtValue();
- if (X >= 0 && X <= 2)
- return State;
-
- if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) {
- if (!BT_illegalwhence)
- BT_illegalwhence.reset(
- new BuiltinBug(this, "Illegal whence argument",
- "The whence argument to fseek() should be "
- "SEEK_SET, SEEK_END, or SEEK_CUR."));
- C.emitReport(std::make_unique<PathSensitiveBugReport>(
- *BT_illegalwhence, BT_illegalwhence->getDescription(), N));
- return nullptr;
- }
-
- return State;
-}
-
-ProgramStateRef StreamChecker::checkDoubleClose(const CallEvent &Call,
- CheckerContext &C,
- ProgramStateRef State) const {
- SymbolRef Sym = Call.getArgSVal(0).getAsSymbol();
+ProgramStateRef
+StreamChecker::ensureStreamNotClosed(SVal StreamVal, CheckerContext &C,
+ ProgramStateRef State) const {
+ SymbolRef Sym = StreamVal.getAsSymbol();
if (!Sym)
return State;
const StreamState *SS = State->get<StreamMap>(Sym);
-
- // If the file stream is not tracked, return.
if (!SS)
return State;
@@ -325,8 +371,30 @@
return State;
}
- // Close the File Descriptor.
- State = State->set<StreamMap>(Sym, StreamState::getClosed());
+ return State;
+}
+
+ProgramStateRef
+StreamChecker::ensureFseekWhenceCorrect(SVal WhenceVal, CheckerContext &C,
+ ProgramStateRef State) const {
+ Optional<nonloc::ConcreteInt> CI = WhenceVal.getAs<nonloc::ConcreteInt>();
+ if (!CI)
+ return State;
+
+ int64_t X = CI->getValue().getSExtValue();
+ if (X >= 0 && X <= 2)
+ return State;
+
+ if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) {
+ if (!BT_illegalwhence)
+ BT_illegalwhence.reset(
+ new BuiltinBug(this, "Illegal whence argument",
+ "The whence argument to fseek() should be "
+ "SEEK_SET, SEEK_END, or SEEK_CUR."));
+ C.emitReport(std::make_unique<PathSensitiveBugReport>(
+ *BT_illegalwhence, BT_illegalwhence->getDescription(), N));
+ return nullptr;
+ }
return State;
}
@@ -337,7 +405,7 @@
// TODO: Clean up the state.
const StreamMapTy &Map = State->get<StreamMap>();
- for (const auto &I: Map) {
+ for (const auto &I : Map) {
SymbolRef Sym = I.first;
const StreamState &SS = I.second;
if (!SymReaper.isDead(Sym) || !SS.isOpened())
@@ -360,6 +428,4 @@
mgr.registerChecker<StreamChecker>();
}
-bool ento::shouldRegisterStreamChecker(const LangOptions &LO) {
- return true;
-}
+bool ento::shouldRegisterStreamChecker(const LangOptions &LO) { return true; }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
