Bug 77691 - [12/13/14/15 regression] experimental/memory_resource/resource_adaptor.cc FAILs
Summary: [12/13/14/15 regression] experimental/memory_resource/resource_adaptor.cc FAILs
Status: ASSIGNED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 7.0
: P4 normal
Target Milestone: 12.5
Assignee: Rainer Orth
URL: https://gcc.gnu.org/ml/gcc-patches/20...
Keywords: testsuite-fail
: 89732 (view as bug list)
Depends on: 90569
Blocks:
  Show dependency treegraph
 
Reported: 2016-09-22 14:11 UTC by Rainer Orth
Modified: 2024-07-19 12:59 UTC (History)
6 users (show)

See Also:
Host: i386-pc-solaris2.12
Target: i386-pc-solaris2.12
Build: i386-pc-solaris2.12
Known to work:
Known to fail:
Last reconfirmed: 2023-01-05 00:00:00


Attachments
test program (152 bytes, text/plain)
2017-06-07 14:20 UTC, Rainer Orth
Details
Patch to fix resource_adaptor failures due to max_align_t bugs (1.94 KB, patch)
2019-03-11 17:18 UTC, Jonathan Wakely
Details | Diff
Patch to fix test failure on hppa64-hp-hpux11.11 (299 bytes, patch)
2023-01-08 23:10 UTC, John David Anglin
Details | Diff
Generalise special case for malloc not afreeing with max_align_t (572 bytes, patch)
2023-01-09 11:20 UTC, Jonathan Wakely
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rainer Orth 2016-09-22 14:11:51 UTC
Between 20160916 and 20160922, experimental/memory_resource/resource_adaptor.cc 
started to FAIL on 32-bit Solaris 12/x86 (not yet tried on S10 or S11), S12/SPARC
is fine:

FAIL: experimental/memory_resource/resource_adaptor.cc execution test

Assertion failed: aligned<max_align_t>(p), file /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/experimental/memory_resource/resource_adaptor.cc, line 56, function test05

The failure only happens at -O1 and above.

  Rainer
Comment 1 Jonathan Wakely 2016-09-22 14:23:25 UTC
I think that means malloc is not returning memory suitably aligned for max_align_t.
Comment 2 Jonathan Wakely 2016-09-22 14:24:50 UTC
Oh, except that it might now be using aligned_alloc (or posix_memalign) instead of malloc.
Comment 3 Jonathan Wakely 2016-09-22 14:36:03 UTC
Presumably caused by r240187 or r240192
Comment 4 ro@CeBiTec.Uni-Bielefeld.DE 2016-09-22 14:40:06 UTC
> --- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> Presumably caused by r240187 or r240192

I think I had r240187 (or a slightly earlier variant thereof) in my
r240175 tree when bootstrapping on 20160916.

	Rainer
Comment 5 Jakub Jelinek 2017-05-02 15:56:25 UTC
GCC 7.1 has been released.
Comment 6 ro@CeBiTec.Uni-Bielefeld.DE 2017-06-07 14:18:08 UTC
I've digged a bit further now.  

Running the testcase under gdb, I find that p at the failing assertion
is 0x806fda8, i.e. not aligned to 16 bytes but to 8 bytes instead.  It's
ultimately returned from malloc in libsupc++/new_op.cc (operator new
(std::size_t sz)).

With test attached testcase and gcc 7.1.0, I find:

  alignof	long long	long double	double _Complex	std::max_align_t

  i386		 8   		 4   		 8     		16
  amd64		 8		16		 8		16
  sparc		 8		 8		 8		 8
  sparcv9	 8		16		 8		16

However, this changed from gcc 6 to 7:

  i386/gcc6	 8		 4		 8		 8
  i386/gcc7	 8		 4		 8		16

where the gcc6 value matches what one would expect from <stddef.h>:

typedef struct {
#if defined(__clang__) || defined(__llvm__)
	long long __clang_max_align_nonce1 _ALIGNMENT(long long);
	long double __clang_max_align_nonce2 _ALIGNMENT(long, double);
#elif defined(__GNUC__)
	long long __max_align_ll _ALIGNMENT(long, long);
	long double __max_align_ld _ALIGNMENT(long, double);
#else
	__ATOMIC long long __max_align_ll;
	__ATOMIC long double __max_align_ld;

#if defined(__i386) || defined(__amd64)
	__ATOMIC double __COMPLEX  __max_align_d;
#endif	/* defined(__i386) || defined(__amd64) */
#endif	/* defined(__clang__) || defined(__llvm__) */
} max_align_t;
#endif	/* _MAX_ALIGN_T */

	Rainer
Comment 7 Rainer Orth 2017-06-07 14:20:37 UTC
Created attachment 41491 [details]
test program
Comment 8 Richard Biener 2018-01-25 08:21:21 UTC
GCC 7.3 is being released, adjusting target milestone.
Comment 9 Jonathan Wakely 2018-03-16 16:41:54 UTC
This doesn't seem like a libstdc++ bug. malloc must return memory suitably aligned for any type of object with a fundamental alignment, i.e. <= _Alignof(max_align_t).

So GCC's definition of max_align_t is not consistent with malloc in Solaris 12, which means that GCC and malloc don't agree on the maximum fundamental alignment for the target.
Comment 10 Jonathan Wakely 2018-03-16 16:45:00 UTC
(In reply to Jonathan Wakely from comment #9)
> So GCC's definition of max_align_t is not consistent with malloc in Solaris

Oh, I'm assuming here that the definition of max_align_t is coming from GCC's <stddef.h>.

Wherever that max_align_t definitions comes from, it should match what malloc does.
Comment 11 ro@CeBiTec.Uni-Bielefeld.DE 2018-03-21 20:30:09 UTC
> --- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> (In reply to Jonathan Wakely from comment #9)
>> So GCC's definition of max_align_t is not consistent with malloc in Solaris
>
> Oh, I'm assuming here that the definition of max_align_t is coming from GCC's
> <stddef.h>.

Thanks for that reminder: I'd completely forgotten about
gcc/ginclude/stddef.h and only looked at the system header.  That header
includes

  /* _Float128 is defined as a basic type, so max_align_t must be
     sufficiently aligned for it.  This code must work in C++, so we
     use __float128 here; that is only available on some
     architectures, but only on i386 is extra alignment needed for
     __float128.  */
#ifdef __i386__
  __float128 __max_align_f128 __attribute__((__aligned__(__alignof(__float128))));
#endif

(which <stddef.h> of course doesn't have), introduced in

	https://gcc.gnu.org/ml/gcc-patches/2016-09/msg00652.html

> Wherever that max_align_t definitions comes from, it should match what malloc
> does.

I don't think that's necessarily true:

* According to the i386 psABI, p.8, Table 2.1, support for __float128 is
  optional.

* Studio 12.6 cc doesn't support it:

  https://docs.oracle.com/cd/E77782_01/html/E77792/gqexw.html#OSGCCgqexp

* Thus neither does the system malloc.  Even if this could be changed
  for (say) Solaris 11.5 (I'll check about this once discussion of this
  bug is finished), that leaves us with older Solaris versions.

As an experiment, I've disabled the __float128/i386 part of the
max_align_t definition in gcc/ginclude/stdlib.h.  Not unexpectedly, the
current test PASSes now:

-FAIL: experimental/memory_resource/resource_adaptor.cc execution test

while another FAILs:

+FAIL: gcc.dg/float128-align.c (test for excess errors)

Excess errors:
/vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.dg/floatn-align.h:17:1: error: static assertion failed: "max_align_t must be at least as aligned as _Float* types"

I'm uncertain how to deal with this: of course we could xfail the
current test and be done with it, but perhaps there are other options?

Joseph, any suggestions?  You mentioned that older versions of glibc
didn't provide 16-byte alignment in i386 malloc; maybe there are other
systems with the same issue that could equally benefit from some fix?

	Rainer
Comment 12 jsm-csl@polyomino.org.uk 2018-03-21 22:09:04 UTC
On Wed, 21 Mar 2018, ro at CeBiTec dot Uni-Bielefeld.DE wrote:

> Joseph, any suggestions?  You mentioned that older versions of glibc
> didn't provide 16-byte alignment in i386 malloc; maybe there are other
> systems with the same issue that could equally benefit from some fix?

The fix in glibc was to increase malloc alignment for i386.  This is an 
area where properly supporting a feature (_Float128 and _Decimal128, both 
of which are 16-byte aligned on i386) requires cooperation between the 
compiler and library - in this case, even without other library support 
for those types, users can still legitimately expect memory from malloc to 
be suitably aligned for them, and for max_align_t to have suitable 
alignment for them.
Comment 13 ro@CeBiTec.Uni-Bielefeld.DE 2018-03-22 07:58:57 UTC
> --- Comment #12 from joseph at codesourcery dot com <joseph at codesourcery
> dot com> ---
> On Wed, 21 Mar 2018, ro at CeBiTec dot Uni-Bielefeld.DE wrote:
>
>> Joseph, any suggestions?  You mentioned that older versions of glibc
>> didn't provide 16-byte alignment in i386 malloc; maybe there are other
>> systems with the same issue that could equally benefit from some fix?
>
> The fix in glibc was to increase malloc alignment for i386.  This is an 
> area where properly supporting a feature (_Float128 and _Decimal128, both 
> of which are 16-byte aligned on i386) requires cooperation between the 
> compiler and library - in this case, even without other library support 
> for those types, users can still legitimately expect memory from malloc to 
> be suitably aligned for them, and for max_align_t to have suitable 
> alignment for them.

I see.  So I'll go ahead and xfail the current testcase (unless Jonathan
prefers to move the failing max_align_t part to a separate testcase) and
pursue increasing i386 malloc alignment and the __float128 addition to
<stddef.h> max_align_t (for non-Studio compilers only, I guess) in Solaris.

Thanks.
	Rainer
Comment 14 Jonathan Wakely 2018-03-22 08:43:43 UTC
We *could* make the libstdc++ operator new call malloc repeatedly until it gets something aligned to max_align_t, so that operator new and the C++ allocators meet the alignment requirements.

But that seems quite horrible.
Comment 15 Rainer Orth 2018-03-22 10:06:23 UTC
Mine, patch posted.
Comment 16 Rainer Orth 2018-03-22 13:34:01 UTC
Author: ro
Date: Thu Mar 22 13:33:29 2018
New Revision: 258766

URL: https://gcc.gnu.org/viewcvs?rev=258766&root=gcc&view=rev
Log:
xfail experimental/memory_resource/resource_adaptor.cc on 32-bit Solaris/x86 (PR libstdc++/77691)

	PR libstdc++/77691
	* testsuite/experimental/memory_resource/resource_adaptor.cc:
	xfail execution on 32-bit Solaris/x86.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/testsuite/experimental/memory_resource/resource_adaptor.cc
Comment 17 ro@CeBiTec.Uni-Bielefeld.DE 2018-03-22 13:39:22 UTC
> --- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> We *could* make the libstdc++ operator new call malloc repeatedly until it gets
> something aligned to max_align_t, so that operator new and the C++ allocators
> meet the alignment requirements.
>
> But that seems quite horrible.

Indeed, and I'd rather not avoid that penalty for a corner case.  To
achieve this, one could instead use aligned_alloc instead, which is only
in Solaris 11.4, however.

	Rainer
Comment 18 Rainer Orth 2018-03-22 13:41:07 UTC
Xfailed for GCC 8.1.

I'm leaving the PR open for the underlying issue.
Comment 19 Rainer Orth 2018-03-22 17:43:03 UTC
Author: ro
Date: Thu Mar 22 17:42:32 2018
New Revision: 258778

URL: https://gcc.gnu.org/viewcvs?rev=258778&root=gcc&view=rev
Log:
xfail experimental/memory_resource/resource_adaptor.cc on 32-bit Solaris/x86 (PR libstdc++/77691)

	PR libstdc++/77691
	* testsuite/experimental/memory_resource/resource_adaptor.cc:
	xfail execution on 32-bit Solaris/x86.

Modified:
    branches/gcc-7-branch/libstdc++-v3/ChangeLog
    branches/gcc-7-branch/libstdc++-v3/testsuite/experimental/memory_resource/resource_adaptor.cc
Comment 20 Rainer Orth 2018-03-22 17:45:02 UTC
Xfailed for 7.4, too.
Comment 21 Rainer Orth 2018-07-26 12:36:53 UTC
Author: ro
Date: Thu Jul 26 12:36:21 2018
New Revision: 263000

URL: https://gcc.gnu.org/viewcvs?rev=263000&root=gcc&view=rev
Log:
xfail experimental/memory_resource/new_delete_resource.cc on 32-bit Solaris/x86 (PR libstdc++/77691)

	PR libstdc++/77691
	* testsuite/experimental/memory_resource/new_delete_resource.cc:
	xfail execution on 32-bit Solaris/x86.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc
Comment 22 Rainer Orth 2018-08-31 15:27:15 UTC
While debugging what could be memory corruption as reported in PR bootstrap/87134,
I found another related issue: when I preload libumem.so (an alternative
debugging allocator library), two new failures occur:

+FAIL: experimental/memory_resource/new_delete_resource.cc execution test

/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc:117: void test03(): Assertion 'aligned<max_align_t>(p)' failed.

+FAIL: experimental/memory_resource/resource_adaptor.cc execution test

/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/experimental/memory_resource/resource_adaptor.cc:136: void test05(): Assertion 'aligned<max_align_t>(p)' failed.

this time for the 64-bit test only.  I find that malloc(1) returns 8-byte
aligned storage, while alignof(max_align_t) is 16.

Reading https://github.com/jemalloc/jemalloc/issues/1072 suggests that this might
be in accordance with C11.
Comment 23 Jonathan Wakely 2018-08-31 15:52:29 UTC
Reasonable people have different opinions on whether that's valid. But since they're unlikely to change, I need to accept that with some implementations, malloc(n) is aligned to min(n, alignof(max_align_t)).
Comment 24 Jonathan Wakely 2018-09-03 15:35:21 UTC
I think this would make the tests pass, so you could remove the xfail directives:

--- a/libstdc++-v3/include/experimental/memory_resource
+++ b/libstdc++-v3/include/experimental/memory_resource
@@ -421,7 +421,11 @@ namespace pmr {
       do_allocate(size_t __bytes, size_t __alignment) override
       {
        if (__alignment <= __guaranteed_alignment<_Alloc>::value)
-         return _M_alloc.allocate(__bytes);
+         {
+           if (__bytes < __alignment)
+             __bytes = __alignment;
+           return _M_alloc.allocate(__bytes);
+         }
 
        const _AlignMgr __mgr(__bytes, __alignment);
        // Assume _M_alloc returns 1-byte aligned memory, so allocate enough
@@ -437,6 +441,8 @@ namespace pmr {
        auto __ptr = static_cast<char*>(__p);
        if (__alignment <= __guaranteed_alignment<_Alloc>::value)
          {
+           if (__bytes < __alignment)
+             __bytes = __alignment;
            _M_alloc.deallocate(__ptr, __bytes);
            return;
          }
Comment 25 Jonathan Wakely 2018-10-11 23:21:43 UTC
Author: redi
Date: Thu Oct 11 23:21:11 2018
New Revision: 265068

URL: https://gcc.gnu.org/viewcvs?rev=265068&root=gcc&view=rev
Log:
PR libstdc++/77691 increase allocation size to at least alignment

It's not safe to assume that malloc(n) returns memory aligned to more
than n, so when relying on the guaranteed alignment of malloc ensure
that the number of bytes allocated is at least as large as the
alignment.

	PR libstdc++/77691
	* include/experimental/memory_resource (__resource_adaptor_imp): Do
	not allocate sizes smaller than alignment when relying on guaranteed
	alignment.
	* testsuite/experimental/memory_resource/new_delete_resource.cc:
	Adjust expected number of bytes allocated for alignof(max_align_t).

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/experimental/memory_resource
    trunk/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc
Comment 26 Jonathan Wakely 2018-10-11 23:23:32 UTC
The new failures mentioned in comment 22 might be fixed on trunk now (although the disagreement about alignof(max_align_t) might mean it isn't).
Comment 27 John David Anglin 2019-02-22 21:54:24 UTC
Fails with gcc-9 on hppa64-hp-hpux11.11.
Comment 28 Jonathan Wakely 2019-02-27 09:22:52 UTC
(In reply to John David Anglin from comment #27)
> Fails with gcc-9 on hppa64-hp-hpux11.11.

What's the error in the libstdc++.log file?
Comment 29 John David Anglin 2019-03-09 16:21:20 UTC
/test/gnu/gcc/gcc/libstdc++-v3/testsuite/experimental/memory_resource/resource_adaptor.cc:136: void test05(): Assertion 'aligned<max_align_t>(p)' failed.
FAIL: experimental/memory_resource/resource_adaptor.cc execution test
Comment 30 Jonathan Wakely 2019-03-11 17:18:58 UTC
Created attachment 45940 [details]
Patch to fix resource_adaptor failures due to max_align_t bugs

Could you please try this patch on Soalris and HP-UX?

Commit log:

    Remove the hardcoded whitelist of allocators expected to return memory
    aligned to alignof(max_align_t), because that doesn't work when the
    platform's malloc() and GCC's max_align_t do not agree what the largest
    fundamental alignment is.
    
    Instead use a hardcoded list of fundamental types that will definitely
    be supported by the platform malloc, and use a copy of the allocator
    rebound to the first type that matches the requested alignment.
    
    This means allocations for alignof(__float128) might use additional
    memory to store the token, because we can't assume the platform malloc
    will return memory aligned to alignof(__float128). Ideally we'd be able
    to add __float128 to the list of fundamental types when we know it works
    for a given target.
Comment 31 John David Anglin 2019-03-15 23:03:53 UTC
The patch didn't the fail on hppa64-hp-hpux11.11 but the error has changed:

/test/gnu/gcc/gcc/libstdc++-v3/testsuite/experimental/memory_resource/resource_a
daptor.cc:64: void test05(): Assertion 'aligned<max_align_t>(p)' failed.
FAIL: experimental/memory_resource/resource_adaptor.cc execution test

The patched also introduced:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89732
Comment 32 Jonathan Wakely 2019-05-20 16:14:51 UTC
*** Bug 89732 has been marked as a duplicate of this bug. ***
Comment 33 Jonathan Wakely 2019-05-21 23:00:56 UTC
I've been working on this again, and I think that the resource_adaptor type is the wrong place to fix the malloc alignment problem.

The correct fix is to adjust the value of __STDCPP_DEFAULT_NEW_ALIGNMENT__ on targets where malloc doesn't agree with GCC's alignof(max_align_t). Otherwise even if I make resource_adaptor work for those targets, this test will still fail:

    #include <memory>
    #include <cstddef>
    #include <cstdint>
    #include <assert.h>

    template<typename T>
      inline bool aligned(void* p)
      { return (reinterpret_cast<std::uintptr_t>(p) % alignof(T)) == 0; }

    int
    main()
    {
      using test_type = std::max_align_t; // largest fundamental alignment
      std::allocator<test_type> a;
      for (int i = 0; i < 10; ++i)
      {
        test_type* p1 = a.allocate(1);
        VERIFY( aligned<test_type>(p1) );
        test_type* p2 = a.allocate(20);
        VERIFY( aligned<test_type>(p2) );
        a.deallocate(p1, 1);
        a.deallocate(p2, 20);
      }
    }

If we make std::allocator<max_align_t> work then the tests for resource_adaptor will also work, at least on Solaris.
Comment 34 Jonathan Wakely 2019-05-22 15:03:20 UTC
(In reply to Jonathan Wakely from comment #33)
> The correct fix is to adjust the value of __STDCPP_DEFAULT_NEW_ALIGNMENT__
> on targets where malloc doesn't agree with GCC's alignof(max_align_t).

That only helps for C++17 and later though :-(

The <experimental/memory_resource> header is defined for C++14.
Comment 35 dave.anglin 2019-05-22 15:32:51 UTC
On 2019-05-22 11:03 a.m., redi at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77691
>
> --- Comment #34 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> (In reply to Jonathan Wakely from comment #33)
>> The correct fix is to adjust the value of __STDCPP_DEFAULT_NEW_ALIGNMENT__
>> on targets where malloc doesn't agree with GCC's alignof(max_align_t).
> That only helps for C++17 and later though :-(
>
> The <experimental/memory_resource> header is defined for C++14.
>
Reminds me of this patch:
https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00528.html
Comment 36 Jonathan Wakely 2019-05-22 19:41:06 UTC
Interesting. Yes, definitely similar ideas. It looks like it was solved differently though, as config/pa/pa.h has

#define MALLOC_ABI_ALIGNMENT (TARGET_64BIT ? 128 : 64)

which should get used by the aligned new code, even without my suggested change in PR 90569.

As an aside, the comment on MALLOC_ABI_ALIGNMENT says "The glibc implementation currently provides 8-byte alignment." But glibc malloc was changed to 16-byte alignment a couple of years ago.
Comment 37 dave.anglin 2019-05-22 20:25:13 UTC
On 2019-05-22 3:41 p.m., redi at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77691
>
> --- Comment #36 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> Interesting. Yes, definitely similar ideas. It looks like it was solved
> differently though, as config/pa/pa.h has
>
> #define MALLOC_ABI_ALIGNMENT (TARGET_64BIT ? 128 : 64)
>
> which should get used by the aligned new code, even without my suggested change
> in PR 90569.
>
> As an aside, the comment on MALLOC_ABI_ALIGNMENT says "The glibc implementation
> currently provides 8-byte alignment." But glibc malloc was changed to 16-byte
> alignment a couple of years ago.
I believe I changed the glibc value because of the pthread mutex issue.

MALLOC_ABI_ALIGNMENT is defined in pa32-linux.h as follows:
#define MALLOC_ABI_ALIGNMENT 128

So, the defines are now consistent on linux.  The only remaining problem is 64-bit hpux where the actual
malloc alignment is 8 bytes.  The resource_adapter.cc test still fails on it.  Maybe I should change BIGGEST_ALIGNMENT
and MALLOC_ABI_ALIGNMENT to match the malloc implementation?
Comment 38 Jonathan Wakely 2019-05-22 20:30:10 UTC
Author: redi
Date: Wed May 22 20:29:39 2019
New Revision: 271522

URL: https://gcc.gnu.org/viewcvs?rev=271522&root=gcc&view=rev
Log:
PR libstdc++/77691 fix resource_adaptor failures due to max_align_t bugs

Remove the hardcoded whitelist of allocators expected to return memory
aligned to alignof(max_align_t), because that doesn't work when the
platform's malloc() and GCC's max_align_t do not agree what the largest
fundamental alignment is. It's also sub-optimal for user-defined
allocators that return memory suitable for any fundamental alignment.

Instead use a hardcoded list of alignments that are definitely supported
by the platform malloc, and use a copy of the allocator rebound to a POD
type with the requested alignment. Only allocate an oversized
buffer to use with std::align for alignments larger than any of the
hardcoded values.

For 32-bit Solaris x86 do not include alignof(max_align_t) in the
hardcoded values.

	PR libstdc++/77691
	* include/experimental/memory_resource: Add system header pragma.
	(__resource_adaptor_common::__guaranteed_alignment): Remove.
	(__resource_adaptor_common::_Types)
	(__resource_adaptor_common::__new_list)
	(__resource_adaptor_common::_New_list)
	(__resource_adaptor_common::_Alignments)
	(__resource_adaptor_common::_Fund_align_types): New utilities for
	creating a list of types with fundamental alignments.
	(__resource_adaptor_imp::do_allocate): Call new _M_allocate function.
	(__resource_adaptor_imp::do_deallocate): Call new _M_deallocate
	function.
	(__resource_adaptor_imp::_M_allocate): New function that first tries
	to use an allocator rebound to a type with a fundamental alignment.
	(__resource_adaptor_imp::_M_deallocate): Likewise for deallocation.
	* testsuite/experimental/memory_resource/new_delete_resource.cc:
	Adjust expected allocation sizes.
	* testsuite/experimental/memory_resource/resource_adaptor.cc: Remove
	xfail.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/experimental/memory_resource
    trunk/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc
    trunk/libstdc++-v3/testsuite/experimental/memory_resource/resource_adaptor.cc
Comment 39 Jonathan Wakely 2019-05-22 20:38:19 UTC
(In reply to dave.anglin from comment #37)
> I believe I changed the glibc value because of the pthread mutex issue.

Aha.
 
> MALLOC_ABI_ALIGNMENT is defined in pa32-linux.h as follows:
> #define MALLOC_ABI_ALIGNMENT 128
> 
> So, the defines are now consistent on linux.  The only remaining problem is
> 64-bit hpux where the actual
> malloc alignment is 8 bytes.  The resource_adapter.cc test still fails on

I've just committed a change to the resource_adaptor implementation, but I don't expect it to change the FAIL for hpux yet. I hope the FAILs are fixed for Solaris now though, and if so then we make the special case apply to 64-bit hpux too, like so (are these the right macros to check for?):

diff --git a/libstdc++-v3/include/experimental/memory_resource b/libstdc++-v3/include/experimental/memory_resource
index dde3753fab7..dd6f3099a78 100644
--- a/libstdc++-v3/include/experimental/memory_resource
+++ b/libstdc++-v3/include/experimental/memory_resource
@@ -413,7 +413,8 @@ 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 __i386__) \
+       && ! (defined __hpux && defined _LP64)
        if (__alignment == alignof(max_align_t))
          return _M_allocate<alignof(max_align_t)>(__bytes);
 #endif
@@ -439,7 +440,8 @@ namespace pmr {
       do_deallocate(void* __ptr, size_t __bytes, size_t __alignment) noexcept
       override
       {
-#if ! (defined __sun__ && defined __i386__)
+#if ! (defined __sun__ && defined __i386__) \
+       && ! (defined __hpux && defined _LP64)
        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 7dcb408f3f7..d4353ff6464 100644
--- a/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc
+++ b/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc
@@ -23,7 +23,8 @@
 #include <cstdlib>
 #include <testsuite_hooks.h>
 
-#if defined __sun__ && defined __i386__
+#if (defined __sun__ && defined __i386__) \
+       || (defined __hpux && defined _LP64)
 // See PR libstdc++/77691
 # define BAD_MAX_ALIGN_T 1
 #endif




> it.  Maybe I should change BIGGEST_ALIGNMENT
> and MALLOC_ABI_ALIGNMENT to match the malloc implementation?

I think that makes sense (although it won't change anything until we make the suggestion from PR 90569 as well, so I'll do that this week).
Comment 40 Richard Biener 2019-11-14 07:48:56 UTC
The GCC 7 branch is being closed, re-targeting to GCC 8.4.
Comment 41 Jakub Jelinek 2020-03-04 09:31:26 UTC
GCC 8.4.0 has been released, adjusting target milestone.
Comment 42 GCC Commits 2020-05-13 07:51:41 UTC
The master branch has been updated by Alexandre Oliva <aoliva@gcc.gnu.org>:

https://gcc.gnu.org/g:883246530f1bb10d854f455e1c3d55b93675690a

commit r11-347-g883246530f1bb10d854f455e1c3d55b93675690a
Author: Alexandre Oliva <oliva@adacore.com>
Date:   Wed May 13 04:49:00 2020 -0300

    x86-vxworks malloc aligns to 8 bytes like solaris
    
    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 remove the
    check for char-aligned requested allocation to be aligned like
    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.
    
    
    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): Drop max-align test for char-aligned alloc.
Comment 43 GCC Commits 2020-05-26 10:15:22 UTC
The releases/gcc-10 branch has been updated by Alexandre Oliva <aoliva@gcc.gnu.org>:

https://gcc.gnu.org/g:b425be2c4c6763436d63543501c6762ae031e43c

commit r10-8186-gb425be2c4c6763436d63543501c6762ae031e43c
Author: Alexandre Oliva <oliva@adacore.com>
Date:   Wed May 13 05:21:42 2020 -0300

    x86-vxworks malloc aligns to 8 bytes like solaris
    
    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 remove the
    check for char-aligned requested allocation to be aligned like
    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.
    
    
    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): Drop max-align test for char-aligned alloc.
    
    (cherry picked from commit 883246530f1bb10d854f455e1c3d55b93675690a)
Comment 44 Jonathan Wakely 2020-10-31 21:16:02 UTC
It looks like mingw* has the same problem:
https://sourceforge.net/p/mingw-w64/bugs/778/
mlloc returns memory aligned to 8 bytes, GCC's stddef.h says 16 is a fundamental alignment. Even worse, mingw's own stddef.h says 8, and which definition of max_align_t you get depends on include order.
Comment 45 John David Anglin 2021-02-17 16:30:10 UTC
We see this fail on hppa-linux:

https://buildd.debian.org/status/fetch.php?pkg=mysql-8.0&arch=hppa&ver=8.0.23-3&stamp=1613526368&raw=0
[ 49%] Building CXX object storage/innobase/CMakeFiles/innobase.dir/api/api0api.cc.o
cd /<<PKGBUILDDIR>>/builddir/storage/innobase && /usr/bin/hppa-linux-gnu-g++ -DBOOST_GEOMETRY_SQRT_CHECK_FINITENESS -DCOMPILER_HINTS -DHAVE_CONFIG_H -DHAVE_FALLOC_FL_ZERO_RANGE=1 -DHAVE_FALLOC_PUNCH_HOLE_AND_KEEP_SIZE=1 -DHAVE_IB_GCC_ATOMIC_THREAD_FENCE=1 -DHAVE_IB_GCC_SYNC_SYNCHRONISE=1 -DHAVE_IB_LINUX_FUTEX=1 -DHAVE_LZ4=1 -DHAVE_NANOSLEEP=1 -DHAVE_SCHED_GETCPU=1 -DHAVE_TLSv13 -DLINUX_NATIVE_AIO=1 -DLOG_SUBSYSTEM_TAG=\"InnoDB\" -DLZ4_DISABLE_DEPRECATE_WARNINGS -DMUTEX_EVENT -DMYSQL_SERVER -DPFS_DIRECT_CALL -DRAPIDJSON_NO_SIZETYPEDEFINE -DRAPIDJSON_SCHEMA_USE_INTERNALREGEX=0 -DRAPIDJSON_SCHEMA_USE_STDREGEX=1 -DUNIV_LINUX -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -D_USE_MATH_DEFINES -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/<<PKGBUILDDIR>>/builddir -I/<<PKGBUILDDIR>>/builddir/include -I/<<PKGBUILDDIR>> -I/<<PKGBUILDDIR>>/include -I/<<PKGBUILDDIR>>/storage/innobase -I/<<PKGBUILDDIR>>/storage/innobase/include -I/<<PKGBUILDDIR>>/storage/innobase/handler -I/<<PKGBUILDDIR>>/sql -I/<<PKGBUILDDIR>>/sql/auth -isystem /<<PKGBUILDDIR>>/extra/rapidjson/include -isystem /usr/include/editline -std=c++14 -fno-omit-frame-pointer -g -O2 -ffile-prefix-map=/<<PKGBUILDDIR>>=. -Wformat -Werror=format-security -Wall -Wextra -Wformat-security -Wvla -Wundef -Woverloaded-virtual -Wcast-qual -Wimplicit-fallthrough=2 -Wstringop-truncation -Wsuggest-override -Wlogical-op -Wno-unused-parameter -Wno-cast-qual -DDBUG_OFF -ffunction-sections -fdata-sections -O2 -g -DNDEBUG -fPIC   -Wdate-time -D_FORTIFY_SOURCE=2 -o CMakeFiles/innobase.dir/api/api0api.cc.o -c /<<PKGBUILDDIR>>/storage/innobase/api/api0api.cc
In file included from /<<PKGBUILDDIR>>/storage/innobase/include/sync0types.h:42,
                 from /<<PKGBUILDDIR>>/storage/innobase/include/univ.i:588,
                 from /<<PKGBUILDDIR>>/storage/innobase/os/file.h:47,
                 from /<<PKGBUILDDIR>>/storage/innobase/include/os0file.h:47,
                 from /<<PKGBUILDDIR>>/storage/innobase/include/api0misc.h:40,
                 from /<<PKGBUILDDIR>>/storage/innobase/api/api0api.cc:41:
/<<PKGBUILDDIR>>/storage/innobase/include/ut0new.h: In instantiation of ‘class ut_allocator<PolicyMutex<OSTrackMutex<GenericPolicy> > >’:
/<<PKGBUILDDIR>>/storage/innobase/include/ut0new.h:1033:19:   required from ‘void ut_delete(T*) [with T = PolicyMutex<OSTrackMutex<GenericPolicy> >]’
/<<PKGBUILDDIR>>/storage/innobase/include/dict0mem.h:2568:5:   required from here
/<<PKGBUILDDIR>>/storage/innobase/include/ut0new.h:583:28: error: static assertion failed: ut_allocator does not support over-aligned types. Use aligned_memory or another similar allocator for this type.
  583 |   static_assert(alignof(T) <= alignof(std::max_align_t),
      |                 ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
make[4]: *** [storage/innobase/CMakeFiles/innobase.dir/build.make:85: storage/innobase/CMakeFiles/innobase.dir/api/api0api.cc.o] Error 1

It seems we have 8-byte alignment specified using std::max_align_t and 16-byte
alignment for pthread mutexes and malloc.
Comment 46 Jonathan Wakely 2021-02-17 16:46:25 UTC
(In reply to John David Anglin from comment #45)
> It seems we have 8-byte alignment specified using std::max_align_t and
> 16-byte
> alignment for pthread mutexes and malloc.

That's not a libstdc++ issue though. std::max_align_t isn't defined by libstdc++, it comes from GCC's stddef.h
Comment 47 dave.anglin 2021-02-17 16:58:48 UTC
On 2021-02-17 11:46 a.m., redi at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77691
>
> --- Comment #46 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> (In reply to John David Anglin from comment #45)
>> It seems we have 8-byte alignment specified using std::max_align_t and
>> 16-byte
>> alignment for pthread mutexes and malloc.
> That's not a libstdc++ issue though. std::max_align_t isn't defined by
> libstdc++, it comes from GCC's stddef.h
Thanks, looking at it.
Comment 48 seurer 2021-03-25 15:13:23 UTC
Just an FYI: the patch done for gcc 10 (trunk at the time I believe) fixed a 32-bit failure in experimental/memory_resource/new_delete_resource.cc on powerpc64 32 bit testing.  The failure still occurs with gcc 9.

make  -k check RUNTESTFLAGS="--target_board=unix'{-m32}' conformance.exp=experimental/memory_resource/new_delete_resource.cc"
FAIL: experimental/memory_resource/new_delete_resource.cc execution test
# of expected passes		1
# of unexpected failures	1

/home/seurer/gcc/git/gcc-9-test/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc:125: void test03(): Assertion 'aligned<max_align_t>(p)' failed.
Comment 49 Jakub Jelinek 2021-05-14 09:48:32 UTC
GCC 8 branch is being closed.
Comment 50 Richard Biener 2021-06-01 08:08:18 UTC
GCC 9.4 is being released, retargeting bugs to GCC 9.5.
Comment 51 Richard Biener 2022-05-27 09:36:44 UTC
GCC 9 branch is being closed
Comment 52 Jakub Jelinek 2022-06-28 10:32:40 UTC
GCC 10.4 is being released, retargeting bugs to GCC 10.5.
Comment 53 John David Anglin 2023-01-05 21:26:33 UTC
Still fails on hppa64-hp-hpux11.11.
Comment 54 John David Anglin 2023-01-08 23:10:31 UTC
Created attachment 54213 [details]
Patch to fix test failure on hppa64-hp-hpux11.11
Comment 55 Jonathan Wakely 2023-01-09 11:20:46 UTC
Created attachment 54215 [details]
Generalise special case for malloc not afreeing with max_align_t

I think we also want to fix the actual code to meet the guarantee, not just relax the test.

Maybe like this.
Comment 56 dave.anglin 2023-01-09 22:14:41 UTC
On 2023-01-09 6:20 a.m., redi at gcc dot gnu.org wrote:
> Maybe like this.
Actually, the change i sent was for libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc.

It still fails.  No objection to the approach.
Comment 57 GCC Commits 2023-01-12 20:59:34 UTC
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:f629f63d2d9d7ad2c43f8e451f0f6e32b5f4d06a

commit r13-5127-gf629f63d2d9d7ad2c43f8e451f0f6e32b5f4d06a
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jan 12 10:58:13 2023 +0000

    libstdc++: Extend max_align_t special case to 64-bit HP-UX [PR77691]
    
    GCC's std::max_align_t doesn't agree with the system malloc on HP-UX, so
    generalize the current hack for Solaris to apply to that target too.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/77691
            * include/experimental/memory_resource
            (_GLIBCXX_MAX_ALIGN_MATCHES_MALLOC): Define.
            (do_allocate, do_deallocate): Check it.
            * testsuite/experimental/memory_resource/new_delete_resource.cc:
            Relax expected behaviour for 64-bit hppa-hp-hpux11.11.
Comment 58 Richard Biener 2023-07-07 10:31:42 UTC
GCC 10 branch is being closed.
Comment 59 Richard Biener 2024-07-19 12:59:27 UTC
GCC 11 branch is being closed.