Re: [glibc] nptl: Add missing placeholder abi symbol from nanosleep move

Re: [glibc] nptl: Add missing placeholder abi symbol from nanosleep move

[Florian Weimer]

* Adhemerval Zanella:

> https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=4f4bb489e0ddd2f24b2a5d352bb39f8dcdb38050
>
> commit 4f4bb489e0ddd2f24b2a5d352bb39f8dcdb38050
> Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> Date:   Thu Nov 7 15:10:35 2019 +0000
>
>     nptl: Add missing placeholder abi symbol from nanosleep move
>     
>     Adds the __libpthread_version_placeholder symbol with the same version
>     of nanosleep/__nanosleep that was removed by 79a547b162657b3f and that
>     is not provided by other symbols.

> +GLIBC_2.2 __libpthread_version_placeholder F

Sorry, this fix does not look correct to me.  Since GLIBC_2.2 has
members (and we can easily detect that), it should not get an additional
placeholder symbol.

I think you need to add a symbol alias and another compat_symbol to
nptl/libpthread-compat.c.

Thanks,
Florian

Re: [glibc] nptl: Add missing placeholder abi symbol from nanosleep move

[Adhemerval Zanella]

On 07/11/2019 13:54, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=4f4bb489e0ddd2f24b2a5d352bb39f8dcdb38050
>>
>> commit 4f4bb489e0ddd2f24b2a5d352bb39f8dcdb38050
>> Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>> Date:   Thu Nov 7 15:10:35 2019 +0000
>>
>>     nptl: Add missing placeholder abi symbol from nanosleep move
>>     
>>     Adds the __libpthread_version_placeholder symbol with the same version
>>     of nanosleep/__nanosleep that was removed by 79a547b162657b3f and that
>>     is not provided by other symbols.
> 
>> +GLIBC_2.2 __libpthread_version_placeholder F
> 
> Sorry, this fix does not look correct to me.  Since GLIBC_2.2 has
> members (and we can easily detect that), it should not get an additional
> placeholder symbol.
> 
> I think you need to add a symbol alias and another compat_symbol to
> nptl/libpthread-compat.c.

It is not strictly incorrect, although superfluous.  Ideally we would need
to add __libpthread_version_placeholder iff there is no other symbol with
same version already present, however I don't see straightforward to optimize
the symbol creation without adding some compat symbol hacking.

Is there a way to force linker to add the .gnu.version_d from the linker
version map without actually requiring the add this place holder symbol?

Re: [glibc] nptl: Add missing placeholder abi symbol from nanosleep move

[Florian Weimer]

* Adhemerval Zanella:

> On 07/11/2019 13:54, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=4f4bb489e0ddd2f24b2a5d352bb39f8dcdb38050
>>>
>>> commit 4f4bb489e0ddd2f24b2a5d352bb39f8dcdb38050
>>> Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>>> Date:   Thu Nov 7 15:10:35 2019 +0000
>>>
>>>     nptl: Add missing placeholder abi symbol from nanosleep move
>>>     
>>>     Adds the __libpthread_version_placeholder symbol with the same version
>>>     of nanosleep/__nanosleep that was removed by 79a547b162657b3f and that
>>>     is not provided by other symbols.
>> 
>>> +GLIBC_2.2 __libpthread_version_placeholder F
>> 
>> Sorry, this fix does not look correct to me.  Since GLIBC_2.2 has
>> members (and we can easily detect that), it should not get an additional
>> placeholder symbol.
>> 
>> I think you need to add a symbol alias and another compat_symbol to
>> nptl/libpthread-compat.c.
>
> It is not strictly incorrect, although superfluous.  Ideally we would need
> to add __libpthread_version_placeholder iff there is no other symbol with
> same version already present, however I don't see straightforward to optimize
> the symbol creation without adding some compat symbol hacking.

This change does not look correct:

-#if (SHLIB_COMPAT (libpthread, GLIBC_2_1_2, GLIBC_2_2))
+#if (SHLIB_COMPAT (libpthread, GLIBC_2_1_2, GLIBC_2_2_6))

You need

#if (SHLIB_COMPAT (libpthread, GLIBC_2_1_2, GLIBC_2_2) \
     || SHLIB_COMPAT (libpthread, GLIBC_2_1_6, GLIBC_2_3))

and then do something like this:

#if (SHLIB_COMPAT (libpthread, GLIBC_2_1_2, GLIBC_2_2))
strong_alias (__libpthread_version_placeholder,
              __libpthread_version_placeholder_212)
compat_symbol (libpthread, __libpthread_version_placeholder_212,
               __libpthread_version_placeholder, GLIBC_2_1_2);
#endif
#if (SHLIB_COMPAT (libpthread, GLIBC_2_1_6, GLIBC_2_3))
strong_alias (__libpthread_version_placeholder,
              __libpthread_version_placeholder_226)
