[Bug libstdc++/113258] New: Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[Bug libstdc++/113258] New: Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[nmiell at gmail dot com]

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

            Bug ID: 113258
           Summary: Pre-C++17 code that supplies operator new/delete
                    crashes when mixed with post-C+17 code that uses the
                    align_val_t variants of new/delete
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: nmiell at gmail dot com
  Target Milestone: ---

Correctly functioning pre-C++17 applications that supply their own versions of
operator new and delete as allowed by the standard crash when mixed with
correctly functioning post-C++17 shared libraries that used the
std::align_val_t variants of operator new/delete.

This is typically the result of the libstdc++ version of operator delete(void*
ptr, std::align_val_t alignment) calling the application-supplied version of
operator delete(void* ptr), and the application-supplied allocator asserting or
crashing because the original pointer was produced by the libstdc++ version of
operator new(std::size_t size, std::align_val_t alignment) and not the
application-supplied operator new(std::size_t size).

This is both a regression and an ABI breakage.

A typical stack trace looks like the following:

 thread #1, name = 'bms_linux', stop reason = signal SIGABRT
  * frame #0: 0xf7f69589 [vdso]`__kernel_vsyscall + 9
    frame #1: 0xf7d5a2c7 libc.so.6`__pthread_kill_implementation + 279
    frame #2: 0xf7d05fc5 libc.so.6`raise + 37
    frame #3: 0xf7ced370 libc.so.6`abort + 242
    frame #4: 0xf7ed7e8a libtcmalloc_minimal.so.4`tcmalloc::Log(mode=kCrash,
filename="", line=278, a=LogItem @ 0xff92cd0c, b=LogItem @ 0xff92cd18,
c=LogItem @ 0xff92cd24, d=LogItem @ 0xff92cd30) at internal_logging.cc:120
    frame #5: 0xf7ed335d libtcmalloc_minimal.so.4`(anonymous
namespace)::InvalidFree(ptr=0x0a5c7d00) at tcmalloc.cc:278
    frame #6: 0xf7ee4762 libtcmalloc_minimal.so.4`::tc_free(void *) [inlined]
free_null_or_invalid(ptr=0x0a5c7d00, invalid_free_fn=0x0000b2b0) at
tcmalloc.cc:1141
    frame #7: 0xf7ee4751 libtcmalloc_minimal.so.4`::tc_free(void *) at
tcmalloc.cc:1185
    frame #8: 0xf7ee4720 libtcmalloc_minimal.so.4`::tc_free(void *) at
tcmalloc.cc:1225
    frame #9: 0xf7ee4720 libtcmalloc_minimal.so.4`::tc_free(void *) [inlined]
do_free(ptr=0x0a5c7d00) at tcmalloc.cc:1234
    frame #10: 0xf7ee4720 libtcmalloc_minimal.so.4`tc_free(ptr=0x0a5c7d00) at
tcmalloc.cc:1585
    frame #11: 0xf7899f4c libstdc++.so.6`operator delete(void*,
std::align_val_t) + 28
    frame #12: 0xeaaac2c8 libLLVM-17.so`llvm::deallocate_buffer(void*, unsigned
int, unsigned int) + 40
    frame #13: 0xeacd0ef6 libLLVM-17.so`llvm::DenseMap<void const*,
llvm::PassInfo const*, llvm::DenseMapInfo<void const*, void>,
llvm::detail::DenseMapPair<void const*, llvm::PassInfo const*>>::grow(unsigned
int) + 662
    frame #14: 0xead0db09 libLLVM-17.so`llvm::detail::DenseMapPair<void const*,
llvm::PassInfo const*>* llvm::DenseMapBase<llvm::DenseMap<void const*,
llvm::PassInfo const*, llvm::DenseMapInfo<void const*, void>,
llvm::detail::DenseMapPair<void const*, llvm::PassInfo const*>>, void const*,
llvm::PassInfo const*, llvm::DenseMapInfo<void const*, void>,
llvm::detail::DenseMapPair<void const*, llvm::PassInfo
const*>>::InsertIntoBucket<void const*, llvm::PassInfo
const*>(llvm::detail::DenseMapPair<void const*, llvm::PassInfo const*>*, void
const*&&, llvm::PassInfo const*&&) + 121
    frame #15: 0xead0d1fb
libLLVM-17.so`llvm::PassRegistry::registerPass(llvm::PassInfo const&, bool) +
283
    frame #16: 0xed0e209a
libLLVM-17.so`initializeSIFoldOperandsPassOnce(llvm::PassRegistry&) + 138
    frame #17: 0xeac6f832
