[clang][dataflow] Add a test demonstrating an issue in unchecked-optional-access-check

[jvoung]

createStorageLocation used in transferCallReturningOptional:

llvm-project/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp

Line 515 in 09ba83b

Loc = &cast<RecordStorageLocation>(State.Env.createStorageLocation(*E));

can stop recursively creating storage locations when it hits a field of
reference type for a non-optional record:

llvm-project/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp

Line 67 in 3ca5d80

if (Field->getType()->isReferenceType())

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.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Jan Voung (jvoung)

Changes

createStorageLocation used in transferCallReturningOptional:

llvm-project/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp

Line 515 in 09ba83b

Loc = &cast<RecordStorageLocation>(State.Env.createStorageLocation(*E));

can stop recursively creating storage locations when it hits a field of
reference type for a non-optional record:

llvm-project/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp

Line 67 in 3ca5d80

if (Field->getType()->isReferenceType())

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.

---

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

1 Files Affected:

• (modified) clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp (+34)

diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index 4227a6bfdeba87..877bfce8b27092 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2873,6 +2873,40 @@ TEST_P(UncheckedOptionalAccessTest, OptionalValueStruct) {
   )");
 }
 
+// FIXME: 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"(