[clang] 14bc11a - [clang][dataflow]Use cast_or_null instead of cast to prevent crash (#68510)

[via cfe-commits]

Author: Qizhi Hu
Date: 2023-10-21T09:39:30+08:00
New Revision: 14bc11a651971bbbdf96565898be2c94fa2b1cf5

URL: https://github.com/llvm/llvm-project/commit/14bc11a651971bbbdf96565898be2c94fa2b1cf5
DIFF: https://github.com/llvm/llvm-project/commit/14bc11a651971bbbdf96565898be2c94fa2b1cf5.diff

LOG: [clang][dataflow]Use cast_or_null instead of cast to prevent crash (#68510)

`getStorageLocation` may return `nullptr` and this will produce crash
when use `cast`, use `dyn_cast_or_null` instead. I test it locally using
[FTXUI](https://github.com/ArthurSonzogni/FTXUI) and it may be the cause
of issue [issue](https://github.com/llvm/llvm-project/issues/68412), but
I am not sure.

Co-authored-by: huqizhi <huqizhi at 836744285@qq.com>

Added: 
    

Modified: 
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/absl/types/optional.h
    clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp
    clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
    clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 59e096282e77917..b5348384e965ab5 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -219,6 +219,10 @@ Changes in existing checks
   <clang-tidy/checks/bugprone/reserved-identifier>` check, so that it does not
   warn on macros starting with underscore and lowercase letter.
 
+- Improved :doc:`bugprone-unchecked-optional-access
+  <clang-tidy/checks/bugprone/unchecked-optional-access>` check, so that it does
+  not crash during handling of optional values.
+
 - Improved :doc:`bugprone-undefined-memory-manipulation
   <clang-tidy/checks/bugprone/undefined-memory-manipulation>` check to support
   fixed-size arrays of non-trivial types.

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/absl/types/optional.h b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/absl/types/optional.h
index 154cc262ab7cd79..3e692f347aa1ea5 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/absl/types/optional.h
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/absl/types/optional.h
@@ -66,6 +66,8 @@ class optional {
   void reset() noexcept;
 
   void swap(optional &rhs) noexcept;
+
+  template <typename U> optional &operator=(const U &u);
 };
 
 } // namespace absl

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp
index 1921291f2187d92..13a3ff52f3ebc59 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp
@@ -180,3 +180,23 @@ void std_forward_rvalue_ref_safe(absl::optional<int>&& opt) {
 
   std::forward<absl::optional<int>>(opt).value();
 }
+
+namespace std {
+
+template <typename T> class vector {
+public:
+  T &operator[](unsigned long index);
+  bool empty();
+};
+
+} // namespace std
+
+struct S {
+  absl::optional<float> x;
+};
+std::vector<S> vec;
+
+void foo() {
+  if (!vec.empty())
+    vec[0].x = 0;
+}

diff  --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index f61f26ff27804ec..8bd9a030f50cda0 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -599,7 +599,7 @@ void transferAssignment(const CXXOperatorCallExpr *E, BoolValue &HasValueVal,
                         LatticeTransferState &State) {
   assert(E->getNumArgs() > 0);
 
-  if (auto *Loc = cast<RecordStorageLocation>(
+  if (auto *Loc = cast_or_null<RecordStorageLocation>(
           State.Env.getStorageLocation(*E->getArg(0)))) {
     createOptionalValue(*Loc, HasValueVal, State.Env);
 

diff  --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index c41a378a8341b71..76af8baba851839 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2131,6 +2131,24 @@ TEST_P(UncheckedOptionalAccessTest, OptionalSwap) {
   )");
 }
 
+TEST_P(UncheckedOptionalAccessTest, OptionalReturnedFromFuntionCall) {
+  ExpectDiagnosticsFor(
+      R"(
+    #include "unchecked_optional_access_test.h"
+    
+    struct S {
+      $ns::$optional<float> x;
+    } s;
+    S getOptional() {
+      return s;
+    }
+
+    void target() {
+      getOptional().x = 0;
+    }
+  )");
+}
+
 TEST_P(UncheckedOptionalAccessTest, StdSwap) {
   ExpectDiagnosticsFor(
       R"(