[Bug libstdc++/65352] New: array<T,0>::begin()/end() etc. forms a null reference and breaks on clang+ubsan

[Bug libstdc++/65352] New: array<T,0>::begin()/end() etc. forms a null reference and breaks on clang+ubsan

[rs2740 at gmail dot com]

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

            Bug ID: 65352
           Summary: array<T,0>::begin()/end() etc. forms a null reference
                    and breaks on clang+ubsan
           Product: gcc
           Version: 4.9.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rs2740 at gmail dot com

Repro:

#include <array>
int main(){
    std::array<int, 0> foo;
    foo.begin(); // or end(), etc.
}

Output (http://coliru.stacked-crooked.com/a/e1cbe7e73bcee449):

> clang++ --version
clang version 3.5.0 (tags/RELEASE_350/final 217394)
Target: x86_64-unknown-linux-gnu
Thread model: posix
> clang++ -std=c++11 -O0 -Wall -pedantic -pthread main.cpp -fsanitize=undefined
> ./a.out
==15356==WARNING: readlink("/proc/self/exe") failed with errno 2, some stack
frames may not be symbolized
/usr/local/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.9.2/../../../../include/c++/4.9.2/array:63:15:
runtime error: reference binding to null pointer of type 'int'
/usr/local/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.9.2/../../../../include/c++/4.9.2/array:222:33:
runtime error: reference binding to null pointer of type 'int'

begin()/end() are supposed to be well-defined even if N = 0. The implementation
in <array> defers to data(), which in turn returns
std::__addressof(_AT_Type::_S_ref(_M_elems, 0)). The problem is that for the N
= 0 case, __array_traits::_S_ref forms and returns a null reference:

     static constexpr _Tp&
     _S_ref(const _Type&, std::size_t) noexcept
     { return *static_cast<_Tp*>(nullptr); }

An obvious possible fix is to provide a pointer-returning helper in addition to
or instead of _S_ref.

[Bug libstdc++/65352] array<T,0>::begin()/end() etc. forms a null reference and breaks on clang+ubsan

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-03-08
     Ever confirmed|0                           |1

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Or just (untested):

--- a/libstdc++-v3/include/std/array
+++ b/libstdc++-v3/include/std/array
@@ -58,9 +58,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
    {
      struct _Type { };

-     static constexpr _Tp&
+     static _Tp&
      _S_ref(const _Type&, std::size_t) noexcept
-     { return *static_cast<_Tp*>(nullptr); }
+     { return reinterpret_cast<_Tp&>(const_cast<_Type&>(t)); }
    };

   /**

It's undefined to refer to the element in the zero-size case, so casting to an
incompatible reference type shouldn't matter as noone will ever access anything
through that reference.

[Bug libstdc++/65352] array<T,0>::begin()/end() etc. forms a null reference and breaks on clang+ubsan

[redi at gcc dot gnu.org]

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

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Oops, that should be:

--- a/libstdc++-v3/include/std/array
+++ b/libstdc++-v3/include/std/array
@@ -58,9 +58,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
    {
      struct _Type { };

-     static constexpr _Tp&
-     _S_ref(const _Type&, std::size_t) noexcept
-     { return *static_cast<_Tp*>(nullptr); }
+     static _Tp&
+     _S_ref(const _Type& __t, std::size_t) noexcept
+     { return reinterpret_cast<_Tp&>(const_cast<_Type&>(__t)); }
    };

   /**

I think it's OK to make it non-constexpr, because _S_ref only needs to be
constexpr for functions which access an element, which are also invalid for the
zero-size array.

[Bug libstdc++/65352] array<T,0>::begin()/end() etc. forms a null reference and breaks on clang+ubsan

[rs2740 at gmail dot com]

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

--- Comment #3 from TC <rs2740 at gmail dot com> ---
Depends on how http://cplusplus.github.io/LWG/lwg-active.html#2443 comes out in
LEWG, it might be a good idea to go with a solution that maintains the
`constexpr`-ness so that it doesn't have to be redone again if the committee
ends up making, say, begin() and end() constexpr.

Of course, you are a lot more familiar with the committee's workings than I do
:)

[Bug libstdc++/65352] array<T,0>::begin()/end() etc. forms a null reference and breaks on clang+ubsan

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tnozicka at gmail dot com

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
*** Bug 66323 has been marked as a duplicate of this bug. ***

[Bug libstdc++/65352] array<T,0>::begin()/end() etc. forms a null reference and breaks on clang+ubsan

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Author: redi
Date: Thu May 28 12:33:41 2015
New Revision: 223812

URL: https://gcc.gnu.org/viewcvs?rev=223812&root=gcc&view=rev
Log:
        PR libstdc++/65352
        * include/std/array (__array_traits::_S_ptr): New function.
        (array::data): Use _S_ptr to avoid creating invalid reference.
        * testsuite/23_containers/array/tuple_interface/get_neg.cc: Adjust
        dg-error line numbers.
        * testsuite/23_containers/array/tuple_interface/tuple_element_neg.cc:
        likewise.

Modified:
    branches/gcc-5-branch/libstdc++-v3/ChangeLog
    branches/gcc-5-branch/libstdc++-v3/include/std/array
   
branches/gcc-5-branch/libstdc++-v3/testsuite/23_containers/array/tuple_interface/get_neg.cc
   
branches/gcc-5-branch/libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_element_neg.cc

[Bug libstdc++/65352] array<T,0>::begin()/end() etc. forms a null reference and breaks on clang+ubsan

[redi at gcc dot gnu.org]

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

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Author: redi
Date: Thu May 28 16:00:09 2015
New Revision: 223838

URL: https://gcc.gnu.org/viewcvs?rev=223838&root=gcc&view=rev
Log:
        PR libstdc++/65352
        * include/profile/array (array::data): Use _S_ptr.
        * include/debug/array (array::data): Likewise.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/debug/array
    trunk/libstdc++-v3/include/profile/array

[Bug libstdc++/65352] array<T,0>::begin()/end() etc. forms a null reference and breaks on clang+ubsan

[redi at gcc dot gnu.org]

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

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Author: redi
Date: Thu May 28 16:27:56 2015
New Revision: 223842

URL: https://gcc.gnu.org/viewcvs?rev=223842&root=gcc&view=rev
Log:
        PR libstdc++/65352
        * include/profile/array (array::data): Use __array_traits::_S_ptr.
        * include/debug/array (array::data): Likewise.
        * include/std/array (__array_traits::_S_ptr): New function.
        (array::data): Use _S_ptr to avoid creating invalid reference.
        * testsuite/23_containers/array/tuple_interface/get_neg.cc: Adjust
        dg-error line numbers.
        * testsuite/23_containers/array/tuple_interface/tuple_element_neg.cc:
        likewise.

Modified:
    branches/gcc-4_9-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_9-branch/libstdc++-v3/include/debug/array
    branches/gcc-4_9-branch/libstdc++-v3/include/profile/array
    branches/gcc-4_9-branch/libstdc++-v3/include/std/array
   
branches/gcc-4_9-branch/libstdc++-v3/testsuite/23_containers/array/tuple_interface/get_neg.cc
   
branches/gcc-4_9-branch/libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_element_neg.cc

[Bug libstdc++/65352] array<T,0>::begin()/end() etc. forms a null reference and breaks on clang+ubsan

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |4.9.3

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed for 4.9.3, 5.2 and 6.0