[clang] 2e3c7db - [analyzer] Note last "fclose" call from "ensureStreamOpened" (#109112)

via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 18 03:22:05 PDT 2024


Author: Balazs Benics
Date: 2024-09-18T12:22:02+02:00
New Revision: 2e3c7dbbcbfa37ae83251bb3da388df772680689

URL: https://github.com/llvm/llvm-project/commit/2e3c7dbbcbfa37ae83251bb3da388df772680689
DIFF: https://github.com/llvm/llvm-project/commit/2e3c7dbbcbfa37ae83251bb3da388df772680689.diff

LOG: [analyzer] Note last "fclose" call from "ensureStreamOpened" (#109112)

Patch by Arseniy Zaostrovnykh!

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
    clang/test/Analysis/stream-error.c
    clang/test/Analysis/stream-note.c
    clang/test/Analysis/stream.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 8bb7880a3cc283..0a823a1126ce3f 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,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;
     }
 

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 3aef707d50056e..2b5d1edb2814f0 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 {{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