llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Balázs Kéri (balazske) <details> <summary>Changes</summary> These functions should not be allowed if the file position is indeterminate (they return the file position). This condition is now checked, and tests are improved to check it. --- Full diff: https://github.com/llvm/llvm-project/pull/84191.diff 2 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+53-30) - (modified) clang/test/Analysis/stream-error.c (+23-4) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index 2ec47bf55df76b..346d4ae8db2dfe 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -307,64 +307,64 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, {{{"fclose"}, 1}, {&StreamChecker::preDefault, &StreamChecker::evalFclose, 0}}, {{{"fread"}, 4}, - {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true), + {&StreamChecker::preRead, std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, true), 3}}, {{{"fwrite"}, 4}, - {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), + {&StreamChecker::preWrite, std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, false), 3}}, {{{"fgetc"}, 1}, - {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true), + {&StreamChecker::preRead, std::bind(&StreamChecker::evalFgetx, _1, _2, _3, _4, true), 0}}, {{{"fgets"}, 3}, - {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true), + {&StreamChecker::preRead, std::bind(&StreamChecker::evalFgetx, _1, _2, _3, _4, false), 2}}, {{{"getc"}, 1}, {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true), std::bind(&StreamChecker::evalFgetx, _1, _2, _3, _4, true), 0}}, {{{"fputc"}, 2}, - {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), + {&StreamChecker::preWrite, std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, true), 1}}, {{{"fputs"}, 2}, - {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), + {&StreamChecker::preWrite, std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, false), 1}}, {{{"putc"}, 2}, {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, true), 1}}, {{{"fprintf"}}, - {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), + {&StreamChecker::preWrite, std::bind(&StreamChecker::evalFprintf, _1, _2, _3, _4), 0}}, {{{"vfprintf"}, 3}, {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), std::bind(&StreamChecker::evalFprintf, _1, _2, _3, _4), 0}}, {{{"fscanf"}}, - {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true), + {&StreamChecker::preRead, std::bind(&StreamChecker::evalFscanf, _1, _2, _3, _4), 0}}, {{{"vfscanf"}, 3}, {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true), std::bind(&StreamChecker::evalFscanf, _1, _2, _3, _4), 0}}, {{{"ungetc"}, 2}, - {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), + {&StreamChecker::preWrite, std::bind(&StreamChecker::evalUngetc, _1, _2, _3, _4), 1}}, {{{"getdelim"}, 4}, - {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true), + {&StreamChecker::preRead, std::bind(&StreamChecker::evalGetdelim, _1, _2, _3, _4), 3}}, {{{"getline"}, 3}, - {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true), + {&StreamChecker::preRead, std::bind(&StreamChecker::evalGetdelim, _1, _2, _3, _4), 2}}, {{{"fseek"}, 3}, {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}}, {{{"fseeko"}, 3}, {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}}, {{{"ftell"}, 1}, - {&StreamChecker::preDefault, &StreamChecker::evalFtell, 0}}, + {&StreamChecker::preWrite, &StreamChecker::evalFtell, 0}}, {{{"ftello"}, 1}, - {&StreamChecker::preDefault, &StreamChecker::evalFtell, 0}}, + {&StreamChecker::preWrite, &StreamChecker::evalFtell, 0}}, {{{"fflush"}, 1}, {&StreamChecker::preFflush, &StreamChecker::evalFflush, 0}}, {{{"rewind"}, 1}, {&StreamChecker::preDefault, &StreamChecker::evalRewind, 0}}, {{{"fgetpos"}, 2}, - {&StreamChecker::preDefault, &StreamChecker::evalFgetpos, 0}}, + {&StreamChecker::preWrite, &StreamChecker::evalFgetpos, 0}}, {{{"fsetpos"}, 2}, {&StreamChecker::preDefault, &StreamChecker::evalFsetpos, 0}}, {{{"clearerr"}, 1}, @@ -384,12 +384,18 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, CallDescriptionMap<FnDescription> FnTestDescriptions = { {{{"StreamTesterChecker_make_feof_stream"}, 1}, {nullptr, - std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4, ErrorFEof), + std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4, ErrorFEof, + false), 0}}, {{{"StreamTesterChecker_make_ferror_stream"}, 1}, {nullptr, std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4, - ErrorFError), + ErrorFError, false), + 0}}, + {{{"StreamTesterChecker_make_ferror_indeterminate_stream"}, 1}, + {nullptr, + std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4, + ErrorFError, true), 0}}, }; @@ -415,8 +421,11 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, void evalFclose(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; - void preReadWrite(const FnDescription *Desc, const CallEvent &Call, - CheckerContext &C, bool IsRead) const; + void preRead(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const; + + void preWrite(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const; void evalFreadFwrite(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C, bool IsFread) const; @@ -467,8 +476,8 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, const StreamErrorState &ErrorKind) const; void evalSetFeofFerror(const FnDescription *Desc, const CallEvent &Call, - CheckerContext &C, - const StreamErrorState &ErrorKind) const; + CheckerContext &C, const StreamErrorState &ErrorKind, + bool Indeterminate) const; void preFflush(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; @@ -849,9 +858,8 @@ void StreamChecker::evalFclose(const FnDescription *Desc, const CallEvent &Call, C.addTransition(E.bindReturnValue(State, C, *EofVal)); } -void StreamChecker::preReadWrite(const FnDescription *Desc, - const CallEvent &Call, CheckerContext &C, - bool IsRead) const { +void StreamChecker::preRead(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const { ProgramStateRef State = C.getState(); SVal StreamVal = getStreamArg(Desc, Call); State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C, @@ -865,11 +873,6 @@ void StreamChecker::preReadWrite(const FnDescription *Desc, if (!State) return; - if (!IsRead) { - C.addTransition(State); - return; - } - SymbolRef Sym = StreamVal.getAsSymbol(); if (Sym && State->get<StreamMap>(Sym)) { const StreamState *SS = State->get<StreamMap>(Sym); @@ -880,6 +883,24 @@ void StreamChecker::preReadWrite(const FnDescription *Desc, } } +void StreamChecker::preWrite(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + SVal StreamVal = getStreamArg(Desc, Call); + State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C, + State); + if (!State) + return; + State = ensureStreamOpened(StreamVal, C, State); + if (!State) + return; + State = ensureNoFilePositionIndeterminate(StreamVal, C, State); + if (!State) + return; + + C.addTransition(State); +} + void StreamChecker::evalFreadFwrite(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C, bool IsFread) const { @@ -1496,14 +1517,16 @@ void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call, void StreamChecker::evalSetFeofFerror(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C, - const StreamErrorState &ErrorKind) const { + const StreamErrorState &ErrorKind, + bool Indeterminate) const { ProgramStateRef State = C.getState(); SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol(); assert(StreamSym && "Operation not permitted on non-symbolic stream value."); const StreamState *SS = State->get<StreamMap>(StreamSym); assert(SS && "Stream should be tracked by the checker."); State = State->set<StreamMap>( - StreamSym, StreamState::getOpened(SS->LastOperation, ErrorKind)); + StreamSym, + StreamState::getOpened(SS->LastOperation, ErrorKind, Indeterminate)); C.addTransition(State); } diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c index ac31083bfc691f..88f7de4234ffb4 100644 --- a/clang/test/Analysis/stream-error.c +++ b/clang/test/Analysis/stream-error.c @@ -11,6 +11,7 @@ void clang_analyzer_dump(int); void clang_analyzer_warnIfReached(void); void StreamTesterChecker_make_feof_stream(FILE *); void StreamTesterChecker_make_ferror_stream(FILE *); +void StreamTesterChecker_make_ferror_indeterminate_stream(FILE *); void error_fopen(void) { FILE *F = fopen("file", "r"); @@ -52,6 +53,8 @@ void stream_error_feof(void) { clearerr(F); clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}} clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}} + StreamTesterChecker_make_ferror_indeterminate_stream(F); + clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}} fclose(F); } @@ -65,6 +68,8 @@ void stream_error_ferror(void) { clearerr(F); clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}} clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}} + StreamTesterChecker_make_ferror_indeterminate_stream(F); + clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}} fclose(F); } @@ -233,7 +238,7 @@ void error_fscanf(int *A) { fscanf(F, "ccc"); // expected-warning {{Stream might be already closed}} } -void error_ungetc() { +void error_ungetc(int TestIndeterminate) { FILE *F = tmpfile(); if (!F) return; @@ -245,8 +250,12 @@ void error_ungetc() { clang_analyzer_eval(Ret == 'X'); // expected-warning {{TRUE}} } fputc('Y', F); // no-warning + if (TestIndeterminate) { + StreamTesterChecker_make_ferror_indeterminate_stream(F); + ungetc('X', F); // expected-warning {{might be 'indeterminate'}} + } fclose(F); - ungetc('A', F); // expected-warning {{Stream might be already closed}} + ungetc('A', F); // expected-warning {{Stream might be already closed}} } void error_getdelim(char *P, size_t Sz) { @@ -449,7 +458,7 @@ void error_fseeko_0(void) { fclose(F); } -void error_ftell(void) { +void error_ftell(int TestIndeterminate) { FILE *F = fopen("file", "r"); if (!F) return; @@ -467,10 +476,14 @@ void error_ftell(void) { rc = ftell(F); clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}} clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}} + if (TestIndeterminate) { + StreamTesterChecker_make_ferror_indeterminate_stream(F); + ftell(F); // expected-warning {{might be 'indeterminate'}} + } fclose(F); } -void error_ftello(void) { +void error_ftello(int TestIndeterminate) { FILE *F = fopen("file", "r"); if (!F) return; @@ -488,6 +501,10 @@ void error_ftello(void) { rc = ftello(F); clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}} clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}} + if (TestIndeterminate) { + StreamTesterChecker_make_ferror_indeterminate_stream(F); + ftell(F); // expected-warning {{might be 'indeterminate'}} + } fclose(F); } @@ -506,6 +523,8 @@ void error_fileno(void) { N = fileno(F); clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}} clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}} + StreamTesterChecker_make_ferror_indeterminate_stream(F); + fileno(F); // no warning fclose(F); } `````````` </details> https://github.com/llvm/llvm-project/pull/84191 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
