https://github.com/benshi001 updated https://github.com/llvm/llvm-project/pull/72627
>From d3ce61fecbe922f09ad1228a3305f484e8e1dcfc Mon Sep 17 00:00:00 2001 From: Ben Shi <benn...@tencent.com> Date: Fri, 17 Nov 2023 17:22:10 +0800 Subject: [PATCH] [clang][analyzer] Support `fgetc` in StreamChecker --- .../StaticAnalyzer/Checkers/StreamChecker.cpp | 70 +++++++++++++----- .../Analysis/Inputs/system-header-simulator.h | 1 + clang/test/Analysis/stream-error.c | 71 ++++++++++++++++++- clang/test/Analysis/stream.c | 6 ++ 4 files changed, 129 insertions(+), 19 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index 1d53e59ca067c27..3acf23f090cb397 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -250,9 +250,12 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, {{{"fwrite"}, 4}, {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, false), 3}}, + {{{"fgetc"}, 1}, + {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true), + std::bind(&StreamChecker::evalFgetcFputc, _1, _2, _3, _4, true), 0}}, {{{"fputc"}, 2}, {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), - &StreamChecker::evalFputc, 1}}, + std::bind(&StreamChecker::evalFgetcFputc, _1, _2, _3, _4, false), 1}}, {{{"fseek"}, 3}, {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}}, {{{"ftell"}, 1}, @@ -314,8 +317,8 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, 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 evalFgetcFputc(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C, bool IsRead) const; void preFseek(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; @@ -751,8 +754,9 @@ void StreamChecker::evalFreadFwrite(const FnDescription *Desc, C.addTransition(StateFailed); } -void StreamChecker::evalFputc(const FnDescription *Desc, const CallEvent &Call, - CheckerContext &C) const { +void StreamChecker::evalFgetcFputc(const FnDescription *Desc, + const CallEvent &Call, CheckerContext &C, + bool IsRead) const { ProgramStateRef State = C.getState(); SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol(); if (!StreamSym) @@ -768,26 +772,56 @@ void StreamChecker::evalFputc(const FnDescription *Desc, const CallEvent &Call, assertStreamStateOpened(OldSS); + // `fgetc` returns the read character on success, otherwise returns EOF. // `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); + // Generate a transition for the success state of fputc. + if (!IsRead) { + 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); + } + // Generate a transition for the success state of fgetc. + // If we know the state to be FEOF at fgetc, do not add a success state. + else if (OldSS->ErrorState != ErrorFEof) { + NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>(); + ProgramStateRef StateNotFailed = + State->BindExpr(CE, C.getLocationContext(), RetVal); + SValBuilder &SVB = C.getSValBuilder(); + auto Cond = SVB.evalBinOp(State, BO_GE, RetVal, + SVB.makeZeroVal(C.getASTContext().IntTy), + SVB.getConditionType()) + .getAs<DefinedOrUnknownSVal>(); + if (!Cond) + return; + StateNotFailed = StateNotFailed->assume(*Cond, true); + if (!StateNotFailed) + return; + C.addTransition(StateNotFailed); + } // Add transition for the failed state. + ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE); + // If a (non-EOF) error occurs, the resulting value of the file position // indicator for the stream is indeterminate. - ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE); - StreamState NewSS = StreamState::getOpened( - Desc, ErrorFError, /*IsFilePositionIndeterminate*/ true); + StreamErrorState NewES; + if (IsRead) + NewES = + OldSS->ErrorState == ErrorFEof ? ErrorFEof : ErrorFEof | ErrorFError; + else + NewES = ErrorFError; + StreamState NewSS = StreamState::getOpened(Desc, NewES, !NewES.isFEof()); StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS); - C.addTransition(StateFailed); + if (IsRead && OldSS->ErrorState != ErrorFEof) + C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym)); + else + C.addTransition(StateFailed); } void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call, diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h b/clang/test/Analysis/Inputs/system-header-simulator.h index 8924103f5046ea2..fc57e8bdc3d30c3 100644 --- a/clang/test/Analysis/Inputs/system-header-simulator.h +++ b/clang/test/Analysis/Inputs/system-header-simulator.h @@ -48,6 +48,7 @@ FILE *freopen(const char *restrict pathname, const char *restrict mode, FILE *re int fclose(FILE *fp); size_t fread(void *restrict, size_t, size_t, FILE *restrict); size_t fwrite(const void *restrict, size_t, size_t, FILE *restrict); +int fgetc(FILE *stream); int fputc(int ch, FILE *stream); int fseek(FILE *__stream, long int __off, int __whence); long int ftell(FILE *__stream); diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c index 5ebdc32bb1b92ff..c86e2bb71172692 100644 --- a/clang/test/Analysis/stream-error.c +++ b/clang/test/Analysis/stream-error.c @@ -101,6 +101,30 @@ void error_fwrite(void) { Ret = fwrite(0, 1, 10, F); // expected-warning {{Stream might be already closed}} } +void error_fgetc(void) { + FILE *F = tmpfile(); + if (!F) + return; + int Ret = fgetc(F); + if (Ret >= 0) { + clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}} + } else { + clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}} + if (feof(F)) { + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + fgetc(F); // expected-warning {{Read function called when stream is in EOF state}} + clang_analyzer_eval(feof(F)); // expected-warning {{TRUE}} + clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}} + } + if (ferror(F)) { + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + fgetc(F); // expected-warning {{might be 'indeterminate'}} + } + } + fclose(F); + fgetc(F); // expected-warning {{Stream might be already closed}} +} + void error_fputc(void) { FILE *F = tmpfile(); if (!F) @@ -259,6 +283,24 @@ void error_indeterminate_clearerr(void) { fclose(F); } +void error_indeterminate_fgetc(void) { + FILE *F = fopen("file", "r+"); + if (!F) + return; + int rc = fseek(F, 0, SEEK_SET); + if (rc) { + if (feof(F)) { + clang_analyzer_warnIfReached(); // no warning + fgetc(F); // no warning + } else if (ferror(F)) { + fgetc(F); // expected-warning {{might be 'indeterminate'}} + } else { + fgetc(F); // expected-warning {{might be 'indeterminate'}} + } + } + fclose(F); +} + void error_indeterminate_fputc(void) { FILE *F = fopen("file", "r+"); if (!F) @@ -266,7 +308,8 @@ void error_indeterminate_fputc(void) { int rc = fseek(F, 0, SEEK_SET); if (rc) { if (feof(F)) { - fputc('X', F); // no warning + clang_analyzer_warnIfReached(); // no warning + fputc('X', F); // no warning } else if (ferror(F)) { fputc('C', F); // expected-warning {{might be 'indeterminate'}} } else { @@ -303,3 +346,29 @@ void error_indeterminate_feof2(void) { } fclose(F); } + +void error_indeterminate_feof3(void) { + FILE *F = fopen("file", "r+"); + if (!F) + return; + if (fgetc(F) == EOF) { + if (feof(F)) { + // error is feof, should be non-indeterminate + fputc('A', F); // no warning + } + } + fclose(F); +} + +void error_indeterminate_feof4(void) { + FILE *F = fopen("file", "r+"); + if (!F) + return; + if (fgetc(F) == EOF) { + if (ferror(F) == 0) { + // error is feof, should be non-indeterminate + fputc('B', F); // no warning + } + } + fclose(F); +} diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c index 9b6a31738d6e0b8..a7ee9982478bb96 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(); + fgetc(fp); // expected-warning {{Stream pointer might be NULL}} + fclose(fp); +} + void check_fputc(void) { FILE *fp = tmpfile(); fputc('A', fp); // expected-warning {{Stream pointer might be NULL}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits