Re: [committed] hppa: Drop 16-byte pthread lock alignment

[Florian Weimer <fw@deneb.enyo.de>]

* John David Anglin:

> On 2023-03-26 8:37 p.m., Sam James wrote:
>> Florian Weimer <fw@deneb.enyo.de> writes:
>>
>>> * John David Anglin:
>>>
>>>> hppa: Drop 16-byte pthread lock alignment
>>>>
>>>> Linux threads were removed about 12 years ago and the current
>>>> nptl implementation only requires 4-byte alignment for pthread
>>>> locks.
>>>>
>>>> The 16-byte alignment causes various issues. For example in
>>>> building ignition-msgs, we have:
>>>>
>>>> /usr/include/google/protobuf/map.h:124:37: error: static assertion failed
>>>>    124 |   static_assert(alignof(value_type) <= 8, "");
>>>>        |                 ~~~~~~~~~~~~~~~~~~~~^~~~
>>>>
>>>> This is caused by the 16-byte pthread lock alignment.
>>> This was done deliberately to preserve ABI.  This change needs a mass
>>> rebuild because struct offsets after pthread_mutex_t members are
>>> likely to change.

> Although the change may change some structure offsets, I don't
> believe the change requires a mass rebuild.  I am running a Debian
> hppa system with the reduced lock alignment and so far I haven't
> observed any breakage.  I checked this prior to committing the
> change.

With nothing rebuilt, no struct offsets change, so this isn't
unexpected.

Maybe it's still the easiest way to fix the underlying issue, but it's
likely not going to stop at just changing the glibc headers.

> Here is comment in map.h:
>    // MapAllocator does not support alignments beyond 8. Technically we should
>    // support up to std::max_align_t, but this fails with ubsan and tcmalloc
>    // debug allocation logic which assume 8 as default alignment.
>    static_assert(alignof(value_type) <= 8, "");
>
> It was the above comment that convinced me that we needed to change
> the pthread lock alignment.

It seems that current tcmalloc honors GCC's
__STDCPP_DEFAULT_NEW_ALIGNMENT__:

  <https://github.com/google/tcmalloc/blob/master/tcmalloc/size_classes.cc>

On panama.debian.net, it seems correct:

$ gcc -x c++ -E - < /dev/null -dM | grep __STDCPP_DEFAULT_NEW_ALIGNMENT__
#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 16

Is it a matter of an outdated bundled tcmalloc copy?  Or is the assert
merely incorrect and should use __STDCPP_DEFAULT_NEW_ALIGNMENT__ as well?