[review v3] nptl: Add more missing placeholder abi symbol from nanosleep move

["Carlos O'Donell (Code Review)" <gerrit at gnutoolchain-gerrit dot osci dot io>]

Carlos O'Donell has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/524
......................................................................


Patch Set 3: Code-Review+2

(12 comments)

This version looks good to me and the expanded example shows the use of __COUNTER__ and how the aliases would be created with an equivalent macro API example.

OK for master.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +1,16 @@ 
| +Parent:     4f4bb489 (nptl: Add missing placeholder abi symbol from nanosleep move)
| +Author:     Adhemerval Zanella <adhemerval.zanella@linaro.org>
| +AuthorDate: 2019-11-07 17:47:07 +0000
| +Commit:     Adhemerval Zanella <adhemerval.zanella@linaro.org>
| +CommitDate: 2019-11-07 14:52:43 -0300
| +
| +nptl: Add more missing placeholder abi symbol from nanosleep move
| +
| +This patch adds the __libpthread_version_placeholder for GLIBC.2.2
| +and GLIBC_2.2.6 resulting the nanosleep implementation move to libc.

PS1, Line 10:

Done

| +This aims to fix the wrong compat symbol definition 79a547b162657b3.
| +
| +Checked with a updated-abi on the all afftected abis of nanosleep

PS1, Line 13:

Done

| +move.
| +
| +Change-Id: I347a4dbdc931bb42b359456932dd1e17aa4d4078
| --- nptl/libpthread-compat.c
| +++ nptl/libpthread-compat.c
| @@ -21,17 +21,31 @@ #include <shlib-compat.h>
|  /* This is an unused compatibility symbol definition, to prevent ld
|     from creating a weak version definition for GLIBC_2.1.2.  (__vfork
|     used to be defined at that version, but it is now provided by libc,
|     and there are no versions left in libpthread for that symbol
|     version.)  If the ABI baseline for glibc is the GLIBC_2.2 symbol
|     version or later, the placeholder symbol is not needed because
|     there are plenty of other symbols which populate those later
|     versions.  */
| -#if (SHLIB_COMPAT (libpthread, GLIBC_2_1_2, GLIBC_2_2_6))
| +#if (SHLIB_COMPAT (libpthread, GLIBC_2_1_2, GLIBC_2_2) \
| +     || SHLIB_COMPAT (libpthread, GLIBC_2_2_6, GLIBC_2_3))

PS1, Line 30:

Done

|  void
|  attribute_compat_text_section
|  __libpthread_version_placeholder (void)
|  {
|  }
|  compat_symbol (libpthread, __libpthread_version_placeholder,
|                 __libpthread_version_placeholder, GLIBC_2_1_2);
|  #endif
| +
| +#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_2_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

PS1, Line 51:

Done

| --- sysdeps/unix/sysv/linux/alpha/libpthread.abilist
| +++ sysdeps/unix/sysv/linux/alpha/libpthread.abilist
| @@ -186,18 +186,19 @@ GLIBC_2.2 pthread_spin_destroy F
|  GLIBC_2.2 pthread_spin_init F
|  GLIBC_2.2 pthread_spin_lock F
|  GLIBC_2.2 pthread_spin_trylock F
|  GLIBC_2.2 pthread_spin_unlock F
|  GLIBC_2.2 pthread_yield F
|  GLIBC_2.2 pwrite F
|  GLIBC_2.2 pwrite64 F
|  GLIBC_2.2 sem_timedwait F
|  GLIBC_2.2.3 pthread_getattr_np F
| +GLIBC_2.2.6 __libpthread_version_placeholder F

PS1, Line 195:

Done

