[review v2] 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 2: Code-Review+1

(7 comments)

Looks good, please post a new version with required changes and double check I understood the COUNTER code correctly.

| --- /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:

OK.

| +
| +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:

s/remove the unrequired/removing the/g

| +__libpthread_version_placeholder added on some ABI 4f4bb489e0dd.

PS2, Line 15:

s/added on some ABI 4f4bb489e0dd./added for some ABIs
(4f4bb489e0dd)./g

| +
| +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:

s/of/of the/g

| +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:

s/incur in/having/g

| +   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:

s/an/a/g

| +   issue compat_symbol on the alias.  */

PS2, Line 80:

s/issue/creates one/g

Please tell me if I understood that correctly.

| +# 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: 2
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: Fri, 06 Dec 2019 18:51:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment