https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/109112
>From 408ee82b1ee3ae15302b2a0dde9faded3e43bf0f Mon Sep 17 00:00:00 2001 From: Arseniy Zaostrovnykh <necto...@gmail.com> Date: Wed, 18 Sep 2024 11:30:10 +0200 Subject: [PATCH 1/3] [analyzer] Note last "fclose" call from "ensureStreamOpened" Patch by Arseniy Zaostrovnykh! --- .../StaticAnalyzer/Checkers/StreamChecker.cpp | 49 +++++++++++++++++-- clang/test/Analysis/stream-note.c | 9 ++++ 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index 8bb7880a3cc283..c0f3591662b418 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -1835,6 +1835,46 @@ StreamChecker::ensureStreamNonNull(SVal StreamVal, const Expr *StreamE, return StateNotNull; } +namespace { +class StreamClosedVisitor final : public BugReporterVisitor { + const SymbolRef StreamSym; + bool Satisfied = false; + +public: + explicit StreamClosedVisitor(SymbolRef StreamSym) : StreamSym(StreamSym) {} + + static void *getTag() { + static int Tag = 0; + return &Tag; + } + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.AddPointer(getTag()); + ID.AddPointer(StreamSym); + } + + PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override { + if (Satisfied) + return nullptr; + const StreamState *PredSS = + N->getFirstPred()->getState()->get<StreamMap>(StreamSym); + if (PredSS && PredSS->isClosed()) + return nullptr; + + const Stmt *S = N->getStmtForDiagnostics(); + if (!S) + return nullptr; + Satisfied = true; + PathDiagnosticLocation Pos(S, BRC.getSourceManager(), + N->getLocationContext()); + llvm::StringLiteral Msg = "Stream is closed here"; + return std::make_shared<PathDiagnosticEventPiece>(Pos, Msg); + } +}; +} // namespace + ProgramStateRef StreamChecker::ensureStreamOpened(SVal StreamVal, CheckerContext &C, ProgramStateRef State) const { @@ -1849,11 +1889,12 @@ ProgramStateRef StreamChecker::ensureStreamOpened(SVal StreamVal, if (SS->isClosed()) { // Using a stream pointer after 'fclose' causes undefined behavior // according to cppreference.com . - ExplodedNode *N = C.generateErrorNode(); - if (N) { - C.emitReport(std::make_unique<PathSensitiveBugReport>( + if (ExplodedNode *N = C.generateErrorNode()) { + auto R = std::make_unique<PathSensitiveBugReport>( BT_UseAfterClose, - "Stream might be already closed. Causes undefined behaviour.", N)); + "Stream might be already closed. Causes undefined behaviour.", N); + R->addVisitor<StreamClosedVisitor>(Sym); + C.emitReport(std::move(R)); return nullptr; } diff --git a/clang/test/Analysis/stream-note.c b/clang/test/Analysis/stream-note.c index 3aef707d50056e..02251e2e12d6a1 100644 --- a/clang/test/Analysis/stream-note.c +++ b/clang/test/Analysis/stream-note.c @@ -264,3 +264,12 @@ void error_fseek_read_eof(void) { fgetc(F); // no warning fclose(F); } + +void check_note_at_use_after_close(void) { + FILE *F = tmpfile(); + if (!F) // expected-note {{'F' is non-null}} expected-note {{Taking false branch}} + return; + fclose(F); // expected-note {{Stream is closed here}} + rewind(F); // expected-warning {{Stream might be already closed. Causes undefined behaviour}} + // expected-note@-1 {{Stream might be already closed. Causes undefined behaviour}} +} >From b5e904ed860c27b03f986c89ffbf373a5910abb1 Mon Sep 17 00:00:00 2001 From: Balazs Benics <benicsbal...@gmail.com> Date: Wed, 18 Sep 2024 11:59:34 +0200 Subject: [PATCH 2/3] Accept reviewer suggestion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Donát Nagy <donat.n...@ericsson.com> --- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index c0f3591662b418..a476dbc4a96d8d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -1892,7 +1892,7 @@ ProgramStateRef StreamChecker::ensureStreamOpened(SVal StreamVal, if (ExplodedNode *N = C.generateErrorNode()) { auto R = std::make_unique<PathSensitiveBugReport>( BT_UseAfterClose, - "Stream might be already closed. Causes undefined behaviour.", N); + "Use of a stream that might be already closed", N); R->addVisitor<StreamClosedVisitor>(Sym); C.emitReport(std::move(R)); return nullptr; >From d4467207f297d54e3e62f4d62f2b4cd6186b7e59 Mon Sep 17 00:00:00 2001 From: Balazs Benics <benicsbal...@gmail.com> Date: Wed, 18 Sep 2024 12:04:39 +0200 Subject: [PATCH 3/3] Uplift tests after diag message change --- clang/test/Analysis/stream-error.c | 22 +++++++++++----------- clang/test/Analysis/stream-note.c | 4 ++-- clang/test/Analysis/stream.c | 10 +++++----- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c index 3f791d13346419..9de56c082e8258 100644 --- a/clang/test/Analysis/stream-error.c +++ b/clang/test/Analysis/stream-error.c @@ -96,7 +96,7 @@ void error_fread(void) { } } fclose(F); - Ret = fread(Buf, 1, 10, F); // expected-warning {{Stream might be already closed}} + Ret = fread(Buf, 1, 10, F); // expected-warning {{Use of a stream that might be already closed}} } void error_fwrite(void) { @@ -113,7 +113,7 @@ void error_fwrite(void) { fwrite(0, 1, 10, F); // expected-warning {{might be 'indeterminate'}} } fclose(F); - Ret = fwrite(0, 1, 10, F); // expected-warning {{Stream might be already closed}} + Ret = fwrite(0, 1, 10, F); // expected-warning {{Use of a stream that might be already closed}} } void error_fgetc(void) { @@ -135,7 +135,7 @@ void error_fgetc(void) { } } fclose(F); - fgetc(F); // expected-warning {{Stream might be already closed}} + fgetc(F); // expected-warning {{Use of a stream that might be already closed}} } void error_fgets(void) { @@ -158,7 +158,7 @@ void error_fgets(void) { } } fclose(F); - fgets(Buf, sizeof(Buf), F); // expected-warning {{Stream might be already closed}} + fgets(Buf, sizeof(Buf), F); // expected-warning {{Use of a stream that might be already closed}} } void error_fputc(int fd) { @@ -176,7 +176,7 @@ void error_fputc(int fd) { fputc('Y', F); // no-warning } fclose(F); - fputc('A', F); // expected-warning {{Stream might be already closed}} + fputc('A', F); // expected-warning {{Use of a stream that might be already closed}} } void error_fputs(void) { @@ -194,7 +194,7 @@ void error_fputs(void) { fputs("QWD", F); // expected-warning {{might be 'indeterminate'}} } fclose(F); - fputs("ABC", F); // expected-warning {{Stream might be already closed}} + fputs("ABC", F); // expected-warning {{Use of a stream that might be already closed}} } void error_fprintf(void) { @@ -211,7 +211,7 @@ void error_fprintf(void) { fprintf(F, "bbb"); // expected-warning {{might be 'indeterminate'}} } fclose(F); - fprintf(F, "ccc"); // expected-warning {{Stream might be already closed}} + fprintf(F, "ccc"); // expected-warning {{Use of a stream that might be already closed}} } void error_fscanf(int *A) { @@ -236,7 +236,7 @@ void error_fscanf(int *A) { } } fclose(F); - fscanf(F, "ccc"); // expected-warning {{Stream might be already closed}} + fscanf(F, "ccc"); // expected-warning {{Use of a stream that might be already closed}} } void error_ungetc(int TestIndeterminate) { @@ -256,7 +256,7 @@ void error_ungetc(int TestIndeterminate) { 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 {{Use of a stream that might be already closed}} } void error_getdelim(char *P, size_t Sz) { @@ -278,7 +278,7 @@ void error_getdelim(char *P, size_t Sz) { } } fclose(F); - getdelim(&P, &Sz, '\n', F); // expected-warning {{Stream might be already closed}} + getdelim(&P, &Sz, '\n', F); // expected-warning {{Use of a stream that might be already closed}} } void error_getline(char *P, size_t Sz) { @@ -300,7 +300,7 @@ void error_getline(char *P, size_t Sz) { } } fclose(F); - getline(&P, &Sz, F); // expected-warning {{Stream might be already closed}} + getline(&P, &Sz, F); // expected-warning {{Use of a stream that might be already closed}} } void write_after_eof_is_allowed(void) { diff --git a/clang/test/Analysis/stream-note.c b/clang/test/Analysis/stream-note.c index 02251e2e12d6a1..2b5d1edb2814f0 100644 --- a/clang/test/Analysis/stream-note.c +++ b/clang/test/Analysis/stream-note.c @@ -270,6 +270,6 @@ void check_note_at_use_after_close(void) { if (!F) // expected-note {{'F' is non-null}} expected-note {{Taking false branch}} return; fclose(F); // expected-note {{Stream is closed here}} - rewind(F); // expected-warning {{Stream might be already closed. Causes undefined behaviour}} - // expected-note@-1 {{Stream might be already closed. Causes undefined behaviour}} + rewind(F); // expected-warning {{Use of a stream that might be already closed}} + // expected-note@-1 {{Use of a stream that might be already closed}} } diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c index b9a5b1ba8cd494..758b40cca49319 100644 --- a/clang/test/Analysis/stream.c +++ b/clang/test/Analysis/stream.c @@ -185,7 +185,7 @@ void f_double_close(void) { if (!p) return; fclose(p); - fclose(p); // expected-warning {{Stream might be already closed}} + fclose(p); // expected-warning {{Use of a stream that might be already closed}} } void f_double_close_alias(void) { @@ -194,7 +194,7 @@ void f_double_close_alias(void) { return; FILE *p2 = p1; fclose(p1); - fclose(p2); // expected-warning {{Stream might be already closed}} + fclose(p2); // expected-warning {{Use of a stream that might be already closed}} } void f_use_after_close(void) { @@ -202,7 +202,7 @@ void f_use_after_close(void) { if (!p) return; fclose(p); - clearerr(p); // expected-warning {{Stream might be already closed}} + clearerr(p); // expected-warning {{Use of a stream that might be already closed}} } void f_open_after_close(void) { @@ -266,7 +266,7 @@ void check_freopen_2(void) { if (f2) { // Check if f1 and f2 point to the same stream. fclose(f1); - fclose(f2); // expected-warning {{Stream might be already closed.}} + fclose(f2); // expected-warning {{Use of a stream that might be already closed}} } else { // Reopen failed. // f1 is non-NULL but points to a possibly invalid stream. @@ -370,7 +370,7 @@ void fflush_after_fclose(void) { if ((Ret = fflush(F)) != 0) clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}} fclose(F); - fflush(F); // expected-warning {{Stream might be already closed}} + fflush(F); // expected-warning {{Use of a stream that might be already closed}} } void fflush_on_open_failed_stream(void) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits