[clang] [analyzer] Avoid crashes in the stream checker (PR #100901)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: None (vabridgers)

<details>
<summary>Changes</summary>

This change avoids crashes in the stream checker when bifurcating the analysis 
state produces a non-null return value for opening a stream and does not 
produces a null return value because of constraints found during an analysis 
path.

A snippet of the crash stack seen is shown here.

...
 #<!-- -->6 __restore_rt sigaction.c:0:0
 #<!-- -->7 clang::ento::ProgramState::getStateManager() const
     clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:148:13
 #<!-- -->8 llvm::IntrusiveRefCntPtr&lt;clang::ento::ProgramState const&gt;
     clang::ento::ProgramState::set&lt;(anonymous 
namespace)::StreamMap&gt;(clang::ento::ProgramStateTrait&lt;(
     anonymous 
namespace)::StreamMap&gt;::key_type,clang::ento::ProgramStateTrait&lt;(anonymous
 namespace)::StreamMap&gt;::value_type) const
     clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:864:10
 #<!-- -->9 (anonymous namespace)::StreamChecker::evalFopen((anonymous 
namespace)::FnDescription const*,
    clang::ento::CallEvent const&amp;, clang::ento::CheckerContext&amp;) const
     clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:951:13
...

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


2 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+12-8) 
- (added) clang/test/Analysis/stream-no-crash.c (+25) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 53770532609d5..c34c6b9fc6c0d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -946,14 +946,18 @@ void StreamChecker::evalFopen(const FnDescription *Desc, 
const CallEvent &Call,
   std::tie(StateNotNull, StateNull) =
       C.getConstraintManager().assumeDual(State, RetVal);
 
-  StateNotNull =
-      StateNotNull->set<StreamMap>(RetSym, StreamState::getOpened(Desc));
-  StateNull =
-      StateNull->set<StreamMap>(RetSym, StreamState::getOpenFailed(Desc));
-
-  C.addTransition(StateNotNull,
-                  constructLeakNoteTag(C, RetSym, "Stream opened here"));
-  C.addTransition(StateNull);
+  if (StateNotNull)
+    StateNotNull =
+        StateNotNull->set<StreamMap>(RetSym, StreamState::getOpened(Desc));
+  if (StateNull)
+    StateNull =
+        StateNull->set<StreamMap>(RetSym, StreamState::getOpenFailed(Desc));
+
+  if (StateNotNull)
+    C.addTransition(StateNotNull,
+                    constructLeakNoteTag(C, RetSym, "Stream opened here"));
+  if (StateNull)
+    C.addTransition(StateNull);
 }
 
 void StreamChecker::preFreopen(const FnDescription *Desc, const CallEvent 
&Call,
diff --git a/clang/test/Analysis/stream-no-crash.c 
b/clang/test/Analysis/stream-no-crash.c
new file mode 100644
index 0000000000000..7a4922c8a35c2
--- /dev/null
+++ b/clang/test/Analysis/stream-no-crash.c
@@ -0,0 +1,25 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Stream -verify %s
+
+// This test is isolate since it uses line markers to repro the problem.
+// The test is expected to find the issues noted below without crashing.
+
+# 1 "" 1
+# 1 "" 1
+# 1 "" 1
+# 1 "" 1 3
+typedef FILE;
+extern *stdout;
+char a;
+*fopen();
+# 0 "" 2
+# 7 "" 2
+# 7 "" 2
+# 7 "" 2
+void b() {
+  fopen(&a, "");
+  int c = stdout && c;
+  b();
+}
+// expected-warning@-3{{Assigned value is garbage or undefined}}
+// expected-warning@-4{{Opened stream never closed. Potential resource leak}}
+

``````````

</details>


https://github.com/llvm/llvm-project/pull/100901
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits