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

[via cfe-commits]

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

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
...

>From ab047fb7bd9ddc0f58d4652fe63c80738d60e8c4 Mon Sep 17 00:00:00 2001
From: Vince Bridgers <vince.a.bridgers at ericsson.com>
Date: Sat, 27 Jul 2024 23:41:17 +0200
Subject: [PATCH] [analyzer] Avoid crashes in the stream checker

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
...
---
 .../StaticAnalyzer/Checkers/StreamChecker.cpp | 20 +++++++++------
 clang/test/Analysis/stream-no-crash.c         | 25 +++++++++++++++++++
 2 files changed, 37 insertions(+), 8 deletions(-)
 create mode 100644 clang/test/Analysis/stream-no-crash.c

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}}
+