[patch] libstdc++/65352 fix ubsan errors in std::arrayT, 0
Unsurprisingly ubsan doesn't like referencing a null pointer.
With this change __array_traits::_S_ref is only used to access an
element, which is invalid for std::arrayT, 0 anyway.
Tested powerpc64le-linux, committed to trunk.
commit 0d999cf16b8f6a0d9bbf4bfe96b29e7b73a259e4
Author: Jonathan Wakely jwak...@redhat.com
Date: Thu May 28 12:21:36 2015 +0100
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.
diff --git a/libstdc++-v3/include/std/array b/libstdc++-v3/include/std/array
index 429506b..24be44f 100644
--- a/libstdc++-v3/include/std/array
+++ b/libstdc++-v3/include/std/array
@@ -51,6 +51,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
static constexpr _Tp
_S_ref(const _Type __t, std::size_t __n) noexcept
{ return const_cast_Tp(__t[__n]); }
+
+ static constexpr _Tp*
+ _S_ptr(const _Type __t) noexcept
+ { return const_cast_Tp*(__t); }
};
templatetypename _Tp
@@ -61,6 +65,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
static constexpr _Tp
_S_ref(const _Type, std::size_t) noexcept
{ return *static_cast_Tp*(nullptr); }
+
+ static constexpr _Tp*
+ _S_ptr(const _Type) noexcept
+ { return nullptr; }
};
/**
@@ -219,11 +227,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
pointer
data() noexcept
- { return std::__addressof(_AT_Type::_S_ref(_M_elems, 0)); }
+ { return _AT_Type::_S_ptr(_M_elems); }
const_pointer
data() const noexcept
- { return std::__addressof(_AT_Type::_S_ref(_M_elems, 0)); }
+ { return _AT_Type::_S_ptr(_M_elems); }
};
// Array comparisons.
diff --git a/libstdc++-v3/testsuite/23_containers/array/tuple_interface/get_neg.cc b/libstdc++-v3/testsuite/23_containers/array/tuple_interface/get_neg.cc
index 7604412..6830964 100644
--- a/libstdc++-v3/testsuite/23_containers/array/tuple_interface/get_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/array/tuple_interface/get_neg.cc
@@ -28,6 +28,6 @@ int n1 = std::get1(a);
int n2 = std::get1(std::move(a));
int n3 = std::get1(ca);
-// { dg-error static assertion failed { target *-*-* } 274 }
-// { dg-error static assertion failed { target *-*-* } 283 }
+// { dg-error static assertion failed { target *-*-* } 282 }
// { dg-error static assertion failed { target *-*-* } 291 }
+// { dg-error static assertion failed { target *-*-* } 299 }
diff --git a/libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_element_neg.cc b/libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_element_neg.cc
index 9788053..5d75366 100644
--- a/libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_element_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_element_neg.cc
@@ -23,4 +23,4 @@
typedef std::tuple_element1, std::arrayint, 1::type type;
-// { dg-error static assertion failed { target *-*-* } 322 }
+// { dg-error static assertion failed { target *-*-* } 330 }
Re: [patch] libstdc++/65352 fix ubsan errors in std::arrayT, 0
On 28/05/15 12:53 +0100, Jonathan Wakely wrote: Unsurprisingly ubsan doesn't like referencing a null pointer. With this change __array_traits::_S_ref is only used to access an element, which is invalid for std::arrayT, 0 anyway. Tested powerpc64le-linux, committed to trunk. And gcc-5-branch. commit 0d999cf16b8f6a0d9bbf4bfe96b29e7b73a259e4 Author: Jonathan Wakely jwak...@redhat.com Date: Thu May 28 12:21:36 2015 +0100 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.
Re: [patch] libstdc++/65352 fix ubsan errors in std::arrayT, 0
On Thu, 28 May 2015, Jonathan Wakely wrote: Unsurprisingly ubsan doesn't like referencing a null pointer. With this change __array_traits::_S_ref is only used to access an element, which is invalid for std::arrayT, 0 anyway. Should return *static_cast_Tp*(nullptr); be replaced with __builtin_unreachable(); then? It seems strange to keep an implementation that is never supposed to be used. -- Marc Glisse
Re: [patch] libstdc++/65352 fix ubsan errors in std::arrayT, 0
On 28/05/15 14:38 +0100, Jonathan Wakely wrote: On 28/05/15 15:26 +0200, Marc Glisse wrote: On Thu, 28 May 2015, Jonathan Wakely wrote: Unsurprisingly ubsan doesn't like referencing a null pointer. With this change __array_traits::_S_ref is only used to access an element, which is invalid for std::arrayT, 0 anyway. Should return *static_cast_Tp*(nullptr); be replaced with __builtin_unreachable(); then? It seems strange to keep an implementation that is never supposed to be used. That's a good idea, I experimented with just not defining it but that fails for explicit instantiations of arrayT, 0. Would there be a danger of an object compiled with gcc-5.1 that calls arrayT, 0::data() finding the _S_ref from an object compiled with gcc-5.2 and hitting the __builtin_unreachable in vali code?
Re: [patch] libstdc++/65352 fix ubsan errors in std::arrayT, 0
On 28/05/15 15:26 +0200, Marc Glisse wrote: On Thu, 28 May 2015, Jonathan Wakely wrote: Unsurprisingly ubsan doesn't like referencing a null pointer. With this change __array_traits::_S_ref is only used to access an element, which is invalid for std::arrayT, 0 anyway. Should return *static_cast_Tp*(nullptr); be replaced with __builtin_unreachable(); then? It seems strange to keep an implementation that is never supposed to be used. That's a good idea, I experimented with just not defining it but that fails for explicit instantiations of arrayT, 0.
Re: [patch] libstdc++/65352 fix ubsan errors in std::arrayT, 0
On 28/05/15 15:52 +0200, Marc Glisse wrote:
On Thu, 28 May 2015, Jonathan Wakely wrote:
Would there be a danger of an object compiled with gcc-5.1 that calls
arrayT, 0::data() finding the _S_ref from an object compiled with
gcc-5.2 and hitting the __builtin_unreachable in vali code?
At -O0, maybe. To be safe you would need to give this _S_ref an
arbitrary abi_tag.
Or just rename it.
You could also replace all uses of _S_ref with *_S_ptr.
I considered this, but I thought that changing _S_ref(_M_elems, n) to
_S_ptr(_M_elems)[n] would fail to give an error for out-of-range
accesses in constant expressions e.g.
constexpr std::arrayint, 1 a{};
constexpr int i = a[1];
But it still seems to give an error, so maybe getting rid of _S_ref
entirely is the way to go.
Re: [patch] libstdc++/65352 fix ubsan errors in std::arrayT, 0
On Thu, 28 May 2015, Jonathan Wakely wrote: Would there be a danger of an object compiled with gcc-5.1 that calls arrayT, 0::data() finding the _S_ref from an object compiled with gcc-5.2 and hitting the __builtin_unreachable in vali code? At -O0, maybe. To be safe you would need to give this _S_ref an arbitrary abi_tag. You could also replace all uses of _S_ref with *_S_ptr. -- Marc Glisse
Re: [patch] libstdc++/65352 fix ubsan errors in std::arrayT, 0
On 28/05/15 12:53 +0100, Jonathan Wakely wrote:
Unsurprisingly ubsan doesn't like referencing a null pointer.
With this change __array_traits::_S_ref is only used to access an
element, which is invalid for std::arrayT, 0 anyway.
Tested powerpc64le-linux, committed to trunk.
I forgot the debug and profile modes, fixed like so.
1) Why do we even have _profile::array? What's it for?
2) If we could run 'make check-sanitize' I could have added tests for
this bug, and could have found it still failed in debug and profile
modes. We need to be able to run the testsuite with ubsan.
I'll commit it to trunk and gcc-5-branch after testing.
commit 7a673c403d77fb2c57620f5e4f027b679bf69635
Author: Jonathan Wakely jwak...@redhat.com
Date: Thu May 28 15:35:43 2015 +0100
PR libstdc++/65352
* include/profile/array (array::data): Use _S_ptr.
* include/debug/array (array::data): Likewise.
diff --git a/libstdc++-v3/include/debug/array b/libstdc++-v3/include/debug/array
index 31d146e..411e816 100644
--- a/libstdc++-v3/include/debug/array
+++ b/libstdc++-v3/include/debug/array
@@ -216,11 +216,11 @@ namespace __debug
pointer
data() noexcept
- { return std::__addressof(_AT_Type::_S_ref(_M_elems, 0)); }
+ { return _AT_Type::_S_ptr(_M_elems); }
const_pointer
data() const noexcept
- { return std::__addressof(_AT_Type::_S_ref(_M_elems, 0)); }
+ { return _AT_Type::_S_ptr(_M_elems); }
};
// Array comparisons.
diff --git a/libstdc++-v3/include/profile/array b/libstdc++-v3/include/profile/array
index a90e396..5198bb3 100644
--- a/libstdc++-v3/include/profile/array
+++ b/libstdc++-v3/include/profile/array
@@ -178,11 +178,11 @@ namespace __profile
pointer
data() noexcept
- { return std::__addressof(_AT_Type::_S_ref(_M_elems, 0)); }
+ { return _AT_Type::_S_ptr(_M_elems); }
const_pointer
data() const noexcept
- { return std::__addressof(_AT_Type::_S_ref(_M_elems, 0)); }
+ { return _AT_Type::_S_ptr(_M_elems); }
};
// Array comparisons.
