[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<clang::ento::ProgramState const>
     clang::ento::ProgramState::set<(anonymous namespace)::StreamMap>(clang::ento::ProgramStateTrait<(
     anonymous namespace)::StreamMap>::key_type,clang::ento::ProgramStateTrait<(anonymous namespace)::StreamMap>::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&, clang::ento::CheckerContext&) 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 at -3{{Assigned value is garbage or undefined}}
+// expected-warning at -4{{Opened stream never closed. Potential resource leak}}
+

``````````

</details>


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