llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Ben Shi (benshi001) <details> <summary>Changes</summary> --- Full diff: https://github.com/llvm/llvm-project/pull/71518.diff 3 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+66-8) - (modified) clang/test/Analysis/stream-error.c (+46) - (modified) clang/test/Analysis/stream.c (+14) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index 898906977ba9bb6..43ef9a2d377c512 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -245,11 +245,14 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, {{{"fclose"}, 1}, {&StreamChecker::preDefault, &StreamChecker::evalFclose, 0}}, {{{"fread"}, 4}, - {std::bind(&StreamChecker::preFreadFwrite, _1, _2, _3, _4, true), + {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true), std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, true), 3}}, {{{"fwrite"}, 4}, - {std::bind(&StreamChecker::preFreadFwrite, _1, _2, _3, _4, false), + {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, false), 3}}, + {{{"fputc"}, 2}, + {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), + &StreamChecker::evalFputc, 1}}, {{{"fseek"}, 3}, {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}}, {{{"ftell"}, 1}, @@ -305,12 +308,15 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, void evalFclose(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; - void preFreadFwrite(const FnDescription *Desc, const CallEvent &Call, - CheckerContext &C, bool IsFread) const; + void preReadWrite(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C, bool IsRead) const; void evalFreadFwrite(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C, bool IsFread) const; + void evalFputc(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const; + void preFseek(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; void evalFseek(const FnDescription *Desc, const CallEvent &Call, @@ -634,9 +640,9 @@ void StreamChecker::evalFclose(const FnDescription *Desc, const CallEvent &Call, C.addTransition(StateFailure); } -void StreamChecker::preFreadFwrite(const FnDescription *Desc, - const CallEvent &Call, CheckerContext &C, - bool IsFread) const { +void StreamChecker::preReadWrite(const FnDescription *Desc, + const CallEvent &Call, CheckerContext &C, + bool IsRead) const { ProgramStateRef State = C.getState(); SVal StreamVal = getStreamArg(Desc, Call); State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C, @@ -650,7 +656,7 @@ void StreamChecker::preFreadFwrite(const FnDescription *Desc, if (!State) return; - if (!IsFread) { + if (!IsRead) { C.addTransition(State); return; } @@ -745,6 +751,58 @@ void StreamChecker::evalFreadFwrite(const FnDescription *Desc, C.addTransition(StateFailed); } +void StreamChecker::evalFputc(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 *OldSS = State->get<StreamMap>(StreamSym); + if (!OldSS) + return; + + assertStreamStateOpened(OldSS); + + // `fputc` returns the written character on success, otherwise returns EOF. + + // Generate a transition for the success state. + std::optional<NonLoc> PutVal = Call.getArgSVal(0).getAs<NonLoc>(); + if (!PutVal) + return; + ProgramStateRef StateNotFailed = + State->BindExpr(CE, C.getLocationContext(), *PutVal); + StateNotFailed = + StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc)); + C.addTransition(StateNotFailed); + + // Add transition for the failed state. + SValBuilder &SVB = C.getSValBuilder(); + NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>(); + ProgramStateRef StateFailed = + State->BindExpr(CE, C.getLocationContext(), RetVal); + auto Cond = SVB.evalBinOp(State, BO_EQ, RetVal, + SVB.makeIntVal(*EofVal, C.getASTContext().IntTy), + SVB.getConditionType()) + .getAs<DefinedOrUnknownSVal>(); + if (!Cond) + return; + StateFailed = StateFailed->assume(*Cond, true); + if (!StateFailed) + return; + + // If a (non-EOF) error occurs, the resulting value of the file position + // indicator for the stream is indeterminate. + StreamState NewSS = StreamState::getOpened( + Desc, ErrorFError, /*IsFilePositionIndeterminate*/ true); + StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS); + C.addTransition(StateFailed); +} + void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c index 3c00e59bb6bd19d..7e895f3fe947158 100644 --- a/clang/test/Analysis/stream-error.c +++ b/clang/test/Analysis/stream-error.c @@ -241,6 +241,23 @@ void error_indeterminate_clearerr(void) { fclose(F); } +void error_indeterminate_fputc(void) { + FILE *F = fopen("file", "r+"); + if (!F) + return; + int rc = fseek(F, 0, SEEK_SET); + if (rc) { + if (feof(F)) { + fputc('X', F); // no warning + } else if (ferror(F)) { + fputc('C', F); // expected-warning {{might be 'indeterminate'}} + } else { + fputc('E', F); // expected-warning {{might be 'indeterminate'}} + } + } + fclose(F); +} + void error_indeterminate_feof1(void) { FILE *F = fopen("file", "r+"); if (!F) @@ -268,3 +285,32 @@ void error_indeterminate_feof2(void) { } fclose(F); } + +void error_indeterminate_feof3(void) { + FILE *F = fopen("file", "r+"); + if (!F) + return; + char Buf[10]; + if (fread(Buf, 1, 10, F) < 10) { + if (feof(F)) { + // error is feof, should be non-indeterminate + fputc(';', F); // no warning + } + if (ferror(F)) { + fputc('=', F); // expected-warning {{might be 'indeterminate'}} + } + } + fclose(F); +} + +void error_indeterminate_feof4(void) { + FILE *F = fopen("file", "r+"); + if (!F) + return; + if (fputc('Y', F) == EOF) { + fputc('W', F); // expected-warning {{might be 'indeterminate'}} + } else { + fputc('H', F); // no warning + } + fclose(F); +} diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c index a01310cfef5dd8a..a2eaaa2bd0142c5 100644 --- a/clang/test/Analysis/stream.c +++ b/clang/test/Analysis/stream.c @@ -14,6 +14,12 @@ void check_fwrite(void) { fclose(fp); } +void check_fgetc(void) { + FILE *fp = tmpfile(); + fputc('A', fp); // expected-warning {{Stream pointer might be NULL}} + fclose(fp); +} + void check_fseek(void) { FILE *fp = tmpfile(); fseek(fp, 0, 0); // expected-warning {{Stream pointer might be NULL}} @@ -111,6 +117,14 @@ void f_use_after_close(void) { clearerr(p); // expected-warning {{Stream might be already closed}} } +void f_write_after_close(void) { + FILE *p = fopen("foo", "r"); + if (!p) + return; + fclose(p); + fputc('A', p); // expected-warning {{Stream might be already closed}} +} + void f_open_after_close(void) { FILE *p = fopen("foo", "r"); if (!p) `````````` </details> https://github.com/llvm/llvm-project/pull/71518 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits