Re: [PATCH v2] async_die_is_recursing: work around GCC v11.x issue on Fedora

[Derrick Stolee <stolee@gmail.com>]

On 11/4/2021 5:42 AM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Wed, Nov 03 2021, Junio C Hamano wrote:
> 
>> "Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>>> From: Victoria Dye <vdye@github.com>
>>>
>>> This fix corrects an issue found in the `dockerized(pedantic, fedora)` CI
>>> build, first appearing after the introduction of a new version of the Fedora
>>> docker image version. This image includes a version of `glibc` with the
>>> attribute `__attr_access_none` added to `pthread_setspecific` [1], the
>>> implementation of which only exists for GCC 11.X - the version included in
>>> the Fedora image. The attribute requires that the pointer provided in the
>>> second argument of `pthread_getspecific` must, if not NULL, be a pointer to
>>> a valid object. In the usage in `async_die_is_recursing`, `(void *)1` is not
>>> valid, causing the error.
>>>
>>> This fix imitates a workaround added in SELinux [2] by using the pointer to
>>> the static `async_die_counter` itself as the second argument to
>>> `pthread_setspecific`. This guaranteed non-NULL, valid pointer matches the
>>> intent of the current usage while not triggering the build error.
>>>
>>> [1] https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=a1561c3bbe8
>>> [2] https://lore.kernel.org/all/20211021140519.6593-1-cgzones@googlemail.com/
>>>
>>> Co-authored-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>>> Signed-off-by: Victoria Dye <vdye@github.com>
>>> ---
>>
>> Looks like they timed their update to disrupt us most effectively,
>> but we are gifted with watchful eyes and competent hands ;-).
>>
>> Thanks for coming up with a clearly written description and a clean
>> fix so quickly.
>>
>> Will apply.
> 
> I don't know what this would be for the "fedora" and other images, but
> it seems to me like the below is something we should do. This replaces
> "latest" with whatever "latest" currently maps onto.
> 
> I.e. I don't think it's a good thing that the carpet gets swept from
> under you as far as these CI images go. We could subscribe to some feed
> of when these images are bumped to see when to update, but having our
> base change from under us just leads to a waste of time for a bunch of
> people wondering why their CI is failing, which now they'll need to
> rebase on some on-list or only-in-upstream-master patch to have it
> "pass".
Having our CI go red because the 'latest' feed changed something is
probably the best "feed" to subscribe to, since we only get notified
if it matters.

Better to have automation go red than for us to not realize our code
doesn't work on newer platforms because our CI hasn't been updated.

Thanks,
-Stolee