Skip to content

Commit 6d0c974

Browse files
steakhaltmsri
authored andcommitted
[analyzer] Note last "fclose" call from "ensureStreamOpened" (llvm#109112)
Patch by Arseniy Zaostrovnykh!
1 parent 1b1d649 commit 6d0c974

File tree

4 files changed

+70
-21
lines changed

4 files changed

+70
-21
lines changed

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1835,6 +1835,46 @@ StreamChecker::ensureStreamNonNull(SVal StreamVal, const Expr *StreamE,
18351835
return StateNotNull;
18361836
}
18371837

1838+
namespace {
1839+
class StreamClosedVisitor final : public BugReporterVisitor {
1840+
const SymbolRef StreamSym;
1841+
bool Satisfied = false;
1842+
1843+
public:
1844+
explicit StreamClosedVisitor(SymbolRef StreamSym) : StreamSym(StreamSym) {}
1845+
1846+
static void *getTag() {
1847+
static int Tag = 0;
1848+
return &Tag;
1849+
}
1850+
1851+
void Profile(llvm::FoldingSetNodeID &ID) const override {
1852+
ID.AddPointer(getTag());
1853+
ID.AddPointer(StreamSym);
1854+
}
1855+
1856+
PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
1857+
BugReporterContext &BRC,
1858+
PathSensitiveBugReport &BR) override {
1859+
if (Satisfied)
1860+
return nullptr;
1861+
const StreamState *PredSS =
1862+
N->getFirstPred()->getState()->get<StreamMap>(StreamSym);
1863+
if (PredSS && PredSS->isClosed())
1864+
return nullptr;
1865+
1866+
const Stmt *S = N->getStmtForDiagnostics();
1867+
if (!S)
1868+
return nullptr;
1869+
Satisfied = true;
1870+
PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
1871+
N->getLocationContext());
1872+
llvm::StringLiteral Msg = "Stream is closed here";
1873+
return std::make_shared<PathDiagnosticEventPiece>(Pos, Msg);
1874+
}
1875+
};
1876+
} // namespace
1877+
18381878
ProgramStateRef StreamChecker::ensureStreamOpened(SVal StreamVal,
18391879
CheckerContext &C,
18401880
ProgramStateRef State) const {
@@ -1849,11 +1889,11 @@ ProgramStateRef StreamChecker::ensureStreamOpened(SVal StreamVal,
18491889
if (SS->isClosed()) {
18501890
// Using a stream pointer after 'fclose' causes undefined behavior
18511891
// according to cppreference.com .
1852-
ExplodedNode *N = C.generateErrorNode();
1853-
if (N) {
1854-
C.emitReport(std::make_unique<PathSensitiveBugReport>(
1855-
BT_UseAfterClose,
1856-
"Stream might be already closed. Causes undefined behaviour.", N));
1892+
if (ExplodedNode *N = C.generateErrorNode()) {
1893+
auto R = std::make_unique<PathSensitiveBugReport>(
1894+
BT_UseAfterClose, "Use of a stream that might be already closed", N);
1895+
R->addVisitor<StreamClosedVisitor>(Sym);
1896+
C.emitReport(std::move(R));
18571897
return nullptr;
18581898
}
18591899

clang/test/Analysis/stream-error.c

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ void error_fread(void) {
9696
}
9797
}
9898
fclose(F);
99-
Ret = fread(Buf, 1, 10, F); // expected-warning {{Stream might be already closed}}
99+
Ret = fread(Buf, 1, 10, F); // expected-warning {{Use of a stream that might be already closed}}
100100
}
101101

102102
void error_fwrite(void) {
@@ -113,7 +113,7 @@ void error_fwrite(void) {
113113
fwrite(0, 1, 10, F); // expected-warning {{might be 'indeterminate'}}
114114
}
115115
fclose(F);
116-
Ret = fwrite(0, 1, 10, F); // expected-warning {{Stream might be already closed}}
116+
Ret = fwrite(0, 1, 10, F); // expected-warning {{Use of a stream that might be already closed}}
117117
}
118118

119119
void error_fgetc(void) {
@@ -135,7 +135,7 @@ void error_fgetc(void) {
135135
}
136136
}
137137
fclose(F);
138-
fgetc(F); // expected-warning {{Stream might be already closed}}
138+
fgetc(F); // expected-warning {{Use of a stream that might be already closed}}
139139
}
140140

141141
void error_fgets(void) {
@@ -158,7 +158,7 @@ void error_fgets(void) {
158158
}
159159
}
160160
fclose(F);
161-
fgets(Buf, sizeof(Buf), F); // expected-warning {{Stream might be already closed}}
161+
fgets(Buf, sizeof(Buf), F); // expected-warning {{Use of a stream that might be already closed}}
162162
}
163163

164164
void error_fputc(int fd) {
@@ -176,7 +176,7 @@ void error_fputc(int fd) {
176176
fputc('Y', F); // no-warning
177177
}
178178
fclose(F);
179-
fputc('A', F); // expected-warning {{Stream might be already closed}}
179+
fputc('A', F); // expected-warning {{Use of a stream that might be already closed}}
180180
}
181181

182182
void error_fputs(void) {
@@ -194,7 +194,7 @@ void error_fputs(void) {
194194
fputs("QWD", F); // expected-warning {{might be 'indeterminate'}}
195195
}
196196
fclose(F);
197-
fputs("ABC", F); // expected-warning {{Stream might be already closed}}
197+
fputs("ABC", F); // expected-warning {{Use of a stream that might be already closed}}
198198
}
199199

200200
void error_fprintf(void) {
@@ -211,7 +211,7 @@ void error_fprintf(void) {
211211
fprintf(F, "bbb"); // expected-warning {{might be 'indeterminate'}}
212212
}
213213
fclose(F);
214-
fprintf(F, "ccc"); // expected-warning {{Stream might be already closed}}
214+
fprintf(F, "ccc"); // expected-warning {{Use of a stream that might be already closed}}
215215
}
216216

217217
void error_fscanf(int *A) {
@@ -236,7 +236,7 @@ void error_fscanf(int *A) {
236236
}
237237
}
238238
fclose(F);
239-
fscanf(F, "ccc"); // expected-warning {{Stream might be already closed}}
239+
fscanf(F, "ccc"); // expected-warning {{Use of a stream that might be already closed}}
240240
}
241241

242242
void error_ungetc(int TestIndeterminate) {
@@ -256,7 +256,7 @@ void error_ungetc(int TestIndeterminate) {
256256
ungetc('X', F); // expected-warning {{might be 'indeterminate'}}
257257
}
258258
fclose(F);
259-
ungetc('A', F); // expected-warning {{Stream might be already closed}}
259+
ungetc('A', F); // expected-warning {{Use of a stream that might be already closed}}
260260
}
261261

262262
void error_getdelim(char *P, size_t Sz) {
@@ -278,7 +278,7 @@ void error_getdelim(char *P, size_t Sz) {
278278
}
279279
}
280280
fclose(F);
281-
getdelim(&P, &Sz, '\n', F); // expected-warning {{Stream might be already closed}}
281+
getdelim(&P, &Sz, '\n', F); // expected-warning {{Use of a stream that might be already closed}}
282282
}
283283

284284
void error_getline(char *P, size_t Sz) {
@@ -300,7 +300,7 @@ void error_getline(char *P, size_t Sz) {
300300
}
301301
}
302302
fclose(F);
303-
getline(&P, &Sz, F); // expected-warning {{Stream might be already closed}}
303+
getline(&P, &Sz, F); // expected-warning {{Use of a stream that might be already closed}}
304304
}
305305

306306
void write_after_eof_is_allowed(void) {

clang/test/Analysis/stream-note.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,3 +264,12 @@ void error_fseek_read_eof(void) {
264264
fgetc(F); // no warning
265265
fclose(F);
266266
}
267+
268+
void check_note_at_use_after_close(void) {
269+
FILE *F = tmpfile();
270+
if (!F) // expected-note {{'F' is non-null}} expected-note {{Taking false branch}}
271+
return;
272+
fclose(F); // expected-note {{Stream is closed here}}
273+
rewind(F); // expected-warning {{Use of a stream that might be already closed}}
274+
// expected-note@-1 {{Use of a stream that might be already closed}}
275+
}

clang/test/Analysis/stream.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ void f_double_close(void) {
185185
if (!p)
186186
return;
187187
fclose(p);
188-
fclose(p); // expected-warning {{Stream might be already closed}}
188+
fclose(p); // expected-warning {{Use of a stream that might be already closed}}
189189
}
190190

191191
void f_double_close_alias(void) {
@@ -194,15 +194,15 @@ void f_double_close_alias(void) {
194194
return;
195195
FILE *p2 = p1;
196196
fclose(p1);
197-
fclose(p2); // expected-warning {{Stream might be already closed}}
197+
fclose(p2); // expected-warning {{Use of a stream that might be already closed}}
198198
}
199199

200200
void f_use_after_close(void) {
201201
FILE *p = fopen("foo", "r");
202202
if (!p)
203203
return;
204204
fclose(p);
205-
clearerr(p); // expected-warning {{Stream might be already closed}}
205+
clearerr(p); // expected-warning {{Use of a stream that might be already closed}}
206206
}
207207

208208
void f_open_after_close(void) {
@@ -266,7 +266,7 @@ void check_freopen_2(void) {
266266
if (f2) {
267267
// Check if f1 and f2 point to the same stream.
268268
fclose(f1);
269-
fclose(f2); // expected-warning {{Stream might be already closed.}}
269+
fclose(f2); // expected-warning {{Use of a stream that might be already closed}}
270270
} else {
271271
// Reopen failed.
272272
// f1 is non-NULL but points to a possibly invalid stream.
@@ -370,7 +370,7 @@ void fflush_after_fclose(void) {
370370
if ((Ret = fflush(F)) != 0)
371371
clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}}
372372
fclose(F);
373-
fflush(F); // expected-warning {{Stream might be already closed}}
373+
fflush(F); // expected-warning {{Use of a stream that might be already closed}}
374374
}
375375

376376
void fflush_on_open_failed_stream(void) {

0 commit comments

Comments
 (0)