[1/9] Introduce <elf_machine_sym_no_match.h>

  From: Florian Weimer <fweimer@redhat.com>

MIPS needs to ignore certain existing symbols during symbol lookup.
The old scheme uses the ELF_MACHINE_SYM_NO_MATCH macro, with an
inline function, within its own header, with a sysdeps override for
MIPS.  This allows re-use of the function from another file (without
having to include <dl-machine.h> or providing the default definition
for ELF_MACHINE_SYM_NO_MATCH).

Built with build-many-glibcs.py, with manual verification that
sysdeps/mips/elf_machine_sym_no_match.h is picked up on MIPS.  Tested
on aarch64-linux-gnu, i686-linux-gnu, powerpc64-linux-gnu,
s390x-linux-gnu, x86_64-linux-gnu.
---
 elf/dl-lookup.c                         | 10 ++----
 elf/elf_machine_sym_no_match.h          | 34 +++++++++++++++++++
 sysdeps/mips/dl-machine.h               | 15 ---------
 sysdeps/mips/elf_machine_sym_no_match.h | 43 +++++++++++++++++++++++++
 4 files changed, 79 insertions(+), 23 deletions(-)
 create mode 100644 elf/elf_machine_sym_no_match.h
 create mode 100644 sysdeps/mips/elf_machine_sym_no_match.h
  

On 3/26/20 11:56 AM, Mathieu Desnoyers wrote:
> From: Florian Weimer <fweimer@redhat.com>
> 
> MIPS needs to ignore certain existing symbols during symbol lookup.
> The old scheme uses the ELF_MACHINE_SYM_NO_MATCH macro, with an
> inline function, within its own header, with a sysdeps override for
> MIPS.  This allows re-use of the function from another file (without
> having to include <dl-machine.h> or providing the default definition
> for ELF_MACHINE_SYM_NO_MATCH).
> 
> Built with build-many-glibcs.py, with manual verification that
> sysdeps/mips/elf_machine_sym_no_match.h is picked up on MIPS.  Tested
> on aarch64-linux-gnu, i686-linux-gnu, powerpc64-linux-gnu,
> s390x-linux-gnu, x86_64-linux-gnu.

The refactoring from a macro to a static inline function looks good to
me. There isn't anything that changes.

This patch needs squashing with the second patch in the series.

Technically speaking it's ready to be committed, but I want to see the
squashed patch before ACK'ing it.

> ---
>  elf/dl-lookup.c                         | 10 ++----
>  elf/elf_machine_sym_no_match.h          | 34 +++++++++++++++++++
>  sysdeps/mips/dl-machine.h               | 15 ---------
>  sysdeps/mips/elf_machine_sym_no_match.h | 43 +++++++++++++++++++++++++
>  4 files changed, 79 insertions(+), 23 deletions(-)
>  create mode 100644 elf/elf_machine_sym_no_match.h
>  create mode 100644 sysdeps/mips/elf_machine_sym_no_match.h
> 
> diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c
> index 12a229f06c..807f3ea9b6 100644
> --- a/elf/dl-lookup.c
> +++ b/elf/dl-lookup.c
> @@ -28,18 +28,12 @@
>  #include <libc-lock.h>
>  #include <tls.h>
>  #include <atomic.h>
> +#include <elf_machine_sym_no_match.h>

OK.

