https://github.com/benshi001 updated https://github.com/llvm/llvm-project/pull/73335
>From a335821f73529a0460a848fb12820fb60dc91c09 Mon Sep 17 00:00:00 2001 From: Ben Shi <benn...@tencent.com> Date: Fri, 24 Nov 2023 22:51:27 +0800 Subject: [PATCH] [clang][analyzer] Support `fputs` in the StreamChecker --- .../StaticAnalyzer/Checkers/StreamChecker.cpp | 105 +++++++++++++----- .../Analysis/Inputs/system-header-simulator.h | 1 + clang/test/Analysis/stream-error.c | 30 ++++- clang/test/Analysis/stream.c | 6 + 4 files changed, 109 insertions(+), 33 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index 8eca989d7bcdea4..eccb2063fad10d0 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -252,10 +252,13 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, 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}}, + std::bind(&StreamChecker::evalFgetc, _1, _2, _3, _4), 0}}, {{{"fputc"}, 2}, {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), - std::bind(&StreamChecker::evalFgetcFputc, _1, _2, _3, _4, false), 1}}, + std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, true), 1}}, + {{{"fputs"}, 2}, + {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), + std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, false), 1}}, {{{"fseek"}, 3}, {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}}, {{{"ftell"}, 1}, @@ -317,8 +320,11 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, void evalFreadFwrite(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C, bool IsFread) const; - void evalFgetcFputc(const FnDescription *Desc, const CallEvent &Call, - CheckerContext &C, bool IsRead) const; + void evalFgetc(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const; + + void evalFputx(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C, bool IsSingleChar) const; void preFseek(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; @@ -754,9 +760,8 @@ void StreamChecker::evalFreadFwrite(const FnDescription *Desc, C.addTransition(StateFailed); } -void StreamChecker::evalFgetcFputc(const FnDescription *Desc, - const CallEvent &Call, CheckerContext &C, - bool IsRead) const { +void StreamChecker::evalFgetc(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const { ProgramStateRef State = C.getState(); SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol(); if (!StreamSym) @@ -773,27 +778,15 @@ void StreamChecker::evalFgetcFputc(const FnDescription *Desc, 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 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. + // 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) { + if (OldSS->ErrorState != ErrorFEof) { NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>(); ProgramStateRef StateNotFailed = State->BindExpr(CE, C.getLocationContext(), RetVal); SValBuilder &SVB = C.getSValBuilder(); - auto &ASTC = C.getASTContext(); + ASTContext &ASTC = C.getASTContext(); // The returned 'unsigned char' of `fgetc` is converted to 'int', // so we need to check if it is in range [0, 255]. auto CondLow = @@ -824,20 +817,74 @@ void StreamChecker::evalFgetcFputc(const FnDescription *Desc, // If a (non-EOF) error occurs, the resulting value of the file position // indicator for the stream is indeterminate. - StreamErrorState NewES; - if (IsRead) - NewES = - OldSS->ErrorState == ErrorFEof ? ErrorFEof : ErrorFEof | ErrorFError; - else - NewES = ErrorFError; + StreamErrorState NewES = + OldSS->ErrorState == ErrorFEof ? ErrorFEof : ErrorFEof | ErrorFError; StreamState NewSS = StreamState::getOpened(Desc, NewES, !NewES.isFEof()); StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS); - if (IsRead && OldSS->ErrorState != ErrorFEof) + if (OldSS->ErrorState != ErrorFEof) C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym)); else C.addTransition(StateFailed); } +void StreamChecker::evalFputx(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C, bool IsSingleChar) 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. + // `fputs` returns a non negative value on sucecess, otherwise returns EOF. + + if (IsSingleChar) { + // Generate a transition for the success state of `fputc`. + 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); + } else { + // Generate a transition for the success state of `fputs`. + NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>(); + ProgramStateRef StateNotFailed = + State->BindExpr(CE, C.getLocationContext(), RetVal); + SValBuilder &SVB = C.getSValBuilder(); + auto &ASTC = C.getASTContext(); + auto Cond = SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(ASTC.IntTy), + SVB.getConditionType()) + .getAs<DefinedOrUnknownSVal>(); + if (!Cond) + return; + StateNotFailed = StateNotFailed->assume(*Cond, true); + if (!StateNotFailed) + return; + StateNotFailed = + StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc)); + C.addTransition(StateNotFailed); + } + + // Add transition for the failed state. 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, 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/Inputs/system-header-simulator.h b/clang/test/Analysis/Inputs/system-header-simulator.h index fc57e8bdc3d30c3..7ef5f29fbf42cb1 100644 --- a/clang/test/Analysis/Inputs/system-header-simulator.h +++ b/clang/test/Analysis/Inputs/system-header-simulator.h @@ -50,6 +50,7 @@ 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 fputs(const char *restrict s, FILE *restrict stream); int fseek(FILE *__stream, long int __off, int __whence); long int ftell(FILE *__stream); void rewind(FILE *__stream); diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c index 38e6b77b9bb5053..8ad29f48ed49514 100644 --- a/clang/test/Analysis/stream-error.c +++ b/clang/test/Analysis/stream-error.c @@ -131,16 +131,38 @@ void error_fputc(void) { if (Ret == EOF) { clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}} clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}} - fputc('Y', F); // expected-warning {{might be 'indeterminate'}} + fputc('Y', F); // expected-warning {{might be 'indeterminate'}} } else { - clang_analyzer_eval(Ret == 'X'); // expected-warning {{TRUE}} - clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}} - fputc('Y', F); // no-warning + clang_analyzer_eval(Ret == 'X'); // expected-warning {{TRUE}} + clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}} + StreamTesterChecker_make_feof_stream(F); + if (fputc('Y', F) == 'Y') // no-warning + clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}} } fclose(F); fputc('A', F); // expected-warning {{Stream might be already closed}} } +void error_fputs(void) { + FILE *F = tmpfile(); + if (!F) + return; + int Ret = fputs("XYZ", F); + if (Ret >= 0) { + clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}} + StreamTesterChecker_make_feof_stream(F); + if (fputs("QWD", F) >= 0) // no-warning + clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}} + } else { + clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}} + clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}} + clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}} + fputs("QWD", F); // expected-warning {{might be 'indeterminate'}} + } + fclose(F); + fputs("ABC", F); // expected-warning {{Stream might be already closed}} +} + void freadwrite_zerosize(FILE *F) { size_t Ret; Ret = fwrite(0, 1, 0, F); diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c index a7ee9982478bb96..b0f39af8eb2f3a3 100644 --- a/clang/test/Analysis/stream.c +++ b/clang/test/Analysis/stream.c @@ -26,6 +26,12 @@ void check_fputc(void) { fclose(fp); } +void check_fputs(void) { + FILE *fp = tmpfile(); + fputs("ABC", 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}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits