Skip to content

[analyzer] Note last "fclose" call from "ensureStreamOpened" #109112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 45 additions & 5 deletions clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -1849,11 +1889,11 @@ 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>(
BT_UseAfterClose,
"Stream might be already closed. Causes undefined behaviour.", N));
if (ExplodedNode *N = C.generateErrorNode()) {
auto R = std::make_unique<PathSensitiveBugReport>(
BT_UseAfterClose, "Use of a stream that might be already closed", N);
R->addVisitor<StreamClosedVisitor>(Sym);
C.emitReport(std::move(R));
return nullptr;
}

Expand Down
22 changes: 11 additions & 11 deletions clang/test/Analysis/stream-error.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
9 changes: 9 additions & 0 deletions clang/test/Analysis/stream-note.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 {{Use of a stream that might be already closed}}
// expected-note@-1 {{Use of a stream that might be already closed}}
}
10 changes: 5 additions & 5 deletions clang/test/Analysis/stream.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -194,15 +194,15 @@ 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) {
FILE *p = fopen("foo", "r");
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) {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down
Loading