[Bug c++/90569] __STDCPP_DEFAULT_NEW_ALIGNMENT__ is wrong for i386-pc-solaris2.11

[redi at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90569

Jonathan Wakely  changed:

   What|Removed |Added

 Blocks||77691

--- Comment #10 from Jonathan Wakely  ---
This isn't solaris-specific, it affects vxworks and mingw too.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77691
[Bug 77691] [8/9/10/11 regression]
experimental/memory_resource/resource_adaptor.cc FAILs

[Bug c++/90569] __STDCPP_DEFAULT_NEW_ALIGNMENT__ is wrong for i386-pc-solaris2.11

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90569

--- Comment #9 from Jonathan Wakely  ---
(In reply to Florian Weimer from comment #8)
> The problem is that popular mallocs do not care about ABI and return
> unaligned pointers for allocations smaller than the max_align_t alignment. 
> As a result, __float128 support with 16-byte alignment would end up with a
> potentially rather short list of supported targets (including targets which
> have a 16-byte-aligned long double already).

Understood.

> I also found this for the first time:
> 
>   
> 
> So if the standard moves to the weak guarantee, GCC probably shouldn't make
> assumptions in the other direction.

Yes, I already fixed libstdc++ to not assume the strong-alignment reading, see
r265068.

But this PR isn't about the alignment of allocations where the size is smaller
than alignof(max_align_t). What makes my life difficult is that some mallocs
(including glibc up to 2.25) do not respect GCC's new alignof(max_align_t) even
for large allocations. I understand why, but it's just annoying that GCC
changed max_align_t to be misleading. If the underlying malloc doesn't agree,
making max_align_t lie isn't very helpful.

[Bug c++/90569] __STDCPP_DEFAULT_NEW_ALIGNMENT__ is wrong for i386-pc-solaris2.11

[fw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90569

Florian Weimer  changed:

   What|Removed |Added

 CC||fw at gcc dot gnu.org

--- Comment #8 from Florian Weimer  ---
(In reply to Jonathan Wakely from comment #1)
> (I'm starting to think that __float128 support should have been disabled on
> targets where it requires greater alignment than malloc guarantees, instead
> of making GCC's max_align_t lie).

The problem is that popular mallocs do not care about ABI and return unaligned
pointers for allocations smaller than the max_align_t alignment.  As a result,
__float128 support with 16-byte alignment would end up with a potentially
rather short list of supported targets (including targets which have a
16-byte-aligned long double already).

We spent a lot energy fixing this in glibc without breaking backwards
compatibility with existing Emacs binaries:

  

Basically, we had to include a heap rewriter that converts the heap from the
old to the new format.  But other mallocs wouldn't even have to do that, and
they still don't want to fix their default alignment choices unfortunately.

I also found this for the first time:

  

So if the standard moves to the weak guarantee, GCC probably shouldn't make
assumptions in the other direction.

[Bug c++/90569] __STDCPP_DEFAULT_NEW_ALIGNMENT__ is wrong for i386-pc-solaris2.11

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90569

--- Comment #7 from Jonathan Wakely  ---
The glibc change was https://sourceware.org/bugzilla/show_bug.cgi?id=21120 and
is present from version 2.26

[Bug c++/90569] __STDCPP_DEFAULT_NEW_ALIGNMENT__ is wrong for i386-pc-solaris2.11

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90569

--- Comment #6 from Jonathan Wakely  ---
This bug also affects 32-bit GNU/Linux with older versions of glibc.

[Bug c++/90569] __STDCPP_DEFAULT_NEW_ALIGNMENT__ is wrong for i386-pc-solaris2.11

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90569

--- Comment #5 from Jonathan Wakely  ---
(In reply to Jonathan Wakely from comment #4)
> Rainer, the change to gcc/cp/init.c would allow you to do:
> 
> #define MALLOC_ABI_ALIGNMENT 8

Oops, it's in bits not bytes, so that should be

#define MALLOC_ABI_ALIGNMENT 64

[Bug c++/90569] __STDCPP_DEFAULT_NEW_ALIGNMENT__ is wrong for i386-pc-solaris2.11

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90569

--- Comment #4 from Jonathan Wakely  ---
Rainer, the change to gcc/cp/init.c would allow you to do:

#define MALLOC_ABI_ALIGNMENT 8

in gcc/config/i386/sol2.h and that would cause std::allocator to know that it
can't rely on malloc for 16-byte alignment.

Although that would only help for C++17, because otherwise __cpp_aligned_new
isn't defined ... drat.

It's better than nothing though.

Does that seem acceptable for your target?

[Bug c++/90569] __STDCPP_DEFAULT_NEW_ALIGNMENT__ is wrong for i386-pc-solaris2.11

[jason at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90569

Jason Merrill  changed:

   What|Removed |Added

 CC||jason at gcc dot gnu.org

--- Comment #3 from Jason Merrill  ---
(In reply to Jonathan Wakely from comment #0)
> unsigned
> malloc_alignment ()
> {
>   if (MALLOC_ABI_ALIGNMENT != BITS_PER_WORD)
> return MALLOC_ABI_ALIGNMENT;
>   return MAX (max_align_t_align(), MALLOC_ABI_ALIGNMENT);
> }

The last line can just be 

  return max_align_t_align();

Otherwise looks good to me.

[Bug c++/90569] __STDCPP_DEFAULT_NEW_ALIGNMENT__ is wrong for i386-pc-solaris2.11

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90569

Jonathan Wakely  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2019-05-22
 Ever confirmed|0   |1

--- Comment #2 from Jonathan Wakely  ---
I thought we could workaround this in libstdc++ like so:

diff --git a/libstdc++-v3/libsupc++/Makefile.am
b/libstdc++-v3/libsupc++/Makefile.am
index eec7b953514..a50a9848461 100644
--- a/libstdc++-v3/libsupc++/Makefile.am
+++ b/libstdc++-v3/libsupc++/Makefile.am
@@ -129,6 +129,8 @@ cp-demangle.o: cp-demangle.c


 # Use special rules for the C++17 sources so that the proper flags are passed.
+new_op.lo: new_op.cc
+   $(LTCXXCOMPILE) -std=gnu++1z -c $<
 new_opa.lo: new_opa.cc
$(LTCXXCOMPILE) -std=gnu++1z -c $<
 new_opant.lo: new_opant.cc
diff --git a/libstdc++-v3/libsupc++/Makefile.in
b/libstdc++-v3/libsupc++/Makefile.in
index 5d8ac5ca0ba..0e3cbff0055 100644
--- a/libstdc++-v3/libsupc++/Makefile.in
+++ b/libstdc++-v3/libsupc++/Makefile.in
@@ -956,6 +956,8 @@ cp-demangle.o: cp-demangle.c
$(C_COMPILE) -DIN_GLIBCPP_V3 -Wno-error -c $<

 # Use special rules for the C++17 sources so that the proper flags are passed.
+new_op.lo: new_op.cc
+   $(LTCXXCOMPILE) -std=gnu++1z -c $<
 new_opa.lo: new_opa.cc
$(LTCXXCOMPILE) -std=gnu++1z -c $<
 new_opant.lo: new_opant.cc
diff --git a/libstdc++-v3/libsupc++/new_op.cc
b/libstdc++-v3/libsupc++/new_op.cc
index 863530b7564..203c57d9171 100644
--- a/libstdc++-v3/libsupc++/new_op.cc
+++ b/libstdc++-v3/libsupc++/new_op.cc
@@ -27,6 +27,9 @@
 #include 
 #include 
 #include "new"
+#if defined __sun__ || defined __i386__
+# include 
+#endif

 using std::new_handler;
 using std::bad_alloc;
@@ -41,6 +44,14 @@ extern "C" void *malloc (std::size_t);
 _GLIBCXX_WEAK_DEFINITION void *
 operator new (std::size_t sz) _GLIBCXX_THROW (std::bad_alloc)
 {
+#if defined __sun__ || defined __i386__
+  if (sz >= alignof(std::max_align_t))
+{
+  std::align_val_t al{alignof(std::max_align_t)};
+  return ::operator new(sz, al);
+}
+#endif
+
   void *p;

   /* malloc (0) is unpredictable; avoid it.  */



This would force operator new to use aligned_alloc instead of malloc for
allocations that might be for objects large enough to require greater alignment
than malloc guarantees. But since Solaris 11 doesn't appear to define
aligned_alloc, this would use the fallback implementation in
libsupc++/new_opa.cc which is much less efficient than plain malloc.

[Bug c++/90569] __STDCPP_DEFAULT_NEW_ALIGNMENT__ is wrong for i386-pc-solaris2.11

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90569

--- Comment #1 from Jonathan Wakely  ---
(I'm starting to think that __float128 support should have been disabled on
targets where it requires greater alignment than malloc guarantees, instead of
making GCC's max_align_t lie).