On 08/05/20 17:22 -0300, Alexandre Oliva wrote:
Vxworks 7's malloc, like Solaris', only ensures 8-byte alignment of
returned pointers on 32-bit x86, though GCC's stddef.h defines
max_align_t with 16-byte alignment for __float128. This patch enables
on x86-vxworks the same memory_resource workaround used for x86-solaris.
The testsuite also had a workaround, defining BAD_MAX_ALIGN_T and
xfailing the test; extend those to x86-vxworks as well, and fix the
one test that could still fail on x86-vxworks, that expected
char-aligned requested allocation to be aligned for max_align_t. With
that change, the test passes on x86-vxworks; I'm guessing that's the
same reason for the test not to pass on x86-solaris (and on
x86_64-solaris -m32), so with the fix, I'm tentatively removing the
xfail.
Tested on x86_64-linux-gnu-x-i586-vxworks7.2. Ok to install?
Yes, but ...
(Couldn't r1->allocate(2, alignof(char)) possibly return a pointer
that's *not* aligned<max_align_t>? Maybe we should drop the test even
if !defined(BAD_MAX_ALIGN_T).)
Yes.
Different malloc implementations interpret the C standard differently
here. One interpretation is that all allocations must be aligned to
alignof(max_align_t) but another is that allocations smaller than that
don't need to meet that requirement. An object that is two bytes in
size cannot require 16-byte alignment (otherwise its sizeof would be
16 too).
Some mallocs behave the second way, so it doesn't really matter what
the correct interpretation of the C standard is, libstdc++ has to
accept reality and work with those mallocs. And so I think that test
is buggy and shouldn't assume small allocations will use
alignof(max_align_t).
Please do remove that line of the test, instead of wrapping it in the
#ifdef.
OK for master.
Also approved for gcc-10 branch if the target maintainers are OK with
it. It's an ABI change (allocations aligned to alignof(max_align_t)
will store the token in the allocation after this change) but the
header is an experimental TS feature so not guaranteed to be stable.
The target maintainers can decide if they'd rather have GCC 10.2 be
consistent with 10.1 or with master.
for libstdc++-v3/ChangeLog
PR libstdc++/77691
* include/experimental/memory_resource
(__resource_adaptor_imp::do_allocate): Handle max_align_t on
x86-vxworks as on x86-solaris.
(__resource_adaptor_imp::do_deallocate): Likewise.
* testsuite/experimental/memory_resource/new_delete_resource.cc:
Drop xfail.
(BAD_MAX_ALIGN_T): Define on x86-vxworks as on x86-solaris.
(test03): Skip align test for char-aligned alloc if
BAD_MAX_ALIGN_T.
---
libstdc++-v3/include/experimental/memory_resource | 4 ++--
.../memory_resource/new_delete_resource.cc | 5 +++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/libstdc++-v3/include/experimental/memory_resource
b/libstdc++-v3/include/experimental/memory_resource
index 850a78d..1c4de70 100644
--- a/libstdc++-v3/include/experimental/memory_resource
+++ b/libstdc++-v3/include/experimental/memory_resource
@@ -413,7 +413,7 @@ namespace pmr {
do_allocate(size_t __bytes, size_t __alignment) override
{
// Cannot use max_align_t on 32-bit Solaris x86, see PR libstdc++/77691
-#if ! (defined __sun__ && defined __i386__)
+#if ! ((defined __sun__ || defined __VXWORKS__) && defined __i386__)
if (__alignment == alignof(max_align_t))
return _M_allocate<alignof(max_align_t)>(__bytes);
#endif
@@ -439,7 +439,7 @@ namespace pmr {
do_deallocate(void* __ptr, size_t __bytes, size_t __alignment) noexcept
override
{
-#if ! (defined __sun__ && defined __i386__)
+#if ! ((defined __sun__ || defined __VXWORKS__) && defined __i386__)
if (__alignment == alignof(max_align_t))
return (void) _M_deallocate<alignof(max_align_t)>(__ptr, __bytes);
#endif
diff --git
a/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc
b/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc
index 8a98954..8119349 100644
--- a/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc
+++ b/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc
@@ -17,13 +17,12 @@
// { dg-do run { target c++14 } }
// { dg-require-cstdint "" }
-// { dg-xfail-run-if "PR libstdc++/77691" { { i?86-*-solaris2.* x86_64-*-solaris2.* }
&& ilp32 } }
#include <experimental/memory_resource>
#include <cstdlib>
#include <testsuite_hooks.h>
-#if defined __sun__ && defined __i386__
+#if (defined __sun__ || defined __VXWORKS__) && defined __i386__
// See PR libstdc++/77691
# define BAD_MAX_ALIGN_T 1
#endif
@@ -128,7 +127,9 @@ test03()
p = r1->allocate(2, alignof(char));
VERIFY( bytes_allocated == 2 );
+#ifndef BAD_MAX_ALIGN_T
VERIFY( aligned<max_align_t>(p) );
+#endif
r1->deallocate(p, 2, alignof(char));
VERIFY( bytes_allocated == 0 );
--
Alexandre Oliva, freedom fighter he/him https://FSFLA.org/blogs/lxo/
Free Software Evangelist Stallman was right, but he's left :(
GNU Toolchain Engineer Live long and free, and prosper ethically
