[libc++] Guard call_once against operator hijacking.

[mordante]

No description provided.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

---

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

3 Files Affected:

• (modified) libcxx/include/__mutex/once_flag.h (+4-3)
• (modified) libcxx/test/std/thread/thread.mutex/thread.once/thread.once.callonce/call_once.pass.cpp (+12)
• (modified) libcxx/test/support/operator_hijacker.h (+1)

diff --git a/libcxx/include/__mutex/once_flag.h b/libcxx/include/__mutex/once_flag.h
index 08ff54bf99265..2b0ba7781faa4 100644
--- a/libcxx/include/__mutex/once_flag.h
+++ b/libcxx/include/__mutex/once_flag.h
@@ -11,6 +11,7 @@
 
 #include <__config>
 #include <__functional/invoke.h>
+#include <__memory/addressof.h>
 #include <__memory/shared_count.h> // __libcpp_acquire_load
 #include <__tuple/tuple_indices.h>
 #include <__tuple/tuple_size.h>
@@ -128,7 +129,7 @@ inline _LIBCPP_HIDE_FROM_ABI void call_once(once_flag& __flag, _Callable&& __fun
     typedef tuple<_Callable&&, _Args&&...> _Gp;
     _Gp __f(std::forward<_Callable>(__func), std::forward<_Args>(__args)...);
     __call_once_param<_Gp> __p(__f);
-    std::__call_once(__flag.__state_, &__p, &__call_once_proxy<_Gp>);
+    std::__call_once(__flag.__state_, std::addressof(__p), std::addressof(__call_once_proxy<_Gp>));
   }
 }
 
@@ -138,7 +139,7 @@ template <class _Callable>
 inline _LIBCPP_HIDE_FROM_ABI void call_once(once_flag& __flag, _Callable& __func) {
   if (__libcpp_acquire_load(&__flag.__state_) != once_flag::_Complete) {
     __call_once_param<_Callable> __p(__func);
-    std::__call_once(__flag.__state_, &__p, &__call_once_proxy<_Callable>);
+    std::__call_once(__flag.__state_, std::addressof(__p), std::addressof(__call_once_proxy<_Callable>));
   }
 }
 
@@ -146,7 +147,7 @@ template <class _Callable>
 inline _LIBCPP_HIDE_FROM_ABI void call_once(once_flag& __flag, const _Callable& __func) {
   if (__libcpp_acquire_load(&__flag.__state_) != once_flag::_Complete) {
     __call_once_param<const _Callable> __p(__func);
-    std::__call_once(__flag.__state_, &__p, &__call_once_proxy<const _Callable>);
+    std::__call_once(__flag.__state_, std::addressof(__p), std::addressof(__call_once_proxy<const _Callable>));
   }
 }
 
diff --git a/libcxx/test/std/thread/thread.mutex/thread.once/thread.once.callonce/call_once.pass.cpp b/libcxx/test/std/thread/thread.mutex/thread.once/thread.once.callonce/call_once.pass.cpp
index 7708efcb54c7f..6874fa671c338 100644
--- a/libcxx/test/std/thread/thread.mutex/thread.once/thread.once.callonce/call_once.pass.cpp
+++ b/libcxx/test/std/thread/thread.mutex/thread.once/thread.once.callonce/call_once.pass.cpp
@@ -21,6 +21,7 @@
 
 #include "make_test_thread.h"
 #include "test_macros.h"
+#include "operator_hijacker.h"
 
 typedef std::chrono::milliseconds ms;
 
@@ -253,7 +254,18 @@ int main(int, char**)
         std::call_once(f2, std::move(rq));
         assert(rq.rv_called == 1);
     }
+	{
+		std::once_flag flag;
+		auto f = [](const operator_hijacker&){};
+        std::call_once(flag, f, operator_hijacker{});
+	}
+
 #endif // TEST_STD_VER >= 11
+	{
+		std::once_flag flag;
+		operator_hijacker f;
+        std::call_once(flag, f);
+	}
 
   return 0;
 }
diff --git a/libcxx/test/support/operator_hijacker.h b/libcxx/test/support/operator_hijacker.h
index fbd8b58ab0f83..ee76e312eba1c 100644
--- a/libcxx/test/support/operator_hijacker.h
+++ b/libcxx/test/support/operator_hijacker.h
@@ -23,6 +23,7 @@
 struct operator_hijacker {
   TEST_CONSTEXPR bool operator<(const operator_hijacker&) const { return true; }
   TEST_CONSTEXPR bool operator==(const operator_hijacker&) const { return true; }
+  TEST_CONSTEXPR void operator()() const {}
 
   template <typename T>
   friend void operator&(T&&) = delete;

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[ldionne]

I don't think we want to introduce TEST_USE_FROZEN_CXX03_HEADERS. Instead, we should mark the test as XFAIL: CXX03-FROZEN-HEADERS-FIXME (or whatever it is). I know this means the whole test is going to be excluded, however the idea behind CXX03-FROZEN-HEADERS-FIXME is that these annotations will (almost) all get removed at some point before we actually flip that switch. And if I've misunderstood and the plan is actually to carry this debt forever in our test suite, I think we need to factor that into our decision for making that switch in the first place. CC @philnik777

[philnik777]

Yeah, I don't think we should introduce this macro. The plan is to remove all the annotations one way or another and then flip to the frozen headers.

[mordante]

Thanks for the info. I've applied Louis' suggestion.

[ldionne]

LGTM w/ comment applied, thanks!

[ldionne]

This is not needed anymore.