[clang] [analyzer] Note last "fclose" call from "ensureStreamOpened" (PR #109112)
Balazs Benics via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 18 03:04:58 PDT 2024
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.ne at 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 at -1 {{Stream might be already closed. Causes undefined behaviour}}
+}
>From b5e904ed860c27b03f986c89ffbf373a5910abb1 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at 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.nagy at 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 <benicsbalazs at 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 at -1 {{Stream might be already closed. Causes undefined behaviour}}
+ rewind(F); // expected-warning {{Use of a stream that might be already closed}}
+ // expected-note at -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) {
More information about the cfe-commits
mailing list