libLLVM-17.so`std::once_flag::_Prepare_execution::_Prepare_execution<void
std::call_once<void* (&)(llvm::PassRegistry&),
std::reference_wrapper<llvm::PassRegistry>>(std::once_flag&, void*
(&)(llvm::PassRegistry&),
std::reference_wrapper<llvm::PassRegistry>&&)::'lambda'()>(void*
(&)(llvm::PassRegistry&))::'lambda'()::__invoke() + 50
    frame #18: 0xf7d5d4da libc.so.6`__pthread_once_slow + 250
    frame #19: 0xed0e1fd2
libLLVM-17.so`llvm::initializeSIFoldOperandsPass(llvm::PassRegistry&) + 130
    frame #20: 0xed042916 libLLVM-17.so`LLVMInitializeAMDGPUTarget + 166
    frame #21: 0xf238c8c7 radeonsi_dri.so`ac_init_llvm_target + 71
    frame #22: 0xf7d5d4da libc.so.6`__pthread_once_slow + 250
    frame #23: 0xf1ae26c0 radeonsi_dri.so`call_once + 32
    frame #24: 0xf238c946 radeonsi_dri.so`ac_init_shared_llvm_once + 38
    frame #25: 0xf238c968 radeonsi_dri.so`ac_init_llvm_once + 24

etc.

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This is a C++ standard issue I suspect which cannot be fixed just in libstdc++.
I suspect libc++ and MSVC's C++ library all have a similar issue too.

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
So the situtation here is you have a pre-C++17 application using a C++17
library.

If the application was C++17 and using a C++11 library, it would just work.

So the ABI is broken in the C++17 library rather than in libstdc++ as far as I
can tell. That is this is NOT a libstdc++ issue but the C++17 library making
sure it does not call the new `operator new`. Which means this is a bug in LLVM
....

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[nmiell at gmail dot com]

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

--- Comment #3 from Nicholas Miell <nmiell at gmail dot com> ---
Windows and macOS (and Solaris) have sensible dynamic linker semantics where
the pre-C++17 code and the post-C++17 code uses two different C++ runtimes and
furthermore the application can use tcmalloc while the GPU driver uses whatever
allocator it feels like.

GCC has dealt with similar issues before, cf. bug 68210.

One way to fix this issue would probably be to cause the libstdc++ supplied
std::align_val_t variants to always invoke the pre-C++17 operators, since that
is the implicit ABI contract.

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[nmiell at gmail dot com]

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

