[NFC][libc++] Guard 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/129453.diff

4 Files Affected:

• (modified) libcxx/include/__functional/function.h (+4-4)
• (modified) libcxx/include/__numeric/gcd_lcm.h (+2-1)
• (modified) libcxx/include/__numeric/saturation_arithmetic.h (+4-3)
• (modified) libcxx/include/locale (+22-22)

diff --git a/libcxx/include/__functional/function.h b/libcxx/include/__functional/function.h
index cb800c6b3d192..f33f424c66c22 100644
--- a/libcxx/include/__functional/function.h
+++ b/libcxx/include/__functional/function.h
@@ -437,7 +437,7 @@ class __value_func<_Rp(_ArgTypes...)> {
   }
 
   _LIBCPP_HIDE_FROM_ABI void swap(__value_func& __f) _NOEXCEPT {
-    if (&__f == this)
+    if (std::addressof(__f) == this)
       return;
     if ((void*)__f_ == &__buf_ && (void*)__f.__f_ == &__f.__buf_) {
       _LIBCPP_SUPPRESS_DEPRECATED_PUSH
@@ -550,8 +550,8 @@ struct __policy {
   template <typename _Fun>
   _LIBCPP_HIDE_FROM_ABI static const __policy* __choose_policy(/* is_small = */ false_type) {
     static constexpr __policy __policy = {
-        &__large_clone<_Fun>,
-        &__large_destroy<_Fun>,
+        std::addressof(__large_clone<_Fun>),
+        std::addressof(__large_destroy<_Fun>),
         false,
 #  if _LIBCPP_HAS_RTTI
         &typeid(typename _Fun::_Target)
@@ -600,7 +600,7 @@ struct __policy_invoker<_Rp(_ArgTypes...)> {
   // Creates an invoker that calls the given instance of __func.
   template <typename _Fun>
   _LIBCPP_HIDE_FROM_ABI static __policy_invoker __create() {
-    return __policy_invoker(&__call_impl<_Fun>);
+    return __policy_invoker(std::addressof(__call_impl<_Fun>));
   }
 
 private:
diff --git a/libcxx/include/__numeric/gcd_lcm.h b/libcxx/include/__numeric/gcd_lcm.h
index f15f64ea5568d..ce58f8698f726 100644
--- a/libcxx/include/__numeric/gcd_lcm.h
+++ b/libcxx/include/__numeric/gcd_lcm.h
@@ -14,6 +14,7 @@
 #include <__assert>
 #include <__bit/countr.h>
 #include <__config>
+#include <__memory/addressof.h>
 #include <__type_traits/common_type.h>
 #include <__type_traits/is_integral.h>
 #include <__type_traits/is_same.h>
@@ -115,7 +116,7 @@ constexpr _LIBCPP_HIDE_FROM_ABI common_type_t<_Tp, _Up> lcm(_Tp __m, _Up __n) {
   _Rp __val1 = __ct_abs<_Rp, _Tp>()(__m) / std::gcd(__m, __n);
   _Rp __val2 = __ct_abs<_Rp, _Up>()(__n);
   _Rp __res;
-  [[maybe_unused]] bool __overflow = __builtin_mul_overflow(__val1, __val2, &__res);
+  [[maybe_unused]] bool __overflow = __builtin_mul_overflow(__val1, __val2, std::addressof(__res));
   _LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(!__overflow, "Overflow in lcm");
   return __res;
 }
diff --git a/libcxx/include/__numeric/saturation_arithmetic.h b/libcxx/include/__numeric/saturation_arithmetic.h
index 2390b42aaec31..4110a8cb142a5 100644
--- a/libcxx/include/__numeric/saturation_arithmetic.h
+++ b/libcxx/include/__numeric/saturation_arithmetic.h
@@ -13,6 +13,7 @@
 #include <__assert>
 #include <__concepts/arithmetic.h>
 #include <__config>
+#include <__memory/addressof.h>
 #include <__utility/cmp.h>
 #include <limits>
 
@@ -29,7 +30,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <__libcpp_integer _Tp>
 _LIBCPP_HIDE_FROM_ABI constexpr _Tp __add_sat(_Tp __x, _Tp __y) noexcept {
-  if (_Tp __sum; !__builtin_add_overflow(__x, __y, &__sum))
+  if (_Tp __sum; !__builtin_add_overflow(__x, __y, std::addressof(__sum)))
     return __sum;
   // Handle overflow
   if constexpr (__libcpp_unsigned_integer<_Tp>) {
@@ -47,7 +48,7 @@ _LIBCPP_HIDE_FROM_ABI constexpr _Tp __add_sat(_Tp __x, _Tp __y) noexcept {
 
 template <__libcpp_integer _Tp>
 _LIBCPP_HIDE_FROM_ABI constexpr _Tp __sub_sat(_Tp __x, _Tp __y) noexcept {
-  if (_Tp __sub; !__builtin_sub_overflow(__x, __y, &__sub))
+  if (_Tp __sub; !__builtin_sub_overflow(__x, __y, std::addressof(__sub)))
     return __sub;
   // Handle overflow
   if constexpr (__libcpp_unsigned_integer<_Tp>) {
@@ -66,7 +67,7 @@ _LIBCPP_HIDE_FROM_ABI constexpr _Tp __sub_sat(_Tp __x, _Tp __y) noexcept {
 
 template <__libcpp_integer _Tp>
 _LIBCPP_HIDE_FROM_ABI constexpr _Tp __mul_sat(_Tp __x, _Tp __y) noexcept {
-  if (_Tp __mul; !__builtin_mul_overflow(__x, __y, &__mul))
+  if (_Tp __mul; !__builtin_mul_overflow(__x, __y, std::addressof(__mul)))
     return __mul;
   // Handle overflow
   if constexpr (__libcpp_unsigned_integer<_Tp>) {
diff --git a/libcxx/include/locale b/libcxx/include/locale
index 4f2716fa53d62..801412bdd4c44 100644
--- a/libcxx/include/locale
+++ b/libcxx/include/locale
@@ -3110,7 +3110,7 @@ public:
   }
   _LIBCPP_HIDE_FROM_ABI wide_string from_bytes(const char* __first, const char* __last);
 
-  _LIBCPP_HIDE_FROM_ABI byte_string to_bytes(_Elem __wchar) { return to_bytes(&__wchar, &__wchar + 1); }
+  _LIBCPP_HIDE_FROM_ABI byte_string to_bytes(_Elem __wchar) { return to_bytes(std::addressof(__wchar), std::addressof(__wchar) + 1); }
   _LIBCPP_HIDE_FROM_ABI byte_string to_bytes(const _Elem* __wptr) {
     return to_bytes(__wptr, __wptr + char_traits<_Elem>::length(__wptr));
   }
@@ -3172,7 +3172,7 @@ wstring_convert<_Codecvt, _Elem, _WideAlloc, _ByteAlloc>::from_bytes(const char*
     codecvt_base::result __r = codecvt_base::ok;
     state_type __st          = __cvtstate_;
     if (__frm != __frm_end) {
-      _Elem* __to     = &__ws[0];
+      _Elem* __to     = std::addressof(__ws[0]);
       _Elem* __to_end = __to + __ws.size();
       const char* __frm_nxt;
       do {
@@ -3182,19 +3182,19 @@ wstring_convert<_Codecvt, _Elem, _WideAlloc, _ByteAlloc>::from_bytes(const char*
         if (__frm_nxt == __frm) {
           __r = codecvt_base::error;
         } else if (__r == codecvt_base::noconv) {
-          __ws.resize(__to - &__ws[0]);
+          __ws.resize(__to - std::addressof(__ws[0]));
           // This only gets executed if _Elem is char
           __ws.append((const _Elem*)__frm, (const _Elem*)__frm_end);
           __frm = __frm_nxt;
           __r   = codecvt_base::ok;
         } else if (__r == codecvt_base::ok) {
-          __ws.resize(__to_nxt - &__ws[0]);
+          __ws.resize(__to_nxt - std::addressof(__ws[0]));
           __frm = __frm_nxt;
         } else if (__r == codecvt_base::partial) {
-          ptrdiff_t __s = __to_nxt - &__ws[0];
+          ptrdiff_t __s = __to_nxt - std::addressof(__ws[0]);
           __ws.resize(2 * __s);
-          __to     = &__ws[0] + __s;
-          __to_end = &__ws[0] + __ws.size();
+          __to     = std::addressof(__ws[0]) + __s;
+          __to_end = std::addressof(__ws[0]) + __ws.size();
           __frm    = __frm_nxt;
         }
       } while (__r == codecvt_base::partial && __frm_nxt < __frm_end);
@@ -3220,7 +3220,7 @@ wstring_convert<_Codecvt, _Elem, _WideAlloc, _ByteAlloc>::to_bytes(const _Elem*
     codecvt_base::result __r = codecvt_base::ok;
     state_type __st          = __cvtstate_;
     if (__frm != __frm_end) {
-      char* __to     = &__bs[0];
+      char* __to     = std::addressof(__bs[0]);
       char* __to_end = __to + __bs.size();
       const _Elem* __frm_nxt;
       do {
@@ -3230,19 +3230,19 @@ wstring_convert<_Codecvt, _Elem, _WideAlloc, _ByteAlloc>::to_bytes(const _Elem*
         if (__frm_nxt == __frm) {
           __r = codecvt_base::error;
         } else if (__r == codecvt_base::noconv) {
-          __bs.resize(__to - &__bs[0]);
+          __bs.resize(__to - std::addressof(__bs[0]));
           // This only gets executed if _Elem is char
           __bs.append((const char*)__frm, (const char*)__frm_end);
           __frm = __frm_nxt;
           __r   = codecvt_base::ok;
         } else if (__r == codecvt_base::ok) {
-          __bs.resize(__to_nxt - &__bs[0]);
+          __bs.resize(__to_nxt - std::addressof(__bs[0]));
           __frm = __frm_nxt;
         } else if (__r == codecvt_base::partial) {
-          ptrdiff_t __s = __to_nxt - &__bs[0];
+          ptrdiff_t __s = __to_nxt - std::addressof(__bs[0]);
           __bs.resize(2 * __s);
-          __to     = &__bs[0] + __s;
-          __to_end = &__bs[0] + __bs.size();
+          __to     = std::addressof(__bs[0]) + __s;
+          __to_end = std::addressof(__bs[0]) + __bs.size();
           __frm    = __frm_nxt;
         }
       } while (__r == codecvt_base::partial && __frm_nxt < __frm_end);
@@ -3250,21 +3250,21 @@ wstring_convert<_Codecvt, _Elem, _WideAlloc, _ByteAlloc>::to_bytes(const _Elem*
     if (__r == codecvt_base::ok) {
       size_t __s = __bs.size();
       __bs.resize(__bs.capacity());
-      char* __to     = &__bs[0] + __s;
+      char* __to     = std::addressof(__bs[0]) + __s;
       char* __to_end = __to + __bs.size();
       do {
         char* __to_nxt;
         __r = __cvtptr_->unshift(__st, __to, __to_end, __to_nxt);
         if (__r == codecvt_base::noconv) {
-          __bs.resize(__to - &__bs[0]);
+          __bs.resize(__to - std::addressof(__bs[0]));
           __r = codecvt_base::ok;
         } else if (__r == codecvt_base::ok) {
-          __bs.resize(__to_nxt - &__bs[0]);
+          __bs.resize(__to_nxt - std::addressof(__bs[0]));
         } else if (__r == codecvt_base::partial) {
-          ptrdiff_t __sp = __to_nxt - &__bs[0];
+          ptrdiff_t __sp = __to_nxt - std::addressof(__bs[0]);
           __bs.resize(2 * __sp);
-          __to     = &__bs[0] + __sp;
-          __to_end = &__bs[0] + __bs.size();
+          __to     = std::addressof(__bs[0]) + __sp;
+          __to_end = std::addressof(__bs[0]) + __bs.size();
         }
       } while (__r == codecvt_base::partial);
       if (__r == codecvt_base::ok)
@@ -3383,7 +3383,7 @@ typename wbuffer_convert<_Codecvt, _Elem, _Tr>::int_type wbuffer_convert<_Codecv
   bool __initial = __read_mode();
   char_type __1buf;
   if (this->gptr() == 0)
-    this->setg(&__1buf, &__1buf + 1, &__1buf + 1);
+    this->setg(std::addressof(__1buf), std::addressof(__1buf) + 1, std::addressof(__1buf) + 1);
   const size_t __unget_sz = __initial ? 0 : std::min<size_t>((this->egptr() - this->eback()) / 2, 4);
   int_type __c            = traits_type::eof();
   if (this->gptr() == this->egptr()) {
@@ -3425,7 +3425,7 @@ typename wbuffer_convert<_Codecvt, _Elem, _Tr>::int_type wbuffer_convert<_Codecv
     }
   } else
     __c = *this->gptr();
-  if (this->eback() == &__1buf)
+  if (this->eback() == std::addressof(__1buf))
     this->setg(0, 0, 0);
   return __c;
 }
@@ -3461,7 +3461,7 @@ typename wbuffer_convert<_Codecvt, _Elem, _Tr>::int_type wbuffer_convert<_Codecv
   char_type* __epb_save = this->epptr();
   if (!traits_type::eq_int_type(__c, traits_type::eof())) {
     if (this->pptr() == 0)
-      this->setp(&__1buf, &__1buf + 1);
+      this->setp(std::addressof(__1buf), std::addressof(__1buf) + 1);
     *this->pptr() = traits_type::to_char_type(__c);
     this->pbump(1);
   }

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff be542610074b4efb39c3364d0fd2ca82ea1deb1e 687de6398d607fd9da7f9f20cdca4a2fee7dd78d --extensions ,h -- libcxx/include/__functional/function.h libcxx/include/__numeric/gcd_lcm.h libcxx/include/__numeric/saturation_arithmetic.h libcxx/include/locale

View the diff from clang-format here.

diff --git a/libcxx/include/locale b/libcxx/include/locale
index 801412bdd4..5b1f2937c3 100644
--- a/libcxx/include/locale
+++ b/libcxx/include/locale
@@ -3110,7 +3110,9 @@ public:
   }
   _LIBCPP_HIDE_FROM_ABI wide_string from_bytes(const char* __first, const char* __last);
 
-  _LIBCPP_HIDE_FROM_ABI byte_string to_bytes(_Elem __wchar) { return to_bytes(std::addressof(__wchar), std::addressof(__wchar) + 1); }
+  _LIBCPP_HIDE_FROM_ABI byte_string to_bytes(_Elem __wchar) {
+    return to_bytes(std::addressof(__wchar), std::addressof(__wchar) + 1);
+  }
   _LIBCPP_HIDE_FROM_ABI byte_string to_bytes(const _Elem* __wptr) {
     return to_bytes(__wptr, __wptr + char_traits<_Elem>::length(__wptr));
   }

[philnik777]

AFAICT every single change here is for a case where we control the type. Why do you think we should do this change?

[mordante]

AFAICT every single change here is for a case where we control the type. Why do you think we should do this change?

That's correct, this is needed so #128366 will run cleanly.
In the past I already did quite some work on this patch, at that time I found several real positives that have since been fixed. So this patch seems underwhelming, but this check has real value.