On 25/10/18 13:36 +0100, Jonathan Wakely wrote:
On 25/10/18 14:30 +0200, Marc Glisse wrote:
On Tue, 23 Oct 2018, Jonathan Wakely wrote:
+ template<typename _Tp, typename _Up, typename _Allocator>
+ inline void
+ __relocate_a(_Tp* __dest, _Up* __orig, _Allocator& __alloc)
I find it a little surprising that this overload for single objects
using the memmove argument ordering (dest, source) but the range overload
below uses the STL ordering (source_begin, source_end, dest).
But I wouldn't be surprised if we're already doing that somewhere that
I've forgotten about.
WOuld it make sense to either rename this overload, or to use
consistent argument ordering for the two __relocate_a overloads?
The functions were not meant as overloads, it just happened that
I arrived at the same name for both, but it would make perfect
sense to give them different names. I started from
__relocate(dest, source) for one element, and later added an
allocator to it. The other one corresponds to
__uninitialized_move_a, and naming it __uninitialized_relocate_a
would be silly since "uninitialized" is included in the
definition of relocate.
Yes, my first thought was to add "uninitialized" and I rejected it for
that same reason.
I think I'd rather rename than change the order. Do you have
suggestions? __relocate_range_a?
I was thinking the single object one could be __relocate_1_a with the
_1 being like copy_n, fill_n etc. but that's going to be confusing
with __relocate_a_1 instead!
It seems unfortunate to have to put "range" in the name when no other
algos that work on ranges bother to say that in the name.
Maybe the single object one could be __relocate_single_a? Or
__do_relocate_a? __relocate_obj_a? None of them really makes me happy.
I went with __relocate_object_a, but that's easy to change.
I'm OK with that. If anybody thinks of a better name we can change it.
We may want to specialize / overload __relocate_object_a for some
specific types in the future, although we would more likely have
__relocate_object_a call __relocate_object (no allocator) when it
receives the default allocator, and specialize that one. And this
should anyway be less common that specializing
__is_trivially_relocatable.
I just realized that __reallocate_a and construct don't take the
allocator argument in the same position... I can switch if it helps.
No, leaving it at the end is consistent with __uninitialized_copy_a
et al, and it's closer in spirit to them.
OK for trunk, thanks!
Here's the fix for the regression this introduced.
Tested x86_64-linux, committed to trunk.
commit caf2adcb563d7dd62c3de7bb49f528753f958ba3
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Fri Nov 9 19:19:16 2018 +0000
PR libstdc++/87787 fix UBsan error in std::vector
PR libstdc++/87787
* include/bits/stl_uninitialized.h (__relocate_a_1): Do not call
memmove when there's nothing to copy (and pointers could be null).
diff --git a/libstdc++-v3/include/bits/stl_uninitialized.h b/libstdc++-v3/include/bits/stl_uninitialized.h
index 94c7e151e29..8839bfdcc90 100644
--- a/libstdc++-v3/include/bits/stl_uninitialized.h
+++ b/libstdc++-v3/include/bits/stl_uninitialized.h
@@ -904,7 +904,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_Tp* __result, allocator<_Up>& __alloc)
{
ptrdiff_t __count = __last - __first;
- __builtin_memmove(__result, __first, __count * sizeof(_Tp));
+ if (__count > 0)
+ __builtin_memmove(__result, __first, __count * sizeof(_Tp));
return __result + __count;
}
