balazske updated this revision to Diff 270140.
balazske added a comment.

- Report every path of resource leak.
- Do not report if non-returning function was encountered.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81407/new/

https://reviews.llvm.org/D81407

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

Index: clang/test/Analysis/stream.c
===================================================================
--- clang/test/Analysis/stream.c
+++ clang/test/Analysis/stream.c
@@ -139,7 +139,7 @@
   if (!p)
     return;
   if(c)
-    return; // expected-warning {{Opened File never closed. Potential Resource leak}}
+    return; // expected-warning {{Opened stream never closed. Potential resource leak}}
   fclose(p);
 }
 
Index: clang/test/Analysis/stream-note.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/stream-note.c
@@ -0,0 +1,48 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-store region -analyzer-output text -verify %s
+
+#include "Inputs/system-header-simulator.h"
+
+void check_note_at_correct_open() {
+  FILE *F1 = tmpfile(); // expected-note {{Stream opened here}}
+  if (!F1)
+    // expected-note@-1 {{'F1' is non-null}}
+    // expected-note@-2 {{Taking false branch}}
+    return;
+  FILE *F2 = tmpfile();
+  if (!F2) {
+    // expected-note@-1 {{'F2' is non-null}}
+    // expected-note@-2 {{Taking false branch}}
+    fclose(F1);
+    return;
+  }
+  rewind(F2);
+  fclose(F2);
+  rewind(F1);
+}
+// expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
+// expected-note@-2 {{Opened stream never closed. Potential resource leak}}
+
+void check_note_fopen() {
+  FILE *F = fopen("file", "r"); // expected-note {{Stream opened here}}
+  if (!F)
+    // expected-note@-1 {{'F' is non-null}}
+    // expected-note@-2 {{Taking false branch}}
+    return;
+}
+// expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
+// expected-note@-2 {{Opened stream never closed. Potential resource leak}}
+
+void check_note_freopen() {
+  FILE *F = fopen("file", "r"); // expected-note {{Stream opened here}}
+  if (!F)
+    // expected-note@-1 {{'F' is non-null}}
+    // expected-note@-2 {{Taking false branch}}
+    return;
+  F = freopen(0, "w", F); // expected-note {{Stream reopened here}}
+  if (!F)
+    // expected-note@-1 {{'F' is non-null}}
+    // expected-note@-2 {{Taking false branch}}
+    return;
+}
+// expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
+// expected-note@-2 {{Opened stream never closed. Potential resource leak}}
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -216,8 +216,8 @@
                           "Read function called when stream is in EOF state. "
                           "Function has no effect."};
   BuiltinBug BT_ResourceLeak{
-      this, "Resource Leak",
-      "Opened File never closed. Potential Resource leak."};
+      this, "Resource leak",
+      "Opened stream never closed. Potential resource leak."};
 
 public:
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
@@ -365,6 +365,20 @@
 
     return FnDescriptions.lookup(Call);
   }
+
+  /// Generate a message for BugReporterVisitor if the stored symbol is
+  /// marked as interesting by the actual bug report.
+  struct NoteFn {
+    SymbolRef StreamSym;
+    std::string Message;
+
+    std::string operator()(PathSensitiveBugReport &BR) const {
+      if (BR.isInteresting(StreamSym))
+        return Message;
+
+      return "";
+    }
+  };
 };
 
 } // end anonymous namespace
@@ -421,7 +435,8 @@
   StateNull =
       StateNull->set<StreamMap>(RetSym, StreamState::getOpenFailed(Desc));
 
-  C.addTransition(StateNotNull);
+  const NoteTag *T = C.getNoteTag(NoteFn{RetSym, "Stream opened here"});
+  C.addTransition(StateNotNull, T);
   C.addTransition(StateNull);
 }
 
@@ -476,7 +491,8 @@
   StateRetNull =
       StateRetNull->set<StreamMap>(StreamSym, StreamState::getOpenFailed(Desc));
 
-  C.addTransition(StateRetNotNull);
+  const NoteTag *T = C.getNoteTag(NoteFn{StreamSym, "Stream reopened here"});
+  C.addTransition(StateRetNotNull, T);
   C.addTransition(StateRetNull);
 }
 
@@ -921,8 +937,17 @@
     if (!N)
       continue;
 
-    C.emitReport(std::make_unique<PathSensitiveBugReport>(
-        BT_ResourceLeak, BT_ResourceLeak.getDescription(), N));
+    // Do not warn for non-closed stream at program exit.
+    ExplodedNode *Pred = C.getPredecessor();
+    if (Pred && Pred->getCFGBlock() &&
+        Pred->getCFGBlock()->hasNoReturnElement())
+      continue;
+
+    std::unique_ptr<PathSensitiveBugReport> R =
+        std::make_unique<PathSensitiveBugReport>(
+            BT_ResourceLeak, BT_ResourceLeak.getDescription(), N);
+    R->markInteresting(Sym);
+    C.emitReport(std::move(R));
   }
 }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to