balazske created this revision.
Herald added subscribers: manas, steakhal, ASDenysPetrov, martong, gamesh411,
dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet,
baloghadamsoftware, xazax.hun, whisperity.
Herald added a reviewer: Szelethus.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Recognize initial value of standard streams (stdin, ...)
and assume that a new opened stream is not equal to these.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D106644
Files:
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream.c
Index: clang/test/Analysis/stream.c
===================================================================
--- clang/test/Analysis/stream.c
+++ clang/test/Analysis/stream.c
@@ -267,3 +267,12 @@
} // expected-warning {{Opened stream never closed. Potential resource leak}}
// FIXME: This warning should be placed at the `return` above.
// See https://reviews.llvm.org/D83120 about details.
+
+void check_fopen_is_not_std(FILE *F, int Std) {
+ if (!Std)
+ F = fopen("file", "r");
+ if (!F)
+ return;
+ if (F != stdin && F != stdout && F != stderr)
+ fclose(F); // no warning: the opened file can not be equal to std streams
+}
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -10,6 +10,7 @@
//
//===----------------------------------------------------------------------===//
+#include "clang/AST/ParentMapContext.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
@@ -157,6 +158,11 @@
// StreamChecker class and utility functions.
//===----------------------------------------------------------------------===//
+// This map holds the state of a stream.
+// The stream is identified with a SymbolRef that is created when a stream
+// opening function is modeled by the checker.
+REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)
+
namespace {
class StreamChecker;
@@ -206,8 +212,47 @@
return State;
}
-class StreamChecker : public Checker<check::PreCall, eval::Call,
- check::DeadSymbols, check::PointerEscape> {
+const VarDecl *findStdStreamDecl(StringRef StdName, CheckerContext &C) {
+ ASTContext &ACtx = C.getASTContext();
+
+ IdentifierInfo &II = ACtx.Idents.get(StdName);
+ auto LookupRes = ACtx.getTranslationUnitDecl()->lookup(&II);
+ QualType FILEType = ACtx.getFILEType();
+ if (!FILEType.isNull())
+ FILEType = ACtx.getPointerType(FILEType);
+
+ for (Decl *D : LookupRes) {
+ D = D->getCanonicalDecl();
+ if (!C.getSourceManager().isInSystemHeader(D->getLocation()))
+ continue;
+ if (auto *VD = dyn_cast<VarDecl>(D)) {
+ if (!FILEType.isNull() && !ACtx.hasSameType(FILEType, VD->getType()))
+ continue;
+ return VD;
+ }
+ }
+
+ return nullptr;
+}
+
+SymbolRef findStdStream(StringRef StdName, CheckerContext &C) {
+ const LocationContext *LCtx = C.getLocationContext();
+
+ const VarDecl *StdDecl = findStdStreamDecl(StdName, C);
+ if (!StdDecl)
+ return nullptr;
+
+ const VarRegion *RegionOfStdStreamVar =
+ C.getState()->getRegion(StdDecl, LCtx);
+ if (!RegionOfStdStreamVar)
+ return nullptr;
+ SVal StdVal = C.getState()->getSVal(RegionOfStdStreamVar, StdDecl->getType());
+ return StdVal.getAsSymbol();
+}
+
+class StreamChecker
+ : public Checker<check::BeginFunction, check::PreCall, eval::Call,
+ check::DeadSymbols, check::PointerEscape> {
BugType BT_FileNull{this, "NULL stream pointer", "Stream handling error"};
BugType BT_UseAfterClose{this, "Closed stream", "Stream handling error"};
BugType BT_UseAfterOpenFailed{this, "Invalid stream",
@@ -221,6 +266,7 @@
/*SuppressOnSink =*/true};
public:
+ void checkBeginFunction(CheckerContext &C) const;
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
@@ -285,6 +331,10 @@
0}},
};
+ mutable SymbolRef OrigStdin;
+ mutable SymbolRef OrigStdout;
+ mutable SymbolRef OrigStderr;
+
void evalFopen(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
@@ -414,16 +464,15 @@
} // end anonymous namespace
-// This map holds the state of a stream.
-// The stream is identified with a SymbolRef that is created when a stream
-// opening function is modeled by the checker.
-REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)
-
inline void assertStreamStateOpened(const StreamState *SS) {
assert(SS->isOpened() &&
"Previous create of error node for non-opened stream failed?");
}
+//===----------------------------------------------------------------------===//
+// Methods of StreamChecker.
+//===----------------------------------------------------------------------===//
+
const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N,
SymbolRef StreamSym,
CheckerContext &C) {
@@ -445,9 +494,14 @@
return nullptr;
}
-//===----------------------------------------------------------------------===//
-// Methods of StreamChecker.
-//===----------------------------------------------------------------------===//
+void StreamChecker::checkBeginFunction(CheckerContext &C) const {
+ if (!C.inTopFrame())
+ return;
+
+ OrigStdin = findStdStream("stdin", C);
+ OrigStdout = findStdStream("stdout", C);
+ OrigStderr = findStdStream("stderr", C);
+}
void StreamChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
@@ -494,6 +548,21 @@
StateNull =
StateNull->set<StreamMap>(RetSym, StreamState::getOpenFailed(Desc));
+ auto AssumeRetValNotEq = [&C, &StateNotNull, RetVal](SymbolRef StdStream) {
+ SVal NotEqS = C.getSValBuilder().evalBinOp(
+ StateNotNull, BO_NE, RetVal,
+ C.getSValBuilder().makeSymbolVal(StdStream),
+ C.getASTContext().getLogicalOperationType());
+ Optional<DefinedOrUnknownSVal> NotEqDef =
+ NotEqS.getAs<DefinedOrUnknownSVal>();
+ if (!NotEqDef)
+ return;
+ StateNotNull = StateNotNull->assume(*NotEqDef, true);
+ };
+ AssumeRetValNotEq(OrigStdin);
+ AssumeRetValNotEq(OrigStdout);
+ AssumeRetValNotEq(OrigStderr);
+
C.addTransition(StateNotNull,
constructNoteTag(C, RetSym, {&BT_ResourceLeak}));
C.addTransition(StateNull, constructNoteTag(C, RetSym, {&BT_FileNull}));
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
