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 <jvo...@gmail.com> Date: Wed, 2 Oct 2024 15:26:32 +0000 Subject: [PATCH 1/3] [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 <jvo...@gmail.com> Date: Wed, 2 Oct 2024 15:36:45 +0000 Subject: [PATCH 2/3] 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]] } )"); } >From 3c5f997bb8b1b7aa411d6e70b66dc44568f8ba15 Mon Sep 17 00:00:00 2001 From: Jan Voung <jvo...@gmail.com> Date: Wed, 2 Oct 2024 15:46:27 +0000 Subject: [PATCH 3/3] Tag as fixme --- .../Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp index 728f410d96a72b..877bfce8b27092 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp @@ -2873,7 +2873,7 @@ TEST_P(UncheckedOptionalAccessTest, OptionalValueStruct) { )"); } -// A case that we should handle but currently don't. +// 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
