[clang] [clang][dataflow] Add a test demonstrating an issue in unchecked-optional-access-check (PR #110870)

[Jan Voung via cfe-commits]

https://github.com/jvoung updated https://github.com/llvm/llvm-project/pull/110870

>From d148d4b3187d507fb1ba1735a3111c3eac2d2157 Mon Sep 17 00:00:00 2001
From: Jan Voung <jvoung at gmail.com>
Date: Wed, 2 Oct 2024 15:26:32 +0000
Subject: [PATCH 1/2] [clang][dataflow] Add a test demonstrating an issue in
 unchecked-optional-access-check

createStorageLocation used in transferCallReturningOptional:
https://github.com/llvm/llvm-project/blob/09ba83be0ac178851e3c9c9c8fefddbdd4d8353f/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp#L515
can stop recursively creating storage locations when it hits a field of
reference type for a non-optional record:
https://github.com/llvm/llvm-project/blob/3ca5d8082a0c6bd9520544ce3bca11bf3e02a5fa/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp#L67

If an optional is reached through that field then it may not have a
storage location by the type we handle has_value in a transfer function.
---
 .../UncheckedOptionalAccessModelTest.cpp      | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index 4227a6bfdeba87..1f481c0761dc33 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2873,6 +2873,40 @@ TEST_P(UncheckedOptionalAccessTest, OptionalValueStruct) {
   )");
 }
 
+// A case that we should handle but currently don't.
+// When there is a field of type reference to non-optional, we may
+// stop recursively creating storage locations.
+// E.g., the field `second` below in `pair` should eventually lead to
+// the optional `x` in `A`.
+TEST_P(UncheckedOptionalAccessTest, NestedOptionalThroughNonOptionalRefField) {
+  ExpectDiagnosticsFor(R"(
+    #include "unchecked_optional_access_test.h"
+
+    struct A {
+      $ns::$optional<int> x;
+    };
+
+    struct pair {
+      int first;
+      const A &second;
+    };
+
+    struct B {
+      $ns::$optional<pair>& nonConstGetRef();
+   };
+
+    void target(B b) {
+      const auto& maybe_pair = b.nonConstGetRef();
+      if (!maybe_pair.has_value())
+        return;
+
+      if(!maybe_pair->second.x.has_value())
+        return;
+     maybe_pair->second.x.value();  // [[unsafe]]
+    }
+  )");
+}
+
 TEST_P(UncheckedOptionalAccessTest, OptionalValueInitialization) {
   ExpectDiagnosticsFor(
       R"(

>From d3a5952bd44ac5d7c6c9fadb96adcd85c9d79419 Mon Sep 17 00:00:00 2001
From: Jan Voung <jvoung at gmail.com>
Date: Wed, 2 Oct 2024 15:36:45 +0000
Subject: [PATCH 2/2] Fix formatting

---
 .../FlowSensitive/UncheckedOptionalAccessModelTest.cpp        | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index 1f481c0761dc33..728f410d96a72b 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2893,7 +2893,7 @@ TEST_P(UncheckedOptionalAccessTest, NestedOptionalThroughNonOptionalRefField) {
 
     struct B {
       $ns::$optional<pair>& nonConstGetRef();
-   };
+    };
 
     void target(B b) {
       const auto& maybe_pair = b.nonConstGetRef();
@@ -2902,7 +2902,7 @@ TEST_P(UncheckedOptionalAccessTest, NestedOptionalThroughNonOptionalRefField) {
 
       if(!maybe_pair->second.x.has_value())
         return;
-     maybe_pair->second.x.value();  // [[unsafe]]
+      maybe_pair->second.x.value();  // [[unsafe]]
     }
   )");
 }