Author: Balázs Kéri Date: 2020-06-15T15:43:23+02:00 New Revision: efa8b6e884aa4c3ad74ab44489352c0f80ac9741
URL: https://github.com/llvm/llvm-project/commit/efa8b6e884aa4c3ad74ab44489352c0f80ac9741 DIFF: https://github.com/llvm/llvm-project/commit/efa8b6e884aa4c3ad74ab44489352c0f80ac9741.diff LOG: [Analyzer][StreamChecker] Add check for pointer escape. Summary: After an escaped FILE* stream handle it is not possible to make reliable checks on it because any function call can have effect on it. Reviewers: Szelethus, baloghadamsoftware, martong, NoQ Reviewed By: NoQ Subscribers: NoQ, rnkovacs, 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/D80699 Added: Modified: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp clang/test/Analysis/stream.c Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index bfb788c24e24..7e10b2aa4356 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -193,8 +193,8 @@ ProgramStateRef bindInt(uint64_t Value, ProgramStateRef State, return State; } -class StreamChecker - : public Checker<check::PreCall, eval::Call, check::DeadSymbols> { +class StreamChecker : public Checker<check::PreCall, eval::Call, + check::DeadSymbols, check::PointerEscape> { BuiltinBug BT_FileNull{this, "NULL stream pointer", "Stream pointer might be NULL."}; BuiltinBug BT_UseAfterClose{ @@ -223,6 +223,10 @@ class StreamChecker void checkPreCall(const CallEvent &Call, CheckerContext &C) const; bool evalCall(const CallEvent &Call, CheckerContext &C) const; void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const; + ProgramStateRef checkPointerEscape(ProgramStateRef State, + const InvalidatedSymbols &Escaped, + const CallEvent *Call, + PointerEscapeKind Kind) const; /// If true, evaluate special testing stream functions. bool TestMode = false; @@ -448,10 +452,14 @@ void StreamChecker::evalFreopen(const FnDescription *Desc, SymbolRef StreamSym = StreamVal->getAsSymbol(); // Do not care about concrete values for stream ("(FILE *)0x12345"?). - // FIXME: Are stdin, stdout, stderr such values? + // FIXME: Can be stdin, stdout, stderr such values? if (!StreamSym) return; + // Do not handle untracked stream. It is probably escaped. + if (!State->get<StreamMap>(StreamSym)) + return; + // Generate state for non-failed case. // Return value is the passed stream pointer. // According to the documentations, the stream is closed first @@ -918,6 +926,28 @@ void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper, } } +ProgramStateRef StreamChecker::checkPointerEscape( + ProgramStateRef State, const InvalidatedSymbols &Escaped, + const CallEvent *Call, PointerEscapeKind Kind) const { + // Check for file-handling system call that is not handled by the checker. + // FIXME: The checker should be updated to handle all system calls that take + // 'FILE*' argument. These are now ignored. + if (Kind == PSK_DirectEscapeOnCall && Call->isInSystemHeader()) + return State; + + for (SymbolRef Sym : Escaped) { + // The symbol escaped. + // From now the stream can be manipulated in unknown way to the checker, + // it is not possible to handle it any more. + // Optimistically, assume that the corresponding file handle will be closed + // somewhere else. + // Remove symbol from state so the following stream calls on this symbol are + // not handled by the checker. + State = State->remove<StreamMap>(Sym); + } + return State; +} + void ento::registerStreamChecker(CheckerManager &Mgr) { Mgr.registerChecker<StreamChecker>(); } diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c index 554a234c20d5..dbbcc8715e0d 100644 --- a/clang/test/Analysis/stream.c +++ b/clang/test/Analysis/stream.c @@ -192,4 +192,51 @@ void check_freopen_3() { rewind(f1); // expected-warning {{Stream might be invalid}} fclose(f1); } -} \ No newline at end of file +} + +extern FILE *GlobalF; +extern void takeFile(FILE *); + +void check_escape1() { + FILE *F = tmpfile(); + if (!F) + return; + fwrite("1", 1, 1, F); // may fail + GlobalF = F; + fwrite("1", 1, 1, F); // no warning +} + +void check_escape2() { + FILE *F = tmpfile(); + if (!F) + return; + fwrite("1", 1, 1, F); // may fail + takeFile(F); + fwrite("1", 1, 1, F); // no warning +} + +void check_escape3() { + FILE *F = tmpfile(); + if (!F) + return; + takeFile(F); + F = freopen(0, "w", F); + if (!F) + return; + fwrite("1", 1, 1, F); // may fail + fwrite("1", 1, 1, F); // no warning +} + +void check_escape4() { + FILE *F = tmpfile(); + if (!F) + return; + fwrite("1", 1, 1, F); // may fail + + // no escape at (non-StreamChecker-handled) system call + // FIXME: all such calls should be handled by the checker + fprintf(F, "0"); + + fwrite("1", 1, 1, F); // expected-warning {{might be 'indeterminate'}} + fclose(F); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
