Patchwork x86-64: Use __glibc_likely/__glibc_likely in dl-machine.h

login
register
mail settings
Submitter H.J. Lu
Date Feb. 5, 2018, 12:06 p.m.
Message ID <CAMe9rOrov26tv9ff_C5GPS9EjrQVF-ro59J7JUmvyhNSEpfEJw@mail.gmail.com>
Download mbox | patch
Permalink /patch/25794/
State New
Headers show

Comments

H.J. Lu - Feb. 5, 2018, 12:06 p.m.
On Fri, Feb 2, 2018 at 9:42 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> OK for master?
>
> H.J.
> ---
>         * sysdeps/x86_64/dl-machine.h (elf_machine_rela): Replace
>         __builtin_expect with __glibc_likely and __glibc_likely.
>         (elf_machine_lazy_rel): Likewise.
> ---
>  sysdeps/x86_64/dl-machine.h | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
> index f525600071..5d45ca5342 100644
> --- a/sysdeps/x86_64/dl-machine.h
> +++ b/sysdeps/x86_64/dl-machine.h
> @@ -310,10 +310,9 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
>                           : (ElfW(Addr)) sym_map->l_addr + sym->st_value);
>
>        if (sym != NULL
> -         && __builtin_expect (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC,
> -                              0)
> -         && __builtin_expect (sym->st_shndx != SHN_UNDEF, 1)
> -         && __builtin_expect (!skip_ifunc, 1))
> +         && __glibc_unlikely (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC)
> +         && __glibc_likely (sym->st_shndx != SHN_UNDEF)
> +         && __glibc_likely (!skip_ifunc))
>         {
>  # ifndef RTLD_BOOTSTRAP
>           if (sym_map != map
> @@ -500,8 +499,8 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
>             break;
>           memcpy (reloc_addr_arg, (void *) value,
>                   MIN (sym->st_size, refsym->st_size));
> -         if (__builtin_expect (sym->st_size > refsym->st_size, 0)
> -             || (__builtin_expect (sym->st_size < refsym->st_size, 0)
> +         if (__glibc_unlikely (sym->st_size > refsym->st_size)
> +             || (__glibc_unlikely (sym->st_size < refsym->st_size)
>                   && GLRO(dl_verbose)))
>             {
>               fmt = "\
> @@ -554,7 +553,8 @@ elf_machine_lazy_rel (struct link_map *map,
>    /* Check for unexpected PLT reloc type.  */
>    if (__glibc_likely (r_type == R_X86_64_JUMP_SLOT))
>      {
> -      if (__builtin_expect (map->l_mach.plt, 0) == 0)
> +      /* Prelinked has been deprecated.  */
> +      if (__glibc_likely (map->l_mach.plt == 0))
>         *reloc_addr += l_addr;
>        else
>         *reloc_addr =
> --
> 2.14.3
>

This is what I am checking in.
Szabolcs Nagy - Feb. 5, 2018, 12:11 p.m.
On 05/02/18 12:06, H.J. Lu wrote:
> -      if (__builtin_expect (map->l_mach.plt, 0) == 0)
> +      /* Prelinked has been deprecated.  */
> +      if (__glibc_likely (map->l_mach.plt == 0))

do you know about an email thread/documentation on deprecating prelinking?
(i'd like to have something to point users to when they ask for it)
Adhemerval Zanella Netto - Feb. 5, 2018, 12:22 p.m.
On 05/02/2018 10:06, H.J. Lu wrote:
> On Fri, Feb 2, 2018 at 9:42 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>> OK for master?
>>
>> H.J.
>> ---
>>         * sysdeps/x86_64/dl-machine.h (elf_machine_rela): Replace
>>         __builtin_expect with __glibc_likely and __glibc_likely.
>>         (elf_machine_lazy_rel): Likewise.
>> ---
>>  sysdeps/x86_64/dl-machine.h | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
>> index f525600071..5d45ca5342 100644
>> --- a/sysdeps/x86_64/dl-machine.h
>> +++ b/sysdeps/x86_64/dl-machine.h
>> @@ -310,10 +310,9 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
>>                           : (ElfW(Addr)) sym_map->l_addr + sym->st_value);
>>
>>        if (sym != NULL
>> -         && __builtin_expect (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC,
>> -                              0)
>> -         && __builtin_expect (sym->st_shndx != SHN_UNDEF, 1)
>> -         && __builtin_expect (!skip_ifunc, 1))
>> +         && __glibc_unlikely (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC)
>> +         && __glibc_likely (sym->st_shndx != SHN_UNDEF)
>> +         && __glibc_likely (!skip_ifunc))
>>         {
>>  # ifndef RTLD_BOOTSTRAP
>>           if (sym_map != map
>> @@ -500,8 +499,8 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
>>             break;
>>           memcpy (reloc_addr_arg, (void *) value,
>>                   MIN (sym->st_size, refsym->st_size));
>> -         if (__builtin_expect (sym->st_size > refsym->st_size, 0)
>> -             || (__builtin_expect (sym->st_size < refsym->st_size, 0)
>> +         if (__glibc_unlikely (sym->st_size > refsym->st_size)
>> +             || (__glibc_unlikely (sym->st_size < refsym->st_size)
>>                   && GLRO(dl_verbose)))
>>             {
>>               fmt = "\
>> @@ -554,7 +553,8 @@ elf_machine_lazy_rel (struct link_map *map,
>>    /* Check for unexpected PLT reloc type.  */
>>    if (__glibc_likely (r_type == R_X86_64_JUMP_SLOT))
>>      {
>> -      if (__builtin_expect (map->l_mach.plt, 0) == 0)

Thanks for changing it, these kind of construction with __builtin_expect is
confusing to read.

>> +      /* Prelinked has been deprecated.  */
>> +      if (__glibc_likely (map->l_mach.plt == 0))
>>         *reloc_addr += l_addr;
>>        else
>>         *reloc_addr =
>> --
>> 2.14.3
>>
> 
> This is what I am checking in.

Seems straightforward enough, LGTM.
H.J. Lu - Feb. 5, 2018, 12:26 p.m.
On Mon, Feb 5, 2018 at 4:11 AM, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> On 05/02/18 12:06, H.J. Lu wrote:
>>
>> -      if (__builtin_expect (map->l_mach.plt, 0) == 0)
>> +      /* Prelinked has been deprecated.  */
>> +      if (__glibc_likely (map->l_mach.plt == 0))
>
>
> do you know about an email thread/documentation on deprecating prelinking?
> (i'd like to have something to point users to when they ask for it)

https://lwn.net/Articles/341244/
https://pagure.io/fesco/issue/1183
https://stackoverflow.com/questions/2040256/is-prelinking-on-linux-for-shared-libraries-any-good-proven
https://serverfault.com/questions/39500/is-there-a-point-in-using-prelink-anymore

prelink package has been removed from Fedora a while back.

Patch

From 84fb880a14b55e3212826d9b03b78a372306dfa6 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 2 Feb 2018 09:40:02 -0800
Subject: [PATCH] x86-64: Use __glibc_likely/__glibc_likely in dl-machine.h

The differences in elf/dl-reloc.os are

--- before    	2018-02-05 03:52:32.803125207 -0800
+++ after     	2018-02-05 03:52:14.913711879 -0800
@@ -1129,7 +1129,7 @@  _dl_relocate_object:
 	leaq	__PRETTY_FUNCTION__.9767(%rip), %rcx
 	leaq	.LC11(%rip), %rsi
 	leaq	.LC12(%rip), %rdi
-	movl	$540, %edx
+	movl	$539, %edx
 	call	__GI___assert_fail
 	.p2align 4,,10
 	.p2align 3

	* sysdeps/x86_64/dl-machine.h (elf_machine_rela): Replace
	__builtin_expect with __glibc_likely and __glibc_likely.
	(elf_machine_lazy_rel): Likewise.
---
 sysdeps/x86_64/dl-machine.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
index f525600071..5d45ca5342 100644
--- a/sysdeps/x86_64/dl-machine.h
+++ b/sysdeps/x86_64/dl-machine.h
@@ -310,10 +310,9 @@  elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
 			  : (ElfW(Addr)) sym_map->l_addr + sym->st_value);
 
       if (sym != NULL
-	  && __builtin_expect (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC,
-			       0)
-	  && __builtin_expect (sym->st_shndx != SHN_UNDEF, 1)
-	  && __builtin_expect (!skip_ifunc, 1))
+	  && __glibc_unlikely (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC)
+	  && __glibc_likely (sym->st_shndx != SHN_UNDEF)
+	  && __glibc_likely (!skip_ifunc))
 	{
 # ifndef RTLD_BOOTSTRAP
 	  if (sym_map != map
@@ -500,8 +499,8 @@  elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
 	    break;
 	  memcpy (reloc_addr_arg, (void *) value,
 		  MIN (sym->st_size, refsym->st_size));
-	  if (__builtin_expect (sym->st_size > refsym->st_size, 0)
-	      || (__builtin_expect (sym->st_size < refsym->st_size, 0)
+	  if (__glibc_unlikely (sym->st_size > refsym->st_size)
+	      || (__glibc_unlikely (sym->st_size < refsym->st_size)
 		  && GLRO(dl_verbose)))
 	    {
 	      fmt = "\
@@ -554,7 +553,8 @@  elf_machine_lazy_rel (struct link_map *map,
   /* Check for unexpected PLT reloc type.  */
   if (__glibc_likely (r_type == R_X86_64_JUMP_SLOT))
     {
-      if (__builtin_expect (map->l_mach.plt, 0) == 0)
+      /* Prelinked has been deprecated.  */
+      if (__glibc_likely (map->l_mach.plt == 0))
 	*reloc_addr += l_addr;
       else
 	*reloc_addr =
-- 
2.14.3