Skip to content

Conversation

@andrewshadura
Copy link

Dereferencing a std::optional that does not contain a value is undefined behaviour. It is better to crash when this happens to prevent potential abuse or a crash further down the line.

[reworded the commit message]
Co-Authored-By: Andrej Shadura [email protected]

Dereferencing a std::optional that does not contain a value is
undefined behaviour. It is better to crash when this happens to
prevent potential abuse or a crash further down the line.

[reworded the commit message]
Co-Authored-By: Andrej Shadura <[email protected]>
@andrewshadura andrewshadura requested a review from a team as a code owner November 15, 2024 15:10
@github-actions
Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

Copy link
Contributor

@philnik777 philnik777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want your program to crash on this kind of UB you should enable a hardening mode. See https://libcxx.llvm.org/Hardening.html for more details.

@github-actions
Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Nov 16, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 16, 2024

@llvm/pr-subscribers-libcxx

Author: Andrej Shadura (andrewshadura)

Changes

Dereferencing a std::optional that does not contain a value is undefined behaviour. It is better to crash when this happens to prevent potential abuse or a crash further down the line.

[reworded the commit message]
Co-Authored-By: Andrej Shadura <[email protected]>


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

1 Files Affected:

  • (modified) libcxx/include/optional (+32-6)
diff --git a/libcxx/include/optional b/libcxx/include/optional
index 7ad6a9e116941f..71c0eabed446dc 100644
--- a/libcxx/include/optional
+++ b/libcxx/include/optional
@@ -177,6 +177,8 @@ namespace std {
 
 */
 
+#include <stdio.h>
+
 #include <__assert>
 #include <__compare/compare_three_way_result.h>
 #include <__compare/ordering.h>
@@ -791,33 +793,57 @@ public:
     }
   }
 
-  _LIBCPP_HIDE_FROM_ABI constexpr add_pointer_t<value_type const> operator->() const noexcept {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_AVAILABILITY_THROW_BAD_OPTIONAL_ACCESS constexpr add_pointer_t<value_type const> operator->() const noexcept {
     _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator-> called on a disengaged value");
+        if (!this->has_value()) {
+          fprintf(stderr, "optional operator-> called on a disengaged value\n");
+          __builtin_trap();
+        }
     return std::addressof(this->__get());
   }
 
-  _LIBCPP_HIDE_FROM_ABI constexpr add_pointer_t<value_type> operator->() noexcept {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_AVAILABILITY_THROW_BAD_OPTIONAL_ACCESS constexpr add_pointer_t<value_type> operator->() noexcept {
     _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator-> called on a disengaged value");
+        if (!this->has_value()) {
+          fprintf(stderr, "optional operator-> called on a disengaged value\n");
+          __builtin_trap();
+        }
     return std::addressof(this->__get());
   }
 
-  _LIBCPP_HIDE_FROM_ABI constexpr const value_type& operator*() const& noexcept {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_AVAILABILITY_THROW_BAD_OPTIONAL_ACCESS constexpr const value_type& operator*() const& noexcept {
     _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
+        if (!this->has_value()) {
+          fprintf(stderr, "optional operator-> called on a disengaged value\n");
+          __builtin_trap();
+        }
     return this->__get();
   }
 
-  _LIBCPP_HIDE_FROM_ABI constexpr value_type& operator*() & noexcept {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_AVAILABILITY_THROW_BAD_OPTIONAL_ACCESS constexpr value_type& operator*() & noexcept {
     _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
+        if (!this->has_value()) {
+          fprintf(stderr, "optional operator-> called on a disengaged value\n");
+          __builtin_trap();
+        }
     return this->__get();
   }
 
-  _LIBCPP_HIDE_FROM_ABI constexpr value_type&& operator*() && noexcept {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_AVAILABILITY_THROW_BAD_OPTIONAL_ACCESS constexpr value_type&& operator*() && noexcept {
     _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
+        if (!this->has_value()) {
+          fprintf(stderr, "optional operator-> called on a disengaged value\n");
+          __builtin_trap();
+        }
     return std::move(this->__get());
   }
 
-  _LIBCPP_HIDE_FROM_ABI constexpr const value_type&& operator*() const&& noexcept {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_AVAILABILITY_THROW_BAD_OPTIONAL_ACCESS constexpr const value_type&& operator*() const&& noexcept {
     _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
+        if (!this->has_value()) {
+          fprintf(stderr, "optional operator-> called on a disengaged value\n");
+          __builtin_trap();
+        }
     return std::move(this->__get());
   }
 

@philnik777 philnik777 closed this Nov 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants