Skip to content

Commit

Permalink
[analyzer] Note last "fclose" call from "ensureStreamOpened" (llvm#10…
Browse files Browse the repository at this point in the history 
…9112)

Patch by Arseniy Zaostrovnykh!
  • Loading branch information
@steakhal
steakhal authored Sep 18, 2024
1 parent 737f56f commit 2e3c7db
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 21 deletions.
50 changes: 45 additions & 5 deletions clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
View file
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
View file
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
View file
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
View file
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

0 comments on commit 2e3c7db

Please sign in to comment.