Re: [patch] proposed fix for libstdc++/59829

[Jonathan Wakely]

On 27 January 2014 20:35, Jonathan Wakely wrote:
> On 27 January 2014 20:12, Marc Glisse wrote:
>> On Mon, 27 Jan 2014, Jonathan Wakely wrote:
>>
>>> This is the best I've come up with to avoid dereferencing an invalid
>>> pointer when calling vector::data() on an empty vector.
>>>
>>> For C++03 we reurn the vector's pointer type, so can just return the
>>> internal pointer, but for C++11 we need to convert that to a raw
>>> pointer, which we do by dereferencing, so we must check if it's valid
>>> first.
>>
>>
>> For comparison, libc++ has 2 paths. If pointer really is a pointer, it just
>> returns it, no need to pay for a comparison in that case. And otherwise, it
>> calls _M_start.operator-> and crosses its fingers. There is a helper
>> function doing that used throughout the library.
>
> Ah yes, I remember Howard posting a get_raw_pointer() function to the
> reflector that used operator->() on user-defined types ... I don't
> really like calling that on a potentially invalid pointer though. The
> user-defined pointer type in my new testcase could just as easily
> throw if operator-> is called on an invalid pointer.  As Paolo also
> mentioned avoiding the branch for built-in pointers I'll do that.

How about this? (the testcase remained the same as in the last patch)

        PR libstdc++/59829
        * include/bits/stl_vector.h (vector::data()): Call _M_data_ptr.
        (vector::_M_data_ptr): New overloaded functions to ensure empty
        vectors do not dereference the pointer.
        * testsuite/23_containers/vector/59829.cc: New.

diff --git a/libstdc++-v3/include/bits/stl_vector.h
b/libstdc++-v3/include/bits/stl_vector.h
index f482957..164a7d9 100644
--- a/libstdc++-v3/include/bits/stl_vector.h
+++ b/libstdc++-v3/include/bits/stl_vector.h
@@ -884,7 +884,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       pointer
 #endif
       data() _GLIBCXX_NOEXCEPT
-      { return std::__addressof(front()); }
+      { return _M_data_ptr(this->_M_impl._M_start); }

 #if __cplusplus >= 201103L
       const _Tp*
@@ -892,7 +892,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       const_pointer
 #endif
       data() const _GLIBCXX_NOEXCEPT
-      { return std::__addressof(front()); }
+      { return _M_data_ptr(this->_M_impl._M_start); }

       // [23.2.4.3] modifiers
       /**
@@ -1468,6 +1468,23 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
          }
       }
 #endif
+
+#if __cplusplus >= 201103L
+      template<typename _Up>
+       _Up*
+       _M_data_ptr(_Up* __ptr) const
+       { return __ptr; }
+
+      template<typename _Ptr>
+       typename std::pointer_traits<_Ptr>::element_type*
+       _M_data_ptr(_Ptr __ptr) const
+       { return empty() ? nullptr : std::__addressof(*__ptr); }
+#else
+      template<typename _Ptr>
+       _Ptr
+       _M_data_ptr(_Ptr __ptr) const
+       { return __ptr; }
+#endif
     };