--- Comment #4 from Nicholas Miell <nmiell at gmail dot com> ---
(In reply to Andrew Pinski from comment #2)
> So the situtation here is you have a pre-C++17 application using a C++17
> library.
> 
> If the application was C++17 and using a C++11 library, it would just work.
> 
> So the ABI is broken in the C++17 library rather than in libstdc++ as far as
> I can tell. That is this is NOT a libstdc++ issue but the C++17 library
> making sure it does not call the new `operator new`. Which means this is a
> bug in LLVM ....

No, the problem is that a pre-C++17 application is using a post-C++17 GPU
driver (well, the GPU driver is using a post-C++17 libLLVM), and the post-C++17
version of libstdc++ is incompatible with pre-C++17 users of libstdc++.

If GCC supplied a pre-C++17 version of libstdc++ and a post-C++17 version of
libstdc++ and allowed both of them to be used simultaneously in a single
application, there wouldn't be a problem.

But instead GCC only supplies a post-C++17 version of libstdc++ that is
supposed to be ABI compatible with all previous versions of libstdc++, yet in
this case it isn't.

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[pinskia at gcc dot gnu.org]

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

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Nicholas Miell from comment #3)
> GCC has dealt with similar issues before, cf. bug 68210.

That was a defect to the C++11 standard unlike this one.

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Nicholas Miell from comment #4)
> No, the problem is that a pre-C++17 application is using a post-C++17 GPU
> driver (well, the GPU driver is using a post-C++17 libLLVM), and the
> post-C++17 version of libstdc++ is incompatible with pre-C++17 users of
> libstdc++.

The issue is the ABI of libLLVM changed which requires c++17 support in the
application if the application overrides `operator new`. That is a change in
libLLVM ABI and not in libstdc++ ABI.

libstdc++ ABI didn't change here but rather the compiled code ABI changed.
So in theory this is an ABI change in GCC but this is I suspect is an expected
change.  There is no way to support align_val_t `operator new` without
overriding C++17 version too. This means either libLLVM needs to change back to
making sure it does not call the aligned version (and use C++11) or the
applications (and drivers) which use C++ and override `operator new` will need
to also override the align_val_t version too.


>Windows and macOS (and Solaris) have sensible dynamic linker semantics where the pre-C++17 code and the post-C++17 code uses two different C++ runtimes and furthermore the application can use tcmalloc while the GPU driver uses whatever allocator it feels like.

How do they handle it, by having an `operator new` which is named different
depdent on the C++ version? Seems a little off and that itself would change the
ABI too and will cause pointers passed to each way to crash when deleting too.
I really doubt they handle this case that well either.

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[redi at gcc dot gnu.org]

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

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Nicholas Miell from comment #0)
> This is typically the result of the libstdc++ version of operator
> delete(void* ptr, std::align_val_t alignment) calling the
> application-supplied version of operator delete(void* ptr),

But it doesn't do that.

The libstdc++ version of operator delete(void*, size_t, align_val_t) calls
std::free directly, because that's the correct way to free memory obtained by
the libstdc++ version of the operator new(size_t, align_val_t).

If you are allocating memory with operator new(size_t, std::align_val_t) and
then seeing a crash when it's deallocated with operator delete(void*) that is a
bug in the application. Memory allocated by aligned-new must be deallocated by
aligned-delete, see [new.delete.single] p11:

If the alignment parameter is not present, ptr was returned by an allocation
function without an alignment parameter. If present, the alignment argument is
equal to the alignment argument passed to the allocation function that returned
ptr. If present, the size argument is equal to the size argument passed to the
allocation function that returned ptr.

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[nmiell at gmail dot com]

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

--- Comment #8 from Nicholas Miell <nmiell at gmail dot com> ---
(In reply to Jonathan Wakely from comment #7)
> (In reply to Nicholas Miell from comment #0)
> > This is typically the result of the libstdc++ version of operator
> > delete(void* ptr, std::align_val_t alignment) calling the
> > application-supplied version of operator delete(void* ptr),
> 
> But it doesn't do that.
> 
> The libstdc++ version of operator delete(void*, size_t, align_val_t) calls
> std::free directly, because that's the correct way to free memory obtained
> by the libstdc++ version of the operator new(size_t, align_val_t).
> 

Oh, I'm sorry, I did the initial investigation a couple weeks ago and only
wrote this up (from memory) when I started clearing out stale browser tabs
today. I guess I falsely assumed I had a better understanding of the issue than
I actually did, and I looked at the wrong operator delete when I was checking
my flawed assumptions.

What appears to actually be happening is that the libstdc++ operator
new(std::size_t size, std::align_val_t alignment) is calling glibc's
aligned_alloc, and then the libstdc++ operator delete(void* ptr,
std::align_val_t alignment) is calling std::free(), which is supplied by
tcmalloc.

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[pinskia at gcc dot gnu.org]

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

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Nicholas Miell from comment #8) 
> What appears to actually be happening is that the libstdc++ operator
> new(std::size_t size, std::align_val_t alignment) is calling glibc's
> aligned_alloc, and then the libstdc++ operator delete(void* ptr,
> std::align_val_t alignment) is calling std::free(), which is supplied by
> tcmalloc.


aligned_alloc is part of C11 and C++17 and is required to be free'd by
std::free (or free in the C11 case).

So this seems like a bug in tcmalloc not supplying an override of the C11
function aligned_alloc.

You would run into the same issue with a C11 application that does the same
too.

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2024-01-08

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #9)
> (In reply to Nicholas Miell from comment #8) 
> > What appears to actually be happening is that the libstdc++ operator
> > new(std::size_t size, std::align_val_t alignment) is calling glibc's
> > aligned_alloc, and then the libstdc++ operator delete(void* ptr,
> > std::align_val_t alignment) is calling std::free(), which is supplied by
> > tcmalloc.
> 
> 
> aligned_alloc is part of C11 and C++17 and is required to be free'd by
> std::free (or free in the C11 case).
> 
> So this seems like a bug in tcmalloc not supplying an override of the C11
> function aligned_alloc.
> 
> You would run into the same issue with a C11 application that does the same
> too.


From
https://github.com/gperftools/gperftools/blob/365060c4213a48adb27f63d5dfad41b3dfbdd62e/NEWS#L514
:
> == 30 Nov 2017 ==
> gperftools 2.6.2 is out!
> Most notable change is recently added support for C++17 over-aligned



So this is depedent on the version of tcmalloc that you are using.
What version are you using?

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[pinskia at gcc dot gnu.org]

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

--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
https://github.com/gperftools/gperftools/commit/d406f2285390c402e824dd28e6992f7f890dcdf9
was the commit which fixed tcmalloc .

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[nmiell at gmail dot com]

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

--- Comment #12 from Nicholas Miell <nmiell at gmail dot com> ---
(In reply to Andrew Pinski from comment #9)
> (In reply to Nicholas Miell from comment #8) 
> > What appears to actually be happening is that the libstdc++ operator
> > new(std::size_t size, std::align_val_t alignment) is calling glibc's
> > aligned_alloc, and then the libstdc++ operator delete(void* ptr,
> > std::align_val_t alignment) is calling std::free(), which is supplied by
> > tcmalloc.
> 
> 
> aligned_alloc is part of C11 and C++17 and is required to be free'd by
> std::free (or free in the C11 case).
> 
> So this seems like a bug in tcmalloc not supplying an override of the C11
> function aligned_alloc.
> 
> You would run into the same issue with a C11 application that does the same
> too.

That's not relevant, the language of the C++ standard doesn't require the
application to also displace aligned_free when displacing the aligned versions
of operator new or delete. The language of the standard also doesn't require
the application to displace versions of operator new and delete that didn't yet
exist in the version of the standard that the application targets. 

This version of tcmalloc predates aligned_alloc just as much as it predates the
align_val_t variants of operator new and delete, and the whole point of an ABI
is that existing binaries continue to work.

libstdc++ has elected to provide one implementation of the C++ runtime that is
ostensibly compatible with multiple revisions of the C++ language standard, the
C++23 version of libstdc++.so.6 is required to continue to support all existing
C++ executables dating back to at least 2004 (I'm not sure when version 6 was
introduced). It is also required to simultaneously support multiple versions of
the same C++ standard in a single process image, because that's how you chose
to implement it.

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|WAITING                     |RESOLVED

--- Comment #13 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Nicholas Miell from comment #12)
> That's not relevant, the language of the C++ standard doesn't require the
> application to also displace aligned_free when displacing the aligned
> versions of operator new or delete. The language of the standard also
> doesn't require the application to displace versions of operator new and
> delete that didn't yet exist in the version of the standard that the
> application targets. 

There is no aligned_free; only free, that is part of the issue here; the C and
c++17 standard requires that memory that was allocated with aligned_alloc be
free'able with free.

If you displace free, and don't displace aligned_alloc then things will go
wrong. that is not an backwards compatiable ABI issue since you are overriding
free here and such.


> This version of tcmalloc predates aligned_alloc just as much as it predates
> the align_val_t variants of operator new and delete, and the whole
> point of an ABI is that existing binaries continue to work.

The ABI is backwards compatiable but in this case, it is not forwards
compatiable.
That is an issue.

Also aligned_alloc is from C11 (and C++17), just not many people used it until
recently (in this case via operator new).

Basically glibc ABI might be backwards compatiable but replacing things means
the ABI of tcmalloc needs to match the ABI that is being used for glibc.

Again you mention it is aligned_alloc and free that is being called in the end.
That is exactly not a libstdc++ issue at all, rather it is a glibc ABI issue.

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[nmiell at gmail dot com]

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

Nicholas Miell <nmiell at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|INVALID                     |---
             Status|RESOLVED                    |NEW

--- Comment #14 from Nicholas Miell <nmiell at gmail dot com> ---
Again, this version of tcmalloc predates aligned_alloc and the align_val_t
versions of operator new and delete.

Again, the C++ standard does not require the application to displace
aligned_malloc when it displaces operator new or delete.

Again, it is libstdc++ that is calling aligned_alloc, not the application.

Again, the application displaced all the versions of operator new and delete
that were present in the version of the C++ standard that it targeted, in
addition to all the C malloc functions in the C standard it targeted.

Again, libstdc++ broke the ABI when it introduced the align_val_t versions of
operator new and delete because existing correctly functioning applications
stopped working in the presence of the new version of libstdc++.

Again, libstdc++ has elected to provide only one version of the C++ runtime
implementation that supports multiple versions of the C++ standard
simultaneously, instead of forking the library for every new revision of the
language standard.

Again, you could probably fix this issue by implementing all of the newly
introduced versions of operator new and delete solely using the versions of
operator new and delete that were present when libstdc++.so.6 was first
introduced.

Attempting to stick the blame on something other than libstdc++ doesn't even
solve the problem of the application crashing at startup, ultimately it will
have to be libstdc++ that works around this issue regardless. The application
certainly isn't getting updated, nor should it require updating; that's the
whole point of having ABIs in the first place.

The C++23 standard says that whether the default versions of operator new
"involves a call to the C standard library functions malloc or aligned_alloc is
unspecified." I assert that a C++ runtime that purports to simultaneously
implement both C++14 and previous (which do not have aligned_alloc) and C++17
and later (which do have aligned_alloc) must not call aligned_alloc in the
default behavior of any version of operator new as a basic quality of
implementation detail, given that C++14 and previous applications (which the
C++ runtime explicitly supports) are allowed to displace operator new and
delete without any knowledge of aligned_alloc.

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |INVALID

--- Comment #15 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The problem is libtcmalloc_minimal.so displaced free and this has nothing to do
with operator new/delete really.
It just happens that the aligned operator new calls aligned_alloc and then
calls free on that pointer and the free fails here because libtcmalloc is not
forwards compatible with c11's aligned_alloc.

This is also an issue with some c11 code that would call aligned_alloc and then
call free. The bug is again in libtcmalloc and not forwards compatible.

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[pinskia at gcc dot gnu.org]

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

--- Comment #16 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note multiple copies of the libstdc++ will not fix the issue here since
libtcmalloc provides a free which does not work with aligned_alloc. In fact it
would just fall over the same way. As I mentioned libtcmalloc is not even
compatible with C11. aligned_alloc was around for a few years even.

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[redi at gcc dot gnu.org]

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

--- Comment #17 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Nicholas Miell from comment #14)
> Again, this version of tcmalloc predates aligned_alloc and the align_val_t
> versions of operator new and delete.
> 
> Again, the C++ standard does not require the application to displace
> aligned_malloc when it displaces operator new or delete.
> 
> Again, it is libstdc++ that is calling aligned_alloc, not the application.
> 
> Again, the application displaced all the versions of operator new and delete
> that were present in the version of the C++ standard that it targeted, in
> addition to all the C malloc functions in the C standard it targeted.

Replacing malloc and free is not allowed by the standard at all, so this is
already outside the standard.

> Again, libstdc++ broke the ABI when it introduced the align_val_t versions
> of operator new and delete because existing correctly functioning
> applications stopped working in the presence of the new version of libstdc++.
> 
> Again, libstdc++ has elected to provide only one version of the C++ runtime
> implementation that supports multiple versions of the C++ standard
> simultaneously, instead of forking the library for every new revision of the
> language standard.
> 
> Again, you could probably fix this issue by implementing all of the newly
> introduced versions of operator new and delete solely using the versions of
> operator new and delete that were present when libstdc++.so.6 was first
> introduced.

We could do that, or we could just implement the aligned new in terms of malloc
and over-allocate and manually align in the buffer (which is what we have to do
for some targets without aligned_alloc or anything similar). But that sucks,
and wastes memory and cycles.

If aligned_alloc is supported, we use it. And that means we use free too.



> Attempting to stick the blame on something other than libstdc++ doesn't even
> solve the problem of the application crashing at startup, ultimately it will
> have to be libstdc++ that works around this issue regardless. The
> application certainly isn't getting updated, nor should it require updating;
> that's the whole point of having ABIs in the first place.
> 
> The C++23 standard says that whether the default versions of operator new
> "involves a call to the C standard library functions malloc or aligned_alloc
> is unspecified." I assert that a C++ runtime that purports to simultaneously
> implement both C++14 and previous (which do not have aligned_alloc) and

It's irrelevant whether C++14 has aligned_alloc, since libstdc++ doesn't
provide that anyway. What matters is whether the underlying libc provides it.
If it does, we call it.

> C++17 and later (which do have aligned_alloc) must not call aligned_alloc in
> the default behavior of any version of operator new as a basic quality of
> implementation detail, given that C++14 and previous applications (which the
> C++ runtime explicitly supports) are allowed to displace operator new and
> delete without any knowledge of aligned_alloc.

The problem has ABSOLUTELY NOTHING to do with replacing new/delete, it's
ENTIRELY due to replacing malloc/free.

You would get exactly the same problem in C code if your library using an old
tcmalloc was called by a new application that used aligned_alloc and then
called free, resulting in a call to the old tcmalloc free.

This is not a libstdc++ problem, nor a C++ problem.

If you want to blame the ABI break on something, blame C11 for adding
aligned_alloc and requiring that free can handle it. Libstdc++ has no way to
know if the version of free present at runtime can handle aligned_alloc. All we
can tell is that aligned_alloc exists, we can't tell whether some library that
might be linked to later breaks the libc ABI by replacing free in a way that
makes aligned_alloc unusable.

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[redi at gcc dot gnu.org]

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

--- Comment #18 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #17)
> (In reply to Nicholas Miell from comment #14)
> > Again, you could probably fix this issue by implementing all of the newly
> > introduced versions of operator new and delete solely using the versions of
> > operator new and delete that were present when libstdc++.so.6 was first
> > introduced.
> 
> We could do that, or we could just implement the aligned new in terms of
> malloc and over-allocate and manually align in the buffer (which is what we
> have to do for some targets without aligned_alloc or anything similar). But
> that sucks, and wastes memory and cycles.

To be clear, just like implementing it purely in terms of malloc, implementing
aligned-new in terms of the C++98 new/delete would also require over-allocating
and then aligning within the buffer, which wastes up to A-1 bytes in every
allocation (where A is the requested alignment).

If libc provides one of aligned_alloc, posix_memalign, memalign etc. then it's
much better to use that. And if we do that, then providing multiple different
libstdc++ runtimes for each version of the C++ standard would not help in the
slightest, because there's still only one std::free, because that doesn't come
from libstdc++ anyway.

And multiple libstdc++ runtimes would cause problems when trying to combine a
C++14 library into a C++17 application, because there would be more than one
std::cout and other globals. Unlike the Windows model, the ELF linkage model
doesn't support that well. It's not clear to me that it would solve any
problems, certainly not the one being reported here, but it would cause new
ones.

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[nmiell at gmail dot com]

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

--- Comment #19 from Nicholas Miell <nmiell at gmail dot com> ---
(In reply to Jonathan Wakely from comment #17)

> It's irrelevant whether C++14 has aligned_alloc, since libstdc++ doesn't
> provide that anyway. What matters is whether the underlying libc provides
> it. If it does, we call it.
> 

Yeah, and I'm saying that you can't use aligned_alloc, specifically because
libstdc++.so.6 is supposed to be backwards compatible with binaries that
predate C11 and C++11 (or target versions of C/C++ before then), and because
the language spec allows programs to displace operator new and delete and (in
practice) programs that do that are also going to replace the C malloc
functions *but only the C malloc functions that existed at the time*.

In fact, instead of over-allocating and then aligning the returned pointer in
the enlarged region, you can probably get away with using posix_memalign
instead of aligned_alloc. That dates back to 2001, before the libstdc++.so.5 to
6 SONAME bump and any allocator replacement you're going to encounter in
libstdc++.so.6 would've had to deal with the fact that free() has to be able to
free pointers returned from posix_memalign().

> The problem has ABSOLUTELY NOTHING to do with replacing new/delete, it's
> ENTIRELY due to replacing malloc/free.
> 
> You would get exactly the same problem in C code if your library using an
> old tcmalloc was called by a new application that used aligned_alloc and
> then called free, resulting in a call to the old tcmalloc free.
> 

And if that were to happen it would have to be dealt with in some other way,
but in this particular oddball case where the C++ standard has an
underspecified poorly thought out feature, it unfortunately becomes the C++
runtime's problem to deal with.

You're going to have to deal with this anyway if libstdc++ ever starts using
the aligned new and delete operators internally, because libstdc++ has to stay
compatible with programs that predate aligned_alloc.

> This is not a libstdc++ problem, nor a C++ problem.
> 
> If you want to blame the ABI break on something, blame C11 for adding
> aligned_alloc and requiring that free can handle it. Libstdc++ has no way to
> know if the version of free present at runtime can handle aligned_alloc. All
> we can tell is that aligned_alloc exists, we can't tell whether some library
> that might be linked to later breaks the libc ABI by replacing free in a way
> that makes aligned_alloc unusable.

You can use dladdr() to figure out which shared libraries a symbol comes from
and then make decisions about which features to used based on which subset of
symbols come from which libraries. It probably could be done from an IFUNC
resolver. Seems like overkill, though. Not using aligned_alloc at all would be
easier.

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[pinskia at gcc dot gnu.org]

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

--- Comment #20 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
So if you use an old tcmalloc and a c11 or c++1 library use aligned_alloc it
would crash in the same way.
Is that the same as this issue here, yes. Is it an abi change, NO because the
abi for malloc displacement is not technically a stable one and would be a
glibc issue rather than a gcc/libstdc++ one.

It just happens libstdc++ uses aligned_alloc.

[Bug libstdc++/113258] Pre-C++17 code that supplies operator new/delete crashes when mixed with post-C+17 code that uses the align_val_t variants of new/delete

[redi at gcc dot gnu.org]

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

--- Comment #21 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Nicholas Miell from comment #19)
> (In reply to Jonathan Wakely from comment #17)
>  
> > It's irrelevant whether C++14 has aligned_alloc, since libstdc++ doesn't
> > provide that anyway. What matters is whether the underlying libc provides
> > it. If it does, we call it.
> > 
> 
> Yeah, and I'm saying that you can't use aligned_alloc, specifically because
> libstdc++.so.6 is supposed to be backwards compatible with binaries that
> predate C11 and C++11 (or target versions of C/C++ before then), and because
> the language spec allows programs to displace operator new and delete and
> (in practice) programs that do that are also going to replace the C malloc
> functions *but only the C malloc functions that existed at the time*.

Again, this is not about replacing new/delete. That's allowed by the standard,
and would work fine if you just replaced new/delete. The problem is that you've
replaced malloc/free as well (or tcmalloc has replaced them for you).


> In fact, instead of over-allocating and then aligning the returned pointer
> in the enlarged region, you can probably get away with using posix_memalign
> instead of aligned_alloc. That dates back to 2001, before the libstdc++.so.5
> to 6 SONAME bump and any allocator replacement you're going to encounter in
> libstdc++.so.6 would've had to deal with the fact that free() has to be able
> to free pointers returned from posix_memalign().

Does your library's old tcmalloc replace posix_memalign?

> > The problem has ABSOLUTELY NOTHING to do with replacing new/delete, it's
> > ENTIRELY due to replacing malloc/free.
> > 
> > You would get exactly the same problem in C code if your library using an
> > old tcmalloc was called by a new application that used aligned_alloc and
> > then called free, resulting in a call to the old tcmalloc free.
> > 
> 
> And if that were to happen it would have to be dealt with in some other way,
> but in this particular oddball case where the C++ standard has an
> underspecified poorly thought out feature, it unfortunately becomes the C++
> runtime's problem to deal with.

How is this a problem of the C++ standard? The spec is clear: you can replace
new/delete, and you can replace the aligned ones or the non-aligned ones or
both. Your issue here is that libstdc++ uses aligned_alloc internally, which
has nothing to do with the standard.

[Bug libstdc++/113258] Pre-C++17 code that replaces malloc/free crashes when mixed with post-C++17 code that uses the align_val_t variants of new/delete

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Pre-C++17 code that         |Pre-C++17 code that
                   |supplies operator           |replaces malloc/free
                   |new/delete crashes when     |crashes when mixed with
                   |mixed with post-C+17 code   |post-C++17 code that uses
                   |that uses the align_val_t   |the align_val_t variants of
                   |variants of new/delete      |new/delete

--- Comment #22 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I've fixed the summary, since this is not caused by replacing the old
new/delete functions.

[Bug libstdc++/113258] Pre-C++17 code that replaces malloc/free crashes when mixed with post-C++17 code that uses the align_val_t variants of new/delete

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
         Resolution|INVALID                     |---
             Status|RESOLVED                    |ASSIGNED

--- Comment #23 from Jonathan Wakely <redi at gcc dot gnu.org> ---
And what matters is not that it's pre-C++17 code that replaces malloc/free, but
pre-C11. The C++ standard version being used by the malloc-replacing library is
irrelevant, what matters is how dated the malloc replacement is. A C11-aware
malloc replacement should also replace aligned_alloc.

Anyway, patch posted:
https://gcc.gnu.org/pipermail/gcc-patches/2024-January/642354.html

[Bug libstdc++/113258] Pre-C++17 code that replaces malloc/free crashes when mixed with post-C++17 code that uses the align_val_t variants of new/delete

[cvs-commit at gcc dot gnu.org]

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

--- Comment #24 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

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

commit r14-7146-gf50f2efae9fb0965d8ccdb62cfdb698336d5a933
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jan 9 15:22:46 2024 +0000

    libstdc++: Prefer posix_memalign for aligned-new [PR113258]

    As described in PR libstdc++/113258 there are old versions of tcmalloc
    which replace malloc and related APIs, but do not repalce aligned_alloc
    because it didn't exist at the time they were released. This means that
    when operator new(size_t, align_val_t) uses aligned_alloc to obtain
    memory, it comes from libc's aligned_alloc not from tcmalloc. But when
    operator delete(void*, size_t, align_val_t) uses free to deallocate the
    memory, that goes to tcmalloc's replacement version of free, which
    doesn't know how to free it.

    If we give preference to the older posix_memalign instead of
    aligned_alloc then we're more likely to use a function that will be
    compatible with the replacement version of free. Because posix_memalign
    has been around for longer, it's more likely that old third-party malloc
    replacements will also replace posix_memalign alongside malloc and free.

    libstdc++-v3/ChangeLog:

            PR libstdc++/113258
            * libsupc++/new_opa.cc: Prefer to use posix_memalign if
            available.

[Bug libstdc++/113258] Pre-C++17 code that replaces malloc/free crashes when mixed with post-C++17 code that uses the align_val_t variants of new/delete

[redi at gcc dot gnu.org]

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

--- Comment #25 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed on trunk only so far.

[Bug libstdc++/113258] Pre-C++17 code that replaces malloc/free crashes when mixed with post-C++17 code that uses the align_val_t variants of new/delete

[sjames at gcc dot gnu.org]

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

Sam James <sjames at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sjames at gcc dot gnu.org

--- Comment #26 from Sam James <sjames at gcc dot gnu.org> ---
Just to close the loop: this ended up being covered originally a few months
back with
https://airlied.blogspot.com/2023/04/fedora-38-llvm-vs-team-fortress-2-tf2.html
and https://gitlab.freedesktop.org/mesa/mesa/-/issues/8133. 

That's clearly what the original report above is derived from too.
Unfortunately, getting fixed copies of libtcmalloc in games which were shipped
a decade ago isn't likely.

Thanks for the workaround.

[Bug libstdc++/113258] Pre-C++17 code that replaces malloc/free crashes when mixed with post-C++17 code that uses the align_val_t variants of new/delete

[cvs-commit at gcc dot gnu.org]

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

--- Comment #27 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

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

commit r13-8307-gc5e12bbb45313df876ee2b81e418851822bed694
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jan 9 15:22:46 2024 +0000

    libstdc++: Prefer posix_memalign for aligned-new [PR113258]

    As described in PR libstdc++/113258 there are old versions of tcmalloc
    which replace malloc and related APIs, but do not repalce aligned_alloc
    because it didn't exist at the time they were released. This means that
    when operator new(size_t, align_val_t) uses aligned_alloc to obtain
    memory, it comes from libc's aligned_alloc not from tcmalloc. But when
    operator delete(void*, size_t, align_val_t) uses free to deallocate the
    memory, that goes to tcmalloc's replacement version of free, which
    doesn't know how to free it.

    If we give preference to the older posix_memalign instead of
    aligned_alloc then we're more likely to use a function that will be
    compatible with the replacement version of free. Because posix_memalign
    has been around for longer, it's more likely that old third-party malloc
    replacements will also replace posix_memalign alongside malloc and free.

    libstdc++-v3/ChangeLog:

            PR libstdc++/113258
            * libsupc++/new_opa.cc: Prefer to use posix_memalign if
            available.

    (cherry picked from commit f50f2efae9fb0965d8ccdb62cfdb698336d5a933)

[Bug libstdc++/113258] Pre-C++17 code that replaces malloc/free crashes when mixed with post-C++17 code that uses the align_val_t variants of new/delete

[cvs-commit at gcc dot gnu.org]

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

--- Comment #28 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

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

commit r12-10142-gb255ab901dd0d13ad7f0dc1a823749a5e5f62570
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jan 9 15:22:46 2024 +0000

    libstdc++: Prefer posix_memalign for aligned-new [PR113258]

    As described in PR libstdc++/113258 there are old versions of tcmalloc
    which replace malloc and related APIs, but do not repalce aligned_alloc
    because it didn't exist at the time they were released. This means that
    when operator new(size_t, align_val_t) uses aligned_alloc to obtain
    memory, it comes from libc's aligned_alloc not from tcmalloc. But when
    operator delete(void*, size_t, align_val_t) uses free to deallocate the
    memory, that goes to tcmalloc's replacement version of free, which
    doesn't know how to free it.

    If we give preference to the older posix_memalign instead of
    aligned_alloc then we're more likely to use a function that will be
    compatible with the replacement version of free. Because posix_memalign
    has been around for longer, it's more likely that old third-party malloc
    replacements will also replace posix_memalign alongside malloc and free.

    libstdc++-v3/ChangeLog:

            PR libstdc++/113258
            * libsupc++/new_opa.cc: Prefer to use posix_memalign if
            available.

    (cherry picked from commit f50f2efae9fb0965d8ccdb62cfdb698336d5a933)