compat_symbol (libpthread, __libpthread_version_placeholder_226,
               __libpthread_version_placeholder, GLIBC_2_2_6);
#endif

The strong_alias is needed because some binutils version/target
combinations do not support multiple versions for one symbol.

Thanks,
Florian

Re: [glibc] nptl: Add missing placeholder abi symbol from nanosleep move

[Florian Weimer]

* Adhemerval Zanella:

> Is there a way to force linker to add the .gnu.version_d from the linker
> version map without actually requiring the add this place holder symbol?

Oh, and I looked at this and binutils does strange things if you have a
symbol version that does not have any symbols attached to it, but is
referenced by a higher version number.  It creates a weak symbol
definition, and that's rather uncharted territory.

Thanks,
Florian

Re: [glibc] nptl: Add missing placeholder abi symbol from nanosleep move

[Adhemerval Zanella]

On 07/11/2019 14:21, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 07/11/2019 13:54, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=4f4bb489e0ddd2f24b2a5d352bb39f8dcdb38050
>>>>
>>>> commit 4f4bb489e0ddd2f24b2a5d352bb39f8dcdb38050
>>>> Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>>>> Date:   Thu Nov 7 15:10:35 2019 +0000
>>>>
>>>>     nptl: Add missing placeholder abi symbol from nanosleep move
>>>>     
>>>>     Adds the __libpthread_version_placeholder symbol with the same version
>>>>     of nanosleep/__nanosleep that was removed by 79a547b162657b3f and that
>>>>     is not provided by other symbols.
>>>
>>>> +GLIBC_2.2 __libpthread_version_placeholder F
>>>
>>> Sorry, this fix does not look correct to me.  Since GLIBC_2.2 has
>>> members (and we can easily detect that), it should not get an additional
>>> placeholder symbol.
>>>
>>> I think you need to add a symbol alias and another compat_symbol to
>>> nptl/libpthread-compat.c.
>>
>> It is not strictly incorrect, although superfluous.  Ideally we would need
>> to add __libpthread_version_placeholder iff there is no other symbol with
>> same version already present, however I don't see straightforward to optimize
>> the symbol creation without adding some compat symbol hacking.
> 
> This change does not look correct:
> 
> -#if (SHLIB_COMPAT (libpthread, GLIBC_2_1_2, GLIBC_2_2))
> +#if (SHLIB_COMPAT (libpthread, GLIBC_2_1_2, GLIBC_2_2_6))
> 
> You need
> 
> #if (SHLIB_COMPAT (libpthread, GLIBC_2_1_2, GLIBC_2_2) \
>      || SHLIB_COMPAT (libpthread, GLIBC_2_1_6, GLIBC_2_3))

Ack.

> 
> and then do something like this:
> 
> #if (SHLIB_COMPAT (libpthread, GLIBC_2_1_2, GLIBC_2_2))
> strong_alias (__libpthread_version_placeholder,
>               __libpthread_version_placeholder_212)
> compat_symbol (libpthread, __libpthread_version_placeholder_212,
>                __libpthread_version_placeholder, GLIBC_2_1_2);
> #endif
> #if (SHLIB_COMPAT (libpthread, GLIBC_2_1_6, GLIBC_2_3))
> strong_alias (__libpthread_version_placeholder,
>               __libpthread_version_placeholder_226)
> compat_symbol (libpthread, __libpthread_version_placeholder_226,
>                __libpthread_version_placeholder, GLIBC_2_2_6);
> #endif
> 
> The strong_alias is needed because some binutils version/target
> combinations do not support multiple versions for one symbol.
> 

Yeah, I indeed noted this binutils behaviour.  Alright, sorry for the
trouble, I will send an updated version.

Re: [glibc] nptl: Add missing placeholder abi symbol from nanosleep move

[Adhemerval Zanella]

On 07/11/2019 14:23, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> Is there a way to force linker to add the .gnu.version_d from the linker
>> version map without actually requiring the add this place holder symbol?
> 
> Oh, and I looked at this and binutils does strange things if you have a
> symbol version that does not have any symbols attached to it, but is
> referenced by a higher version number.  It creates a weak symbol
> definition, and that's rather uncharted territory.
> 
Sigh... ack. Looks like we will need to use the complex compat hack solution. 

Re: [glibc] nptl: Add missing placeholder abi symbol from nanosleep move

[Joseph Myers]

On Thu, 7 Nov 2019, Florian Weimer wrote:

> The strong_alias is needed because some binutils version/target
> combinations do not support multiple versions for one symbol.

Note that it's possible to generate such aliases automatically using 
__COUNTER__.  See how the totalorder symbol versioning handles that; it 
would be nice to convert that into something general used directly in the 
definitions of versioned_symbol / compat_symbol (after which it would be 
possible to incrementally eliminate existing definitions of aliases in 
lots of source files that are only for the purpose of applying symbol 
versions).

-- 
Joseph S. Myers
joseph@codesourcery.com