|  GLIBC_2.28 call_once F
|  GLIBC_2.28 cnd_broadcast F
|  GLIBC_2.28 cnd_destroy F
|  GLIBC_2.28 cnd_init F
|  GLIBC_2.28 cnd_signal F
|  GLIBC_2.28 cnd_timedwait F
|  GLIBC_2.28 cnd_wait F
|  GLIBC_2.28 mtx_destroy F
|  GLIBC_2.28 mtx_init F
| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +2,24 @@ Parent:     4db71d2f (elf: Do not run IFUNC resolvers for LD_DEBUG=unused [BZ #24214])
| +Author:     Adhemerval Zanella <adhemerval.zanella@linaro.org>
| +AuthorDate: 2019-12-03 20:32:49 +0000
| +Commit:     Adhemerval Zanella <adhemerval.zanella@linaro.org>
| +CommitDate: 2019-12-04 14:20:50 +0000
| +
| +nptl: Add more missing placeholder abi symbol from nanosleep move
| +
| +This patch adds the missing __libpthread_version_placeholder for
| +GLIBC_2.2.6 version from the nanosleep implementation move from
| +libpthread to libc (79a547b162).

PS2, Line 11:

Done

| +
| +It also fixes the wrong compat symbol definitions added by changing
| +back the version used on vfork check and remove the unrequired

PS2, Line 14:

Done

| +__libpthread_version_placeholder added on some ABI 4f4bb489e0dd.

PS2, Line 15:

Done

| +
| +The __libpthread_version_placeholder is also refactored to make it
| +simpler to add new compat_symbols by adding a new macro
| +compat_symbol_unique which uses the compiler extension __COUNTER__
| +to generate unique strong alias to be used with compat_symbol.
| +
| +Checked with a updated-abi on the all affected abis of nanosleep

PS2, Line 22:

Done

| +move.
| +
| +Change-Id: I347a4dbdc931bb42b359456932dd1e17aa4d4078
| --- include/shlib-compat.h
| +++ include/shlib-compat.h
| @@ -59,8 +59,31 @@ # define versioned_symbol(lib, local, symbol, version) \
|  # define versioned_symbol_1(lib, local, symbol, version) \
|    versioned_symbol_2 (local, symbol, VERSION_##lib##_##version)
|  # define versioned_symbol_2(local, symbol, name) \
|    default_symbol_version (local, symbol, name)
|  
|  # define compat_symbol(lib, local, symbol, version) \
|    compat_symbol_reference (lib, local, symbol, version)
|  
| +/* This is similar to compat_symbol, but allows versioning the same symbol
| +   to multiple version without incur in multiple symbol definitions.  For

PS2, Line 68:

Done

| +   instance:
| +
| +   #if (SHLIB_COMPAT (libpthread, GLIBC_2_1_2, GLIBC_2_2))
| +   compat_symbol_unique (libc, old_foo, GLIBC_2_1_2)
| +   #endif
| +
| +   #if (SHLIB_COMPAT (libpthread, GLIBC_2_2_6, GLIBC_2_3))
| +   compat_symbol_unique (libc, old foo, GLIBC_2_2_6)
| +   #endif
| +
| +   Internally it creates an unique strong alias to the input symbol and

PS2, Line 79:

Done

| +   issue compat_symbol on the alias.  */

PS2, Line 80:

Done

| +# define __compat_symbol_unique_concat(x, y) x ## y
| +# define _compat_symbol_unique_concat(x, y) \
| +  __compat_symbol_unique_concat (x, y)
| +# define _compat_symbol_unique_alias(name) \
| +  _compat_symbol_unique_concat (name, __COUNTER__)
| +# define _compat_symbol_unique(lib, orig_name, name, version) \
| +  strong_alias (orig_name, name) \
| +  compat_symbol (libm, name, orig_name, version)
| +# define compat_symbol_unique(lib, name, version) \

-- 
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: I347a4dbdc931bb42b359456932dd1e17aa4d4078
Gerrit-Change-Number: 524
Gerrit-PatchSet: 3
Gerrit-Owner: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Gerrit-Reviewer: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Gerrit-Reviewer: Carlos O'Donell <carlos@redhat.com>
Gerrit-Comment-Date: Mon, 09 Dec 2019 18:52:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Comment-In-Reply-To: Carlos O'Donell <carlos@redhat.com>
Gerrit-MessageType: comment