[clang][analyzer] Restrict 'fopen' modeling to POSIX versions in SimpleStreamChecker

[benshi001]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Ben Shi (benshi001)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/72016.diff

2 Files Affected:

• (modified) clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp (+21-29)
• (modified) clang/test/Analysis/stream-non-posix-function.c (+1)

diff --git a/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
index 32d95e944195390..d78761b0ea4553b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
@@ -1,4 +1,4 @@
-//===-- SimpleStreamChecker.cpp -----------------------------------------*- C++ -*--//
+//===-- SimpleStreamChecker.cpp -----------------------------------*- C++ -*--//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -31,7 +31,7 @@ typedef SmallVector<SymbolRef, 2> SymbolVector;
 struct StreamState {
 private:
   enum Kind { Opened, Closed } K;
-  StreamState(Kind InK) : K(InK) { }
+  StreamState(Kind InK) : K(InK) {}
 
 public:
   bool isOpened() const { return K == Opened; }
@@ -40,25 +40,19 @@ struct StreamState {
   static StreamState getOpened() { return StreamState(Opened); }
   static StreamState getClosed() { return StreamState(Closed); }
 
-  bool operator==(const StreamState &X) const {
-    return K == X.K;
-  }
-  void Profile(llvm::FoldingSetNodeID &ID) const {
-    ID.AddInteger(K);
-  }
+  bool operator==(const StreamState &X) const { return K == X.K; }
+  void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddInteger(K); }
 };
 
-class SimpleStreamChecker : public Checker<check::PostCall,
-                                           check::PreCall,
-                                           check::DeadSymbols,
-                                           check::PointerEscape> {
+class SimpleStreamChecker
+    : public Checker<check::PostCall, check::PreCall, check::DeadSymbols,
+                     check::PointerEscape> {
   CallDescription OpenFn, CloseFn;
 
   std::unique_ptr<BugType> DoubleCloseBugType;
   std::unique_ptr<BugType> LeakBugType;
 
-  void reportDoubleClose(SymbolRef FileDescSym,
-                         const CallEvent &Call,
+  void reportDoubleClose(SymbolRef FileDescSym, const CallEvent &Call,
                          CheckerContext &C) const;
 
   void reportLeaks(ArrayRef<SymbolRef> LeakedStreams, CheckerContext &C,
@@ -78,9 +72,9 @@ class SimpleStreamChecker : public Checker<check::PostCall,
 
   /// Stop tracking addresses which escape.
   ProgramStateRef checkPointerEscape(ProgramStateRef State,
-                                    const InvalidatedSymbols &Escaped,
-                                    const CallEvent *Call,
-                                    PointerEscapeKind Kind) const;
+                                     const InvalidatedSymbols &Escaped,
+                                     const CallEvent *Call,
+                                     PointerEscapeKind Kind) const;
 };
 
 } // end anonymous namespace
@@ -90,15 +84,14 @@ class SimpleStreamChecker : public Checker<check::PostCall,
 REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)
 
 SimpleStreamChecker::SimpleStreamChecker()
-    : OpenFn({"fopen"}), CloseFn({"fclose"}, 1) {
+    : OpenFn({"fopen"}, 2), CloseFn({"fclose"}, 1) {
   // Initialize the bug types.
   DoubleCloseBugType.reset(
       new BugType(this, "Double fclose", "Unix Stream API Error"));
 
   // Sinks are higher importance bugs as well as calls to assert() or exit(0).
-  LeakBugType.reset(
-      new BugType(this, "Resource Leak", "Unix Stream API Error",
-                  /*SuppressOnSink=*/true));
+  LeakBugType.reset(new BugType(this, "Resource Leak", "Unix Stream API Error",
+                                /*SuppressOnSink=*/true));
 }
 
 void SimpleStreamChecker::checkPostCall(const CallEvent &Call,
@@ -146,8 +139,8 @@ void SimpleStreamChecker::checkPreCall(const CallEvent &Call,
   C.addTransition(State);
 }
 
-static bool isLeaked(SymbolRef Sym, const StreamState &SS,
-                     bool IsSymDead, ProgramStateRef State) {
+static bool isLeaked(SymbolRef Sym, const StreamState &SS, bool IsSymDead,
+                     ProgramStateRef State) {
   if (IsSymDead && SS.isOpened()) {
     // If a symbol is NULL, assume that fopen failed on this path.
     // A symbol should only be considered leaked if it is non-null.
@@ -212,7 +205,8 @@ void SimpleStreamChecker::reportLeaks(ArrayRef<SymbolRef> LeakedStreams,
   }
 }
 
-bool SimpleStreamChecker::guaranteedNotToCloseFile(const CallEvent &Call) const{
+bool SimpleStreamChecker::guaranteedNotToCloseFile(
+    const CallEvent &Call) const {
   // If it's not in a system header, assume it might close a file.
   if (!Call.isInSystemHeader())
     return false;
@@ -229,11 +223,9 @@ bool SimpleStreamChecker::guaranteedNotToCloseFile(const CallEvent &Call) const{
 
 // If the pointer we are tracking escaped, do not track the symbol as
 // we cannot reason about it anymore.
-ProgramStateRef
-SimpleStreamChecker::checkPointerEscape(ProgramStateRef State,
-                                        const InvalidatedSymbols &Escaped,
-                                        const CallEvent *Call,
-                                        PointerEscapeKind Kind) const {
+ProgramStateRef SimpleStreamChecker::checkPointerEscape(
+    ProgramStateRef State, const InvalidatedSymbols &Escaped,
+    const CallEvent *Call, PointerEscapeKind Kind) const {
   // If we know that the call cannot close a file, there is nothing to do.
   if (Kind == PSK_DirectEscapeOnCall && guaranteedNotToCloseFile(*Call)) {
     return State;
diff --git a/clang/test/Analysis/stream-non-posix-function.c b/clang/test/Analysis/stream-non-posix-function.c
index 70b3ab25d026532..ab7c60a2c6c76e6 100644
--- a/clang/test/Analysis/stream-non-posix-function.c
+++ b/clang/test/Analysis/stream-non-posix-function.c
@@ -1,4 +1,5 @@
 // RUN: %clang_analyze_cc1 -fno-builtin -analyzer-checker=core,alpha.unix.Stream -verify %s
+// RUN: %clang_analyze_cc1 -fno-builtin -analyzer-checker=core,alpha.unix.SimpleStream -verify %s
 // expected-no-diagnostics
 
 typedef struct _FILE FILE;

[balazske]

The functional change looks good but the reformatting should be put into a separate change.

[benshi001]

The functional change looks good but the reformatting should be put into a separate change.

Done. Thanks.

[balazske]

The change is OK, automated tests show a failure but it is in an unrelated test.