On 27/01/14 23:37 +0000, Jonathan Wakely wrote:
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.
Here's what I'm committing, the testcase is simplified by reusing the
new PointerBase type I added to testsuite_allocator.h
Tested x86_64-linux, committed to trunk.
commit 36805f8b1b6ef6733c4e018ef005efb1575b75c6
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Tue Jan 28 12:47:25 2014 +0000
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.
* testsuite/23_containers/vector/requirements/dr438/assign_neg.cc:
Adjust dg-error line number.
* testsuite/23_containers/vector/requirements/dr438/
constructor_1_neg.cc: Likewise.
* testsuite/23_containers/vector/requirements/dr438/
constructor_2_neg.cc: Likewise.
* testsuite/23_containers/vector/requirements/dr438/insert_neg.cc:
Likewise.
diff --git a/libstdc++-v3/include/bits/stl_vector.h
b/libstdc++-v3/include/bits/stl_vector.h
index 98ac708..7e52fde 100644
--- a/libstdc++-v3/include/bits/stl_vector.h
+++ b/libstdc++-v3/include/bits/stl_vector.h
@@ -888,7 +888,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*
@@ -896,7 +896,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
/**
@@ -1470,6 +1470,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
};
diff --git a/libstdc++-v3/testsuite/23_containers/vector/59829.cc
b/libstdc++-v3/testsuite/23_containers/vector/59829.cc
new file mode 100644
index 0000000..1818c89
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/vector/59829.cc
@@ -0,0 +1,67 @@
+// Copyright (C) 2014 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++11" }
+
+// libstdc++/59829
+
+#include <vector>
+#include <testsuite_allocator.h>
+
+// User-defined pointer type that throws if a null pointer is dereferenced.
+template<typename T>
+struct Pointer : __gnu_test::PointerBase<Pointer<T>, T>
+{
+ using __gnu_test::PointerBase<Pointer<T>, T>::PointerBase;
+
+ T& operator*() const
+ {
+ if (!this->value)
+ throw "Dereferenced invalid pointer";
+ return *this->value;
+ }
+};
+
+// Minimal allocator using Pointer<T>
+template<typename T>
+struct Alloc
+{
+ typedef T value_type;
+ typedef Pointer<T> pointer;
+
+ Alloc() = default;
+ template<typename U>
+ Alloc(const Alloc<U>&) { }
+
+ pointer allocate(std::size_t n)
+ { return pointer(std::allocator<T>().allocate(n)); }
+
+ void deallocate(pointer p, std::size_t n)
+ { std::allocator<T>().deallocate(p.value, n); }
+};
+
+template<typename T>
+bool operator==(Alloc<T> l, Alloc<T> r) { return true; }
+
+template<typename T>
+bool operator!=(Alloc<T> l, Alloc<T> r) { return false; }
+
+int main()
+{
+ std::vector<int, Alloc<int>> a;
+ a.data();
+}
diff --git
a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc
b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc
index a12b116..191fbc7 100644
---
a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc
+++
b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1316 }
+// { dg-error "no matching" "" { target *-*-* } 1320 }
#include <vector>
diff --git
a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc
b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc
index b839ccc..8818a88 100644
---
a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc
+++
b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1242 }
+// { dg-error "no matching" "" { target *-*-* } 1246 }
#include <vector>
diff --git
a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc
b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc
index e9e966b..09499bc 100644
---
a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc
+++
b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1242 }
+// { dg-error "no matching" "" { target *-*-* } 1246 }
#include <vector>
#include <utility>
diff --git
a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc
b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc
index 71c6c49..674e3b5 100644
---
a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc
+++
b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1357 }
+// { dg-error "no matching" "" { target *-*-* } 1361 }
#include <vector>