>  
>  #include <assert.h>
>  
> -/* Return nonzero if check_match should consider SYM to fail to match a
> -   symbol reference for some machine-specific reason.  */
> -#ifndef ELF_MACHINE_SYM_NO_MATCH
> -# define ELF_MACHINE_SYM_NO_MATCH(sym) 0
> -#endif
> -
>  #define VERSTAG(tag)	(DT_NUM + DT_THISPROCNUM + DT_VERSIONTAGIDX (tag))
>  
> -
>  struct sym_val
>    {
>      const ElfW(Sym) *s;
> @@ -78,7 +72,7 @@ check_match (const char *const undef_name,
>    if (__glibc_unlikely ((sym->st_value == 0 /* No value.  */
>  			 && sym->st_shndx != SHN_ABS
>  			 && stt != STT_TLS)
> -			|| ELF_MACHINE_SYM_NO_MATCH (sym)
> +			|| elf_machine_sym_no_match (sym)

OK. You are my hero every time you kill a macro and make an static inline function.

>  			|| (type_class & (sym->st_shndx == SHN_UNDEF))))
>      return NULL;
>  
> diff --git a/elf/elf_machine_sym_no_match.h b/elf/elf_machine_sym_no_match.h
> new file mode 100644
> index 0000000000..6e299e5ee8
> --- /dev/null
> +++ b/elf/elf_machine_sym_no_match.h
> @@ -0,0 +1,34 @@
> +/* Function to ignore certain symbol matches for machine-specific reasons.
> +   Copyright (C) 2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _ELF_MACHINE_SYM_NO_MATCH_H
> +#define _ELF_MACHINE_SYM_NO_MATCH_H
> +
> +#include <link.h>
> +#include <stdbool.h>
> +
> +/* This can be customized to ignore certain symbols during lookup in
> +   case there are machine-specific rules to disregard some
> +   symbols.  */
> +static inline bool
> +elf_machine_sym_no_match (const ElfW(Sym) *sym)
> +{
> +  return false;
> +}


OK.

> +
> +#endif /* _ELF_MACHINE_SYM_NO_MATCH_H */
> diff --git a/sysdeps/mips/dl-machine.h b/sysdeps/mips/dl-machine.h
> index 06021ea9ab..e3b11a4f21 100644
> --- a/sysdeps/mips/dl-machine.h
> +++ b/sysdeps/mips/dl-machine.h
> @@ -467,21 +467,6 @@ elf_machine_plt_value (struct link_map *map, const ElfW(Rel) *reloc,
>    return value;
>  }
>  
> -/* The semantics of zero/non-zero values of undefined symbols differs
> -   depending on whether the non-PIC ABI is in use.  Under the non-PIC
> -   ABI, a non-zero value indicates that there is an address reference
> -   to the symbol and thus it must always be resolved (except when
> -   resolving a jump slot relocation) to the PLT entry whose address is
> -   provided as the symbol's value; a zero value indicates that this
> -   canonical-address behaviour is not required.  Yet under the classic
> -   MIPS psABI, a zero value indicates that there is an address
> -   reference to the function and the dynamic linker must resolve the
> -   symbol immediately upon loading.  To avoid conflict, symbols for
> -   which the dynamic linker must assume the non-PIC ABI semantics are
> -   marked with the STO_MIPS_PLT flag.  */
> -#define ELF_MACHINE_SYM_NO_MATCH(sym) \
> -  ((sym)->st_shndx == SHN_UNDEF && !((sym)->st_other & STO_MIPS_PLT))
> -

OK.

>  #endif /* !dl_machine_h */
>  
>  #ifdef RESOLVE_MAP
> diff --git a/sysdeps/mips/elf_machine_sym_no_match.h b/sysdeps/mips/elf_machine_sym_no_match.h
> new file mode 100644
> index 0000000000..f2be74caaf
> --- /dev/null
> +++ b/sysdeps/mips/elf_machine_sym_no_match.h
> @@ -0,0 +1,43 @@
> +/* MIPS-specific handling of undefined symbols.
> +   Copyright (C) 2008-2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _ELF_MACHINE_SYM_NO_MATCH_H
> +#define _ELF_MACHINE_SYM_NO_MATCH_H
> +
> +#include <link.h>
> +#include <stdbool.h>
> +
> +/* The semantics of zero/non-zero values of undefined symbols differs
> +   depending on whether the non-PIC ABI is in use.  Under the non-PIC
> +   ABI, a non-zero value indicates that there is an address reference
> +   to the symbol and thus it must always be resolved (except when
> +   resolving a jump slot relocation) to the PLT entry whose address is
> +   provided as the symbol's value; a zero value indicates that this
> +   canonical-address behaviour is not required.  Yet under the classic
> +   MIPS psABI, a zero value indicates that there is an address
> +   reference to the function and the dynamic linker must resolve the
> +   symbol immediately upon loading.  To avoid conflict, symbols for
> +   which the dynamic linker must assume the non-PIC ABI semantics are
> +   marked with the STO_MIPS_PLT flag.  */
> +static inline bool
> +elf_machine_sym_no_match (const ElfW(Sym) *sym)
> +{
> +  return sym->st_shndx == SHN_UNDEF && !(sym->st_other & STO_MIPS_PLT);
> +}
> +
> +#endif /* _ELF_MACHINE_SYM_NO_MATCH_H */
> 

OK. Conditional matches original code.
  

* Carlos O'Donell via Libc-alpha:

> On 3/26/20 11:56 AM, Mathieu Desnoyers wrote:
>> From: Florian Weimer <fweimer@redhat.com>
>> 
>> MIPS needs to ignore certain existing symbols during symbol lookup.
>> The old scheme uses the ELF_MACHINE_SYM_NO_MATCH macro, with an
>> inline function, within its own header, with a sysdeps override for
>> MIPS.  This allows re-use of the function from another file (without
>> having to include <dl-machine.h> or providing the default definition
>> for ELF_MACHINE_SYM_NO_MATCH).
>> 
>> Built with build-many-glibcs.py, with manual verification that
>> sysdeps/mips/elf_machine_sym_no_match.h is picked up on MIPS.  Tested
>> on aarch64-linux-gnu, i686-linux-gnu, powerpc64-linux-gnu,
>> s390x-linux-gnu, x86_64-linux-gnu.
>
> The refactoring from a macro to a static inline function looks good to
> me. There isn't anything that changes.
>
> This patch needs squashing with the second patch in the series.
>
> Technically speaking it's ready to be committed, but I want to see the
> squashed patch before ACK'ing it.

Merged and updated patch below.  I've fixed the copyright years.

I did a mips-linux-gnu build, and the sysdeps override still appears
to be effective:

…/bmg/default/src/glibc/elf/dl-lookup-direct.c:36
  64:   11200031        beqz    t1,12c <check_match+0x12c>
  68:   3043000f        andi    v1,v0,0xf
  6c:   9206000d        lbu     a2,13(s0)
elf_machine_sym_no_match():
…/bmg/default/src/glibc/elf/../sysdeps/mips/elf_machine_sym_no_match.h:40
  70:   15000003        bnez    t0,80 <check_match+0x80>
  74:   30c60008        andi    a2,a2,0x8
  78:   10c00021        beqz    a2,100 <check_match+0x100>
  7c:   00000000        nop
check_match():
…/bmg/default/src/glibc/elf/dl-lookup-direct.c:48
  80:   24020467        li      v0,1127
  84:   00621007        srav    v0,v0,v1

8<------------------------------------------------------------------8<
Subject: [PATCH] elf: Introduce <elf_machine_sym_no_match.h>

MIPS needs to ignore certain existing symbols during symbol lookup.
The old scheme uses the ELF_MACHINE_SYM_NO_MATCH macro, with an
inline function, within its own header, with a sysdeps override for
MIPS.  This allows re-use of the function from another file (without
having to include <dl-machine.h> or providing the default definition
for ELF_MACHINE_SYM_NO_MATCH).

-----
 elf/dl-lookup.c                            | 10 ++-----
 sysdeps/generic/elf_machine_sym_no_match.h | 34 +++++++++++++++++++++++
 sysdeps/mips/dl-machine.h                  | 15 -----------
 sysdeps/mips/elf_machine_sym_no_match.h    | 43 ++++++++++++++++++++++++++++++
 4 files changed, 79 insertions(+), 23 deletions(-)

diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c
index 12a229f06c..807f3ea9b6 100644
--- a/elf/dl-lookup.c
+++ b/elf/dl-lookup.c
@@ -28,18 +28,12 @@
 #include <libc-lock.h>
 #include <tls.h>
 #include <atomic.h>
+#include <elf_machine_sym_no_match.h>
 
 #include <assert.h>
 
-/* Return nonzero if check_match should consider SYM to fail to match a
-   symbol reference for some machine-specific reason.  */
-#ifndef ELF_MACHINE_SYM_NO_MATCH
-# define ELF_MACHINE_SYM_NO_MATCH(sym) 0
-#endif
-
 #define VERSTAG(tag)	(DT_NUM + DT_THISPROCNUM + DT_VERSIONTAGIDX (tag))
 
-
 struct sym_val
   {
     const ElfW(Sym) *s;
@@ -78,7 +72,7 @@ check_match (const char *const undef_name,
   if (__glibc_unlikely ((sym->st_value == 0 /* No value.  */
 			 && sym->st_shndx != SHN_ABS
 			 && stt != STT_TLS)
-			|| ELF_MACHINE_SYM_NO_MATCH (sym)
+			|| elf_machine_sym_no_match (sym)
 			|| (type_class & (sym->st_shndx == SHN_UNDEF))))
     return NULL;
 
diff --git a/sysdeps/generic/elf_machine_sym_no_match.h b/sysdeps/generic/elf_machine_sym_no_match.h
new file mode 100644
index 0000000000..27155de4c0
--- /dev/null
+++ b/sysdeps/generic/elf_machine_sym_no_match.h
@@ -0,0 +1,34 @@
+/* Function to ignore certain symbol matches for machine-specific reasons.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _ELF_MACHINE_SYM_NO_MATCH_H
+#define _ELF_MACHINE_SYM_NO_MATCH_H
+
+#include <link.h>
+#include <stdbool.h>
+
+/* This can be customized to ignore certain symbols during lookup in
+   case there are machine-specific rules to disregard some
+   symbols.  */
+static inline bool
+elf_machine_sym_no_match (const ElfW(Sym) *sym)
+{
+  return false;
+}
+
+#endif /* _ELF_MACHINE_SYM_NO_MATCH_H */
diff --git a/sysdeps/mips/dl-machine.h b/sysdeps/mips/dl-machine.h
index 06021ea9ab..e3b11a4f21 100644
--- a/sysdeps/mips/dl-machine.h
+++ b/sysdeps/mips/dl-machine.h
@@ -467,21 +467,6 @@ elf_machine_plt_value (struct link_map *map, const ElfW(Rel) *reloc,
   return value;
 }
 
-/* The semantics of zero/non-zero values of undefined symbols differs
-   depending on whether the non-PIC ABI is in use.  Under the non-PIC
-   ABI, a non-zero value indicates that there is an address reference
-   to the symbol and thus it must always be resolved (except when
-   resolving a jump slot relocation) to the PLT entry whose address is
-   provided as the symbol's value; a zero value indicates that this
-   canonical-address behaviour is not required.  Yet under the classic
-   MIPS psABI, a zero value indicates that there is an address
-   reference to the function and the dynamic linker must resolve the
-   symbol immediately upon loading.  To avoid conflict, symbols for
-   which the dynamic linker must assume the non-PIC ABI semantics are
-   marked with the STO_MIPS_PLT flag.  */
-#define ELF_MACHINE_SYM_NO_MATCH(sym) \
-  ((sym)->st_shndx == SHN_UNDEF && !((sym)->st_other & STO_MIPS_PLT))
-
 #endif /* !dl_machine_h */
 
 #ifdef RESOLVE_MAP
diff --git a/sysdeps/mips/elf_machine_sym_no_match.h b/sysdeps/mips/elf_machine_sym_no_match.h
new file mode 100644
index 0000000000..9d09e5fa2d
--- /dev/null
+++ b/sysdeps/mips/elf_machine_sym_no_match.h
@@ -0,0 +1,43 @@
+/* MIPS-specific handling of undefined symbols.
+   Copyright (C) 2008-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _ELF_MACHINE_SYM_NO_MATCH_H
+#define _ELF_MACHINE_SYM_NO_MATCH_H
+
+#include <link.h>
+#include <stdbool.h>
+
+/* The semantics of zero/non-zero values of undefined symbols differs
+   depending on whether the non-PIC ABI is in use.  Under the non-PIC
+   ABI, a non-zero value indicates that there is an address reference
+   to the symbol and thus it must always be resolved (except when
+   resolving a jump slot relocation) to the PLT entry whose address is
+   provided as the symbol's value; a zero value indicates that this
+   canonical-address behaviour is not required.  Yet under the classic
+   MIPS psABI, a zero value indicates that there is an address
+   reference to the function and the dynamic linker must resolve the
+   symbol immediately upon loading.  To avoid conflict, symbols for
+   which the dynamic linker must assume the non-PIC ABI semantics are
+   marked with the STO_MIPS_PLT flag.  */
+static inline bool
+elf_machine_sym_no_match (const ElfW(Sym) *sym)
+{
+  return sym->st_shndx == SHN_UNDEF && !(sym->st_other & STO_MIPS_PLT);
+}
+
+#endif /* _ELF_MACHINE_SYM_NO_MATCH_H */