[Bug libstdc++/101571] New: DestroyGuard used by the ranges::uninitialized family should use addressof()

[Bug libstdc++/101571] New: DestroyGuard used by the ranges::uninitialized family should use addressof()

[hewillk at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101571

            Bug ID: 101571
           Summary: DestroyGuard used by the ranges::uninitialized family
                    should use addressof()
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: hewillk at gmail dot com
  Target Milestone: ---

The Standard Library should consistently use addressof() to defend itself
against overloaded operator&().

#include <memory>

struct I {
  using value_type = std::string;
  using difference_type = std::ptrdiff_t;

  I& operator++();
  I operator++(int);
  value_type& operator*() const;
  void operator&() = delete;
  bool operator==(const I&) const;
};

int main() {
  std::ranges::uninitialized_default_construct(I{}, I{});
}

https://godbolt.org/z/5Pb67b9jx

[Bug libstdc++/101571] DestroyGuard used by the ranges::uninitialized family should use addressof()

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101571

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-07-22

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Rather than add __addressof everywhere, we could change the constructor to take
a reference and take the address in the constructor:

--- a/libstdc++-v3/include/bits/ranges_uninitialized.h
+++ b/libstdc++-v3/include/bits/ranges_uninitialized.h
@@ -106,8 +106,8 @@ namespace ranges

       public:
        explicit
-       _DestroyGuard(const _Iter* __iter)
-         : _M_first(*__iter), _M_cur(__iter)
+       _DestroyGuard(const _Iter& __iter)
+         : _M_first(__iter), _M_cur(std::__addressof(__iter))
        { }

        void
@@ -149,7 +149,7 @@ namespace ranges
          return ranges::next(__first, __last);
        else
          {
-           auto __guard = __detail::_DestroyGuard(&__first);
+           auto __guard = __detail::_DestroyGuard(__first);
            for (; __first != __last; ++__first)
              ::new (__detail::__voidify(*__first)) _ValueType;
            __guard.release();

and so on for each use of it.

But I also have a patch to just do this everywhere:

--- a/libstdc++-v3/include/bits/ranges_uninitialized.h
+++ b/libstdc++-v3/include/bits/ranges_uninitialized.h
@@ -149,7 +149,7 @@ namespace ranges
          return ranges::next(__first, __last);
        else
          {
-           auto __guard = __detail::_DestroyGuard(&__first);
+           auto __guard = __detail::_DestroyGuard(std::__addressof(__first));
            for (; __first != __last; ++__first)
              ::new (__detail::__voidify(*__first)) _ValueType;
            __guard.release();

[Bug libstdc++/101571] DestroyGuard used by the ranges::uninitialized family should use addressof()

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101571

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
We should also do this:

--- a/libstdc++-v3/testsuite/util/testsuite_iterators.h
+++ b/libstdc++-v3/testsuite/util/testsuite_iterators.h
@@ -175,10 +175,14 @@ namespace __gnu_test
 #if __cplusplus >= 201103L
     template<typename U>
       void operator,(const U&) const = delete;
+
+    void operator&() const = delete;
 #else
   private:
     template<typename U>
       void operator,(const U&) const;
+
+    void operator&() const;
 #endif
   };

@@ -288,10 +292,14 @@ namespace __gnu_test
 #if __cplusplus >= 201103L
     template<typename U>
       void operator,(const U&) const = delete;
+
+    void operator&() const = delete;
 #else
   private:
     template<typename U>
       void operator,(const U&) const;
+
+    void operator&() const;
 #endif
   };

[Bug libstdc++/101571] DestroyGuard used by the ranges::uninitialized family should use addressof()

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101571

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:aca7a0253d6e3116f846ad530b19d89644a64267

commit r12-2469-gaca7a0253d6e3116f846ad530b19d89644a64267
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jul 22 14:37:24 2021 +0100

    libstdc++: Use std::addressof in ranges::uninitialized_xxx [PR101571]

    Make the ranges::uninitialized_xxx algorithms use std::addressof to
    protect against iterator types that overload operator&.

    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

    libstdc++-v3/ChangeLog:

            PR libstdc++/101571
            * include/bits/ranges_uninitialized.h (_DestroyGuard): Change
            constructor parameter to reference and use addressof.
            * testsuite/util/testsuite_iterators.h: Define deleted operator&
            overloads for test iterators.

[Bug libstdc++/101571] DestroyGuard used by the ranges::uninitialized family should use addressof()

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101571

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
   Target Milestone|---                         |10.4
             Status|NEW                         |ASSIGNED

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed on trunk only so far.

[Bug libstdc++/101571] DestroyGuard used by the ranges::uninitialized family should use addressof()

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101571

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:667339efd8ee10079aac9684c5d1c9c9b28d9da6

commit r11-9268-g667339efd8ee10079aac9684c5d1c9c9b28d9da6
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jul 22 14:37:24 2021 +0100

    libstdc++: Use std::addressof in ranges::uninitialized_xxx [PR101571]

    Make the ranges::uninitialized_xxx algorithms use std::addressof to
    protect against iterator types that overload operator&.

    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

    libstdc++-v3/ChangeLog:

            PR libstdc++/101571
            * include/bits/ranges_uninitialized.h (_DestroyGuard): Change
            constructor parameter to reference and use addressof.
            * testsuite/util/testsuite_iterators.h: Define deleted operator&
            overloads for test iterators.

    (cherry picked from commit aca7a0253d6e3116f846ad530b19d89644a64267)

[Bug libstdc++/101571] DestroyGuard used by the ranges::uninitialized family should use addressof()

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101571

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:0d480b8403f2541402adeed82deb7eb028330b87

commit r10-10310-g0d480b8403f2541402adeed82deb7eb028330b87
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jul 22 14:37:24 2021 +0100

    libstdc++: Use std::addressof in ranges::uninitialized_xxx [PR101571]

    Make the ranges::uninitialized_xxx algorithms use std::addressof to
    protect against iterator types that overload operator&.

    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

    libstdc++-v3/ChangeLog:

            PR libstdc++/101571
            * include/bits/ranges_uninitialized.h (_DestroyGuard): Change
            constructor parameter to reference and use addressof.
            * testsuite/util/testsuite_iterators.h: Define deleted operator&
            overloads for test iterators.

    (cherry picked from commit aca7a0253d6e3116f846ad530b19d89644a64267)

[Bug libstdc++/101571] DestroyGuard used by the ranges::uninitialized family should use addressof()

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101571

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed for 11.3 and 10.4, thanks for the report