Replace most __builtin_expect's with __glibc_{un}likely
diff mbox

Message ID ye6qha6k7ky8.fsf@elbrus2.mtv.corp.google.com
State New
Headers show

Commit Message

Paul Pluzhnikov March 26, 2014, 8:40 p.m. UTC
Greetings,

As promised here: https://sourceware.org/ml/libc-alpha/2014-03/msg00676.html
attached patch converts most of __builtin_expect's into
__glibc_{un}likely in elf/dl-load.c

This patch produces identical dl-load.{o,os} on Linux/x86_64 (except a
few line numbers change).

The remaining 7 __builtin_expect's caused the object file to change in
unexpected ways, which means that either I didn't do them correctly, or
there is something else at play. I'll try to do them next.

Thanks,

--

2013-03-26  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* elf/dl-load.c: Convert __builtin_expect into
        __glibc_{un}likely.

Comments

Ondřej Bílka March 26, 2014, 8:50 p.m. UTC | #1
On Wed, Mar 26, 2014 at 01:40:15PM -0700, Paul Pluzhnikov wrote:
> Greetings,
> 
> As promised here: https://sourceware.org/ml/libc-alpha/2014-03/msg00676.html
> attached patch converts most of __builtin_expect's into
> __glibc_{un}likely in elf/dl-load.c
> 
> This patch produces identical dl-load.{o,os} on Linux/x86_64 (except a
> few line numbers change).
> 
Looks ok, I have in my todo list to make coccinele replace all if (expects)
in source by likely/unlikely.

> The remaining 7 __builtin_expect's caused the object file to change in
> unexpected ways, which means that either I didn't do them correctly, or
> there is something else at play. I'll try to do them next.
>
That could be artefact how gcc optimizes things, what happens if you use
different compiler? 

Note that when rewriting hints that you give may not be same. For
example if 

> -       if (__builtin_expect (fd, 0) == -1)
> +       if (__glibc_unlikely (fd == -1))

would be followed by

if (fd == 0)
  foo ();

then gcc could assume that it is likely in former case but not latter
case.


 
> Thanks,
> 
> --
> 
> 2013-03-26  Paul Pluzhnikov  <ppluzhnikov@google.com>
> 
> 	* elf/dl-load.c: Convert __builtin_expect into
>         __glibc_{un}likely.
> 
> 
> 
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 9455df2..fabe025 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -280,7 +280,7 @@ is_dst (const char *start, const char *name, const char *str,
>  	   && (!is_path || name[len] != ':'))
>      return 0;
>  
> -  if (__builtin_expect (secure, 0)
> +  if (__glibc_unlikely (secure)
>        && ((name[len] != '\0' && name[len] != '/'
>  	   && (!is_path || name[len] != ':'))
>  	  || (name != start + 1 && (!is_path || name[-2] != ':'))))
> @@ -381,7 +381,7 @@ _dl_dst_substitute (struct link_map *l, const char *name, char *result,
>  	      /* In SUID/SGID programs, after $ORIGIN expansion the
>  		 normalized path must be rooted in one of the trusted
>  		 directories.  */
> -	      if (__builtin_expect (check_for_trusted, false)
> +	      if (__glibc_unlikely (check_for_trusted)
>  		  && !is_trusted_path_normalize (last_elem, wp - last_elem))
>  		wp = last_elem;
>  	      else
> @@ -395,7 +395,7 @@ _dl_dst_substitute (struct link_map *l, const char *name, char *result,
>  
>    /* In SUID/SGID programs, after $ORIGIN expansion the normalized
>       path must be rooted in one of the trusted directories.  */
> -  if (__builtin_expect (check_for_trusted, false)
> +  if (__glibc_unlikely (check_for_trusted)
>        && !is_trusted_path_normalize (last_elem, wp - last_elem))
>      wp = last_elem;
>  
> @@ -425,7 +425,7 @@ expand_dynamic_string_token (struct link_map *l, const char *s, int is_path)
>    cnt = DL_DST_COUNT (s, is_path);
>  
>    /* If we do not have to replace anything simply copy the string.  */
> -  if (__builtin_expect (cnt, 0) == 0)
> +  if (__glibc_likely (cnt == 0))
>      return local_strdup (s);
>  
>    /* Determine the length of the substituted string.  */
> @@ -513,7 +513,7 @@ fillin_rpath (char *rpath, struct r_search_path_elem **result, const char *sep,
>  	cp[len++] = '/';
>  
>        /* Make sure we don't use untrusted directories if we run SUID.  */
> -      if (__builtin_expect (check_trusted, 0) && !is_trusted_path (cp, len))
> +      if (__glibc_unlikely (check_trusted) && !is_trusted_path (cp, len))
>  	{
>  	  free (to_free);
>  	  continue;
> @@ -604,7 +604,7 @@ decompose_rpath (struct r_search_path_struct *sps,
>  
>    /* First see whether we must forget the RUNPATH and RPATH from this
>       object.  */
> -  if (__builtin_expect (GLRO(dl_inhibit_rpath) != NULL, 0)
> +  if (__glibc_unlikely (GLRO(dl_inhibit_rpath) != NULL)
>        && !INTUSE(__libc_enable_secure))
>      {
>        const char *inhp = GLRO(dl_inhibit_rpath);
> @@ -964,7 +964,7 @@ _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
>  #ifdef SHARED
>    /* When loading into a namespace other than the base one we must
>       avoid loading ld.so since there can only be one copy.  Ever.  */
> -  if (__builtin_expect (nsid != LM_ID_BASE, 0)
> +  if (__glibc_unlikely (nsid != LM_ID_BASE)
>        && ((st.st_ino == GL(dl_rtld_map).l_ino
>  	   && st.st_dev == GL(dl_rtld_map).l_dev)
>  	  || _dl_name_match_p (name, &GL(dl_rtld_map))))
> @@ -1026,7 +1026,7 @@ _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
>  #ifdef SHARED
>        /* Auditing checkpoint: we are going to add new objects.  */
>        if ((mode & __RTLD_AUDIT) == 0
> -	  && __builtin_expect (GLRO(dl_naudit) > 0, 0))
> +	  && __glibc_unlikely (GLRO(dl_naudit) > 0))
>  	{
>  	  struct link_map *head = GL(dl_ns)[nsid]._ns_loaded;
>  	  /* Do not call the functions for any auditing object.  */
> @@ -1124,14 +1124,13 @@ _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
>  	case PT_LOAD:
>  	  /* A load command tells us to map in part of the file.
>  	     We record the load commands and process them all later.  */
> -	  if (__builtin_expect ((ph->p_align & (GLRO(dl_pagesize) - 1)) != 0,
> -				0))
> +	  if (__glibc_unlikely ((ph->p_align & (GLRO(dl_pagesize) - 1)) != 0))
>  	    {
>  	      errstring = N_("ELF load command alignment not page-aligned");
>  	      goto call_lose;
>  	    }
> -	  if (__builtin_expect (((ph->p_vaddr - ph->p_offset)
> -				 & (ph->p_align - 1)) != 0, 0))
> +	  if (__glibc_unlikely (((ph->p_vaddr - ph->p_offset)
> +				 & (ph->p_align - 1)) != 0))
>  	    {
>  	      errstring
>  		= N_("ELF load command address/offset not properly aligned");
> @@ -1184,10 +1183,10 @@ _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
>  
>  	  /* If not loading the initial set of shared libraries,
>  	     check whether we should permit loading a TLS segment.  */
> -	  if (__builtin_expect (l->l_type == lt_library, 1)
> +	  if (__glibc_likely (l->l_type == lt_library)
>  	      /* If GL(dl_tls_dtv_slotinfo_list) == NULL, then rtld.c did
>  		 not set up TLS data structures, so don't use them now.  */
> -	      || __builtin_expect (GL(dl_tls_dtv_slotinfo_list) != NULL, 1))
> +	      || __glibc_likely (GL(dl_tls_dtv_slotinfo_list) != NULL))
>  	    {
>  	      /* Assign the next available module ID.  */
>  	      l->l_tls_modid = _dl_next_tls_modid ();
> @@ -1214,9 +1213,8 @@ _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
>  
>  	      /* The first call allocates TLS bookkeeping data structures.
>  		 Then we allocate the TCB for the initial thread.  */
> -	      if (__builtin_expect (_dl_tls_setup (), 0)
> -		  || __builtin_expect ((tcb = _dl_allocate_tls (NULL)) == NULL,
> -				       0))
> +	      if (__glibc_unlikely (_dl_tls_setup ())
> +		  || __glibc_unlikely ((tcb = _dl_allocate_tls (NULL)) == NULL))
>  		{
>  		  errval = ENOMEM;
>  		  errstring = N_("\
> @@ -1430,7 +1428,7 @@ cannot allocate TLS data structures for initial thread");
>  
>    /* Make sure we are not dlopen'ing an object that has the
>       DF_1_NOOPEN flag set.  */
> -  if (__builtin_expect (l->l_flags_1 & DF_1_NOOPEN, 0)
> +  if (__glibc_unlikely (l->l_flags_1 & DF_1_NOOPEN)
>        && (mode & __RTLD_DLOPEN))
>      {
>        /* We are not supposed to load this object.  Free all resources.  */
> @@ -1469,8 +1467,8 @@ cannot allocate TLS data structures for initial thread");
>  
>    if (__glibc_unlikely ((stack_flags &~ GL(dl_stack_flags)) & PF_X))
>      {
> -      if (__builtin_expect (__check_caller (RETURN_ADDRESS (0), allow_ldso),
> -			    0) != 0)
> +
> +      if (__glibc_unlikely (__check_caller (RETURN_ADDRESS (0), allow_ldso) != 0))
>  	{
>  	  errstring = N_("invalid caller");
>  	  goto call_lose;
> @@ -1560,7 +1558,7 @@ cannot enable executable stack as shared object requires");
>    /* If this object has DT_SYMBOLIC set modify now its scope.  We don't
>       have to do this for the main map.  */
>    if ((mode & RTLD_DEEPBIND) == 0
> -      && __builtin_expect (l->l_info[DT_SYMBOLIC] != NULL, 0)
> +      && __glibc_unlikely (l->l_info[DT_SYMBOLIC] != NULL)
>        && &l->l_searchlist != l->l_scope[0])
>      {
>        /* Create an appropriate searchlist.  It contains only this map.
> @@ -1586,7 +1584,7 @@ cannot enable executable stack as shared object requires");
>  
>    /* When we profile the SONAME might be needed for something else but
>       loading.  Add it right away.  */
> -  if (__builtin_expect (GLRO(dl_profile) != NULL, 0)
> +  if (__glibc_unlikely (GLRO(dl_profile) != NULL)
>        && l->l_info[DT_SONAME] != NULL)
>      add_name_to_object (l, ((const char *) D_PTR (l, l_info[DT_STRTAB])
>  			    + l->l_info[DT_SONAME]->d_un.d_val));
> @@ -1600,7 +1598,7 @@ cannot enable executable stack as shared object requires");
>  
>  #ifdef SHARED
>    /* Auditing checkpoint: we have a new object.  */
> -  if (__builtin_expect (GLRO(dl_naudit) > 0, 0)
> +  if (__glibc_unlikely (GLRO(dl_naudit) > 0)
>        && !GL(dl_ns)[l->l_ns]._ns_loaded->l_auditing)
>      {
>        struct audit_ifaces *afct = GLRO(dl_audit);
> @@ -1704,7 +1702,7 @@ open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
>  
>  #ifdef SHARED
>    /* Give the auditing libraries a chance.  */
> -  if (__builtin_expect (GLRO(dl_naudit) > 0, 0) && whatcode != 0
> +  if (__glibc_unlikely (GLRO(dl_naudit) > 0) && whatcode != 0
>        && loader->l_auditing == 0)
>      {
>        struct audit_ifaces *afct = GLRO(dl_audit);
> @@ -1748,7 +1746,7 @@ open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
>  	    break;
>  	  fbp->len += retlen;
>  	}
> -      while (__builtin_expect (fbp->len < sizeof (ElfW(Ehdr)), 0));
> +      while (__glibc_unlikely (fbp->len < sizeof (ElfW(Ehdr))));
>  
>        /* This is where the ELF header is loaded.  */
>        ehdr = (ElfW(Ehdr) *) fbp->buf;
> @@ -1830,7 +1828,7 @@ open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
>  	  goto call_lose;
>  	}
>  
> -      if (__builtin_expect (ehdr->e_version, EV_CURRENT) != EV_CURRENT)
> +      if (__glibc_unlikely (ehdr->e_version != EV_CURRENT))
>  	{
>  	  errstring = N_("ELF file version does not match current one");
>  	  goto call_lose;
> @@ -1854,8 +1852,8 @@ open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
>  	  errstring = N_("cannot dynamically load executable");
>  	  goto call_lose;
>  	}
> -      else if (__builtin_expect (ehdr->e_phentsize, sizeof (ElfW(Phdr)))
> -	       != sizeof (ElfW(Phdr)))
> +      else if (__glibc_unlikely (ehdr->e_phentsize != sizeof (ElfW(Phdr))))
> +
>  	{
>  	  errstring = N_("ELF file's phentsize not the expected size");
>  	  goto call_lose;
> @@ -1967,7 +1965,7 @@ open_path (const char *name, size_t namelen, int mode,
>  
>        /* If we are debugging the search for libraries print the path
>  	 now if it hasn't happened now.  */
> -      if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_LIBS, 0)
> +      if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_LIBS)
>  	  && current_what != this_dir->what)
>  	{
>  	  current_what = this_dir->what;
> @@ -2021,7 +2019,7 @@ open_path (const char *name, size_t namelen, int mode,
>  	  /* Remember whether we found any existing directory.  */
>  	  here_any |= this_dir->status[cnt] != nonexisting;
>  
> -	  if (fd != -1 && __builtin_expect (mode & __RTLD_SECURE, 0)
> +	  if (fd != -1 && __glibc_unlikely (mode & __RTLD_SECURE)
>  	      && INTUSE(__libc_enable_secure))
>  	    {
>  	      /* This is an extra security effort to make sure nobody can
> @@ -2108,14 +2106,14 @@ _dl_map_object (struct link_map *loader, const char *name,
>        /* If the requested name matches the soname of a loaded object,
>  	 use that object.  Elide this check for names that have not
>  	 yet been opened.  */
> -      if (__builtin_expect (l->l_faked, 0) != 0
> +      if (__glibc_unlikely (l->l_faked != 0)
>  	  || __builtin_expect (l->l_removed, 0) != 0)
>  	continue;
>        if (!_dl_name_match_p (name, l))
>  	{
>  	  const char *soname;
>  
> -	  if (__builtin_expect (l->l_soname_added, 1)
> +	  if (__glibc_likely (l->l_soname_added)
>  	      || l->l_info[DT_SONAME] == NULL)
>  	    continue;
>  
> @@ -2134,7 +2132,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>      }
>  
>    /* Display information if we are debugging.  */
> -  if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_FILES, 0)
> +  if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES)
>        && loader != NULL)
>      _dl_debug_printf ((mode & __RTLD_CALLMAP) == 0
>  		      ? "\nfile=%s [%lu];  needed by %s [%lu]\n"
> @@ -2144,7 +2142,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>  #ifdef SHARED
>    /* Give the auditing libraries a chance to change the name before we
>       try anything.  */
> -  if (__builtin_expect (GLRO(dl_naudit) > 0, 0)
> +  if (__glibc_unlikely (GLRO(dl_naudit) > 0)
>        && (loader == NULL || loader->l_auditing == 0))
>      {
>        struct audit_ifaces *afct = GLRO(dl_audit);
> @@ -2236,7 +2234,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>        if (fd == -1
>  	  && (__builtin_expect (! (mode & __RTLD_SECURE), 1)
>  	      || ! INTUSE(__libc_enable_secure))
> -	  && __builtin_expect (GLRO(dl_inhibit_cache) == 0, 1))
> +	  && __glibc_likely (GLRO(dl_inhibit_cache) == 0))
>  	{
>  	  /* Check the list of libraries in the file /etc/ld.so.cache,
>  	     for compatibility with Linux's ldconfig program.  */
> @@ -2254,7 +2252,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>  
>  	      /* If the loader has the DF_1_NODEFLIB flag set we must not
>  		 use a cache entry from any of these directories.  */
> -	      if (__builtin_expect (l->l_flags_1 & DF_1_NODEFLIB, 0))
> +	      if (__glibc_unlikely (l->l_flags_1 & DF_1_NODEFLIB))
>  		{
>  		  const char *dirp = system_dirs;
>  		  unsigned int cnt = 0;
> @@ -2297,7 +2295,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>        /* Finally, try the default path.  */
>        if (fd == -1
>  	  && ((l = loader ?: GL(dl_ns)[nsid]._ns_loaded) == NULL
> -	      || __builtin_expect (!(l->l_flags_1 & DF_1_NODEFLIB), 1))
> +	      || __glibc_likely (!(l->l_flags_1 & DF_1_NODEFLIB)))
>  	  && rtld_search_dirs.dirs != (void *) -1)
>  	fd = open_path (name, namelen, mode, &rtld_search_dirs,
>  			&realname, &fb, l, LA_SER_DEFAULT, &found_other_class);
> @@ -2319,7 +2317,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>  	  fd = open_verify (realname, &fb,
>  			    loader ?: GL(dl_ns)[nsid]._ns_loaded, 0, mode,
>  			    &found_other_class, true);
> -	  if (__builtin_expect (fd, 0) == -1)
> +	  if (__glibc_unlikely (fd == -1))
>  	    free (realname);
>  	}
>      }
> @@ -2333,10 +2331,10 @@ _dl_map_object (struct link_map *loader, const char *name,
>    if (mode & __RTLD_CALLMAP)
>      loader = NULL;
>  
> -  if (__builtin_expect (fd, 0) == -1)
> +  if (__glibc_unlikely (fd == -1))
>      {
>        if (trace_mode
> -	  && __builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_PRELINK, 0) == 0)
> +	  && __glibc_likely ((GLRO(dl_debug_mask) & DL_DEBUG_PRELINK) == 0))
>  	{
>  	  /* We haven't found an appropriate library.  But since we
>  	     are only interested in the list of libraries this isn't
Carlos O'Donell March 26, 2014, 9:21 p.m. UTC | #2
On 03/26/2014 04:40 PM, Paul Pluzhnikov wrote:
> Greetings,
> 
> As promised here: https://sourceware.org/ml/libc-alpha/2014-03/msg00676.html
> attached patch converts most of __builtin_expect's into
> __glibc_{un}likely in elf/dl-load.c
> 
> This patch produces identical dl-load.{o,os} on Linux/x86_64 (except a
> few line numbers change).
> 
> The remaining 7 __builtin_expect's caused the object file to change in
> unexpected ways, which means that either I didn't do them correctly, or
> there is something else at play. I'll try to do them next.
> 
> Thanks,
> 
> --
> 
> 2013-03-26  Paul Pluzhnikov  <ppluzhnikov@google.com>
> 
> 	* elf/dl-load.c: Convert __builtin_expect into
>         __glibc_{un}likely.

Looks good to me.

OK, modulo one nit, see below (blank line).

Ondrej had some good suggestions for the other 7.
 
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 9455df2..fabe025 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -280,7 +280,7 @@ is_dst (const char *start, const char *name, const char *str,
>  	   && (!is_path || name[len] != ':'))
>      return 0;
>  
> -  if (__builtin_expect (secure, 0)
> +  if (__glibc_unlikely (secure)

OK.

>        && ((name[len] != '\0' && name[len] != '/'
>  	   && (!is_path || name[len] != ':'))
>  	  || (name != start + 1 && (!is_path || name[-2] != ':'))))
> @@ -381,7 +381,7 @@ _dl_dst_substitute (struct link_map *l, const char *name, char *result,
>  	      /* In SUID/SGID programs, after $ORIGIN expansion the
>  		 normalized path must be rooted in one of the trusted
>  		 directories.  */
> -	      if (__builtin_expect (check_for_trusted, false)
> +	      if (__glibc_unlikely (check_for_trusted)

OK.

>  		  && !is_trusted_path_normalize (last_elem, wp - last_elem))
>  		wp = last_elem;
>  	      else
> @@ -395,7 +395,7 @@ _dl_dst_substitute (struct link_map *l, const char *name, char *result,
>  
>    /* In SUID/SGID programs, after $ORIGIN expansion the normalized
>       path must be rooted in one of the trusted directories.  */
> -  if (__builtin_expect (check_for_trusted, false)
> +  if (__glibc_unlikely (check_for_trusted)

OK.

>        && !is_trusted_path_normalize (last_elem, wp - last_elem))
>      wp = last_elem;
>  
> @@ -425,7 +425,7 @@ expand_dynamic_string_token (struct link_map *l, const char *s, int is_path)
>    cnt = DL_DST_COUNT (s, is_path);
>  
>    /* If we do not have to replace anything simply copy the string.  */
> -  if (__builtin_expect (cnt, 0) == 0)
> +  if (__glibc_likely (cnt == 0))

OK.

>      return local_strdup (s);
>  
>    /* Determine the length of the substituted string.  */
> @@ -513,7 +513,7 @@ fillin_rpath (char *rpath, struct r_search_path_elem **result, const char *sep,
>  	cp[len++] = '/';
>  
>        /* Make sure we don't use untrusted directories if we run SUID.  */
> -      if (__builtin_expect (check_trusted, 0) && !is_trusted_path (cp, len))
> +      if (__glibc_unlikely (check_trusted) && !is_trusted_path (cp, len))

OK.

>  	{
>  	  free (to_free);
>  	  continue;
> @@ -604,7 +604,7 @@ decompose_rpath (struct r_search_path_struct *sps,
>  
>    /* First see whether we must forget the RUNPATH and RPATH from this
>       object.  */
> -  if (__builtin_expect (GLRO(dl_inhibit_rpath) != NULL, 0)
> +  if (__glibc_unlikely (GLRO(dl_inhibit_rpath) != NULL)

OK.

>        && !INTUSE(__libc_enable_secure))
>      {
>        const char *inhp = GLRO(dl_inhibit_rpath);
> @@ -964,7 +964,7 @@ _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
>  #ifdef SHARED
>    /* When loading into a namespace other than the base one we must
>       avoid loading ld.so since there can only be one copy.  Ever.  */
> -  if (__builtin_expect (nsid != LM_ID_BASE, 0)
> +  if (__glibc_unlikely (nsid != LM_ID_BASE)

OK.

>        && ((st.st_ino == GL(dl_rtld_map).l_ino
>  	   && st.st_dev == GL(dl_rtld_map).l_dev)
>  	  || _dl_name_match_p (name, &GL(dl_rtld_map))))
> @@ -1026,7 +1026,7 @@ _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
>  #ifdef SHARED
>        /* Auditing checkpoint: we are going to add new objects.  */
>        if ((mode & __RTLD_AUDIT) == 0
> -	  && __builtin_expect (GLRO(dl_naudit) > 0, 0))
> +	  && __glibc_unlikely (GLRO(dl_naudit) > 0))

OK.

>  	{
>  	  struct link_map *head = GL(dl_ns)[nsid]._ns_loaded;
>  	  /* Do not call the functions for any auditing object.  */
> @@ -1124,14 +1124,13 @@ _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
>  	case PT_LOAD:
>  	  /* A load command tells us to map in part of the file.
>  	     We record the load commands and process them all later.  */
> -	  if (__builtin_expect ((ph->p_align & (GLRO(dl_pagesize) - 1)) != 0,
> -				0))
> +	  if (__glibc_unlikely ((ph->p_align & (GLRO(dl_pagesize) - 1)) != 0))

OK.

>  	    {
>  	      errstring = N_("ELF load command alignment not page-aligned");
>  	      goto call_lose;
>  	    }
> -	  if (__builtin_expect (((ph->p_vaddr - ph->p_offset)
> -				 & (ph->p_align - 1)) != 0, 0))
> +	  if (__glibc_unlikely (((ph->p_vaddr - ph->p_offset)
> +				 & (ph->p_align - 1)) != 0))

OK.

>  	    {
>  	      errstring
>  		= N_("ELF load command address/offset not properly aligned");
> @@ -1184,10 +1183,10 @@ _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
>  
>  	  /* If not loading the initial set of shared libraries,
>  	     check whether we should permit loading a TLS segment.  */
> -	  if (__builtin_expect (l->l_type == lt_library, 1)
> +	  if (__glibc_likely (l->l_type == lt_library)

OK.

>  	      /* If GL(dl_tls_dtv_slotinfo_list) == NULL, then rtld.c did
>  		 not set up TLS data structures, so don't use them now.  */
> -	      || __builtin_expect (GL(dl_tls_dtv_slotinfo_list) != NULL, 1))
> +	      || __glibc_likely (GL(dl_tls_dtv_slotinfo_list) != NULL))

OK.

>  	    {
>  	      /* Assign the next available module ID.  */
>  	      l->l_tls_modid = _dl_next_tls_modid ();
> @@ -1214,9 +1213,8 @@ _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
>  
>  	      /* The first call allocates TLS bookkeeping data structures.
>  		 Then we allocate the TCB for the initial thread.  */
> -	      if (__builtin_expect (_dl_tls_setup (), 0)
> -		  || __builtin_expect ((tcb = _dl_allocate_tls (NULL)) == NULL,
> -				       0))
> +	      if (__glibc_unlikely (_dl_tls_setup ())
> +		  || __glibc_unlikely ((tcb = _dl_allocate_tls (NULL)) == NULL))

OK.

>  		{
>  		  errval = ENOMEM;
>  		  errstring = N_("\
> @@ -1430,7 +1428,7 @@ cannot allocate TLS data structures for initial thread");
>  
>    /* Make sure we are not dlopen'ing an object that has the
>       DF_1_NOOPEN flag set.  */
> -  if (__builtin_expect (l->l_flags_1 & DF_1_NOOPEN, 0)
> +  if (__glibc_unlikely (l->l_flags_1 & DF_1_NOOPEN)

OK.

>        && (mode & __RTLD_DLOPEN))
>      {
>        /* We are not supposed to load this object.  Free all resources.  */
> @@ -1469,8 +1467,8 @@ cannot allocate TLS data structures for initial thread");
>  
>    if (__glibc_unlikely ((stack_flags &~ GL(dl_stack_flags)) & PF_X))
>      {
> -      if (__builtin_expect (__check_caller (RETURN_ADDRESS (0), allow_ldso),
> -			    0) != 0)
> +
> +      if (__glibc_unlikely (__check_caller (RETURN_ADDRESS (0), allow_ldso) != 0))

OK.

>  	{
>  	  errstring = N_("invalid caller");
>  	  goto call_lose;
> @@ -1560,7 +1558,7 @@ cannot enable executable stack as shared object requires");
>    /* If this object has DT_SYMBOLIC set modify now its scope.  We don't
>       have to do this for the main map.  */
>    if ((mode & RTLD_DEEPBIND) == 0
> -      && __builtin_expect (l->l_info[DT_SYMBOLIC] != NULL, 0)
> +      && __glibc_unlikely (l->l_info[DT_SYMBOLIC] != NULL)

OK.

>        && &l->l_searchlist != l->l_scope[0])
>      {
>        /* Create an appropriate searchlist.  It contains only this map.
> @@ -1586,7 +1584,7 @@ cannot enable executable stack as shared object requires");
>  
>    /* When we profile the SONAME might be needed for something else but
>       loading.  Add it right away.  */
> -  if (__builtin_expect (GLRO(dl_profile) != NULL, 0)
> +  if (__glibc_unlikely (GLRO(dl_profile) != NULL)

OK.

>        && l->l_info[DT_SONAME] != NULL)
>      add_name_to_object (l, ((const char *) D_PTR (l, l_info[DT_STRTAB])
>  			    + l->l_info[DT_SONAME]->d_un.d_val));
> @@ -1600,7 +1598,7 @@ cannot enable executable stack as shared object requires");
>  
>  #ifdef SHARED
>    /* Auditing checkpoint: we have a new object.  */
> -  if (__builtin_expect (GLRO(dl_naudit) > 0, 0)
> +  if (__glibc_unlikely (GLRO(dl_naudit) > 0)

OK.

>        && !GL(dl_ns)[l->l_ns]._ns_loaded->l_auditing)
>      {
>        struct audit_ifaces *afct = GLRO(dl_audit);
> @@ -1704,7 +1702,7 @@ open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
>  
>  #ifdef SHARED
>    /* Give the auditing libraries a chance.  */
> -  if (__builtin_expect (GLRO(dl_naudit) > 0, 0) && whatcode != 0
> +  if (__glibc_unlikely (GLRO(dl_naudit) > 0) && whatcode != 0

OK.

>        && loader->l_auditing == 0)
>      {
>        struct audit_ifaces *afct = GLRO(dl_audit);
> @@ -1748,7 +1746,7 @@ open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
>  	    break;
>  	  fbp->len += retlen;
>  	}
> -      while (__builtin_expect (fbp->len < sizeof (ElfW(Ehdr)), 0));
> +      while (__glibc_unlikely (fbp->len < sizeof (ElfW(Ehdr))));

OK.

>  
>        /* This is where the ELF header is loaded.  */
>        ehdr = (ElfW(Ehdr) *) fbp->buf;
> @@ -1830,7 +1828,7 @@ open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
>  	  goto call_lose;
>  	}
>  
> -      if (__builtin_expect (ehdr->e_version, EV_CURRENT) != EV_CURRENT)
> +      if (__glibc_unlikely (ehdr->e_version != EV_CURRENT))

OK. Had to think twice about this one :-)

>  	{
>  	  errstring = N_("ELF file version does not match current one");
>  	  goto call_lose;
> @@ -1854,8 +1852,8 @@ open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
>  	  errstring = N_("cannot dynamically load executable");
>  	  goto call_lose;
>  	}
> -      else if (__builtin_expect (ehdr->e_phentsize, sizeof (ElfW(Phdr)))
> -	       != sizeof (ElfW(Phdr)))
> +      else if (__glibc_unlikely (ehdr->e_phentsize != sizeof (ElfW(Phdr))))
> +

Added blank line we don't need.

>  	{
>  	  errstring = N_("ELF file's phentsize not the expected size");
>  	  goto call_lose;
> @@ -1967,7 +1965,7 @@ open_path (const char *name, size_t namelen, int mode,
>  
>        /* If we are debugging the search for libraries print the path
>  	 now if it hasn't happened now.  */
> -      if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_LIBS, 0)
> +      if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_LIBS)

OK.

>  	  && current_what != this_dir->what)
>  	{
>  	  current_what = this_dir->what;
> @@ -2021,7 +2019,7 @@ open_path (const char *name, size_t namelen, int mode,
>  	  /* Remember whether we found any existing directory.  */
>  	  here_any |= this_dir->status[cnt] != nonexisting;
>  
> -	  if (fd != -1 && __builtin_expect (mode & __RTLD_SECURE, 0)
> +	  if (fd != -1 && __glibc_unlikely (mode & __RTLD_SECURE)

OK.

>  	      && INTUSE(__libc_enable_secure))
>  	    {
>  	      /* This is an extra security effort to make sure nobody can
> @@ -2108,14 +2106,14 @@ _dl_map_object (struct link_map *loader, const char *name,
>        /* If the requested name matches the soname of a loaded object,
>  	 use that object.  Elide this check for names that have not
>  	 yet been opened.  */
> -      if (__builtin_expect (l->l_faked, 0) != 0
> +      if (__glibc_unlikely (l->l_faked != 0)

OK.

>  	  || __builtin_expect (l->l_removed, 0) != 0)
>  	continue;
>        if (!_dl_name_match_p (name, l))
>  	{
>  	  const char *soname;
>  
> -	  if (__builtin_expect (l->l_soname_added, 1)
> +	  if (__glibc_likely (l->l_soname_added)

OK.

>  	      || l->l_info[DT_SONAME] == NULL)
>  	    continue;
>  
> @@ -2134,7 +2132,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>      }
>  
>    /* Display information if we are debugging.  */
> -  if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_FILES, 0)
> +  if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES)

OK.

>        && loader != NULL)
>      _dl_debug_printf ((mode & __RTLD_CALLMAP) == 0
>  		      ? "\nfile=%s [%lu];  needed by %s [%lu]\n"
> @@ -2144,7 +2142,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>  #ifdef SHARED
>    /* Give the auditing libraries a chance to change the name before we
>       try anything.  */
> -  if (__builtin_expect (GLRO(dl_naudit) > 0, 0)
> +  if (__glibc_unlikely (GLRO(dl_naudit) > 0)

OK.

>        && (loader == NULL || loader->l_auditing == 0))
>      {
>        struct audit_ifaces *afct = GLRO(dl_audit);
> @@ -2236,7 +2234,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>        if (fd == -1
>  	  && (__builtin_expect (! (mode & __RTLD_SECURE), 1)
>  	      || ! INTUSE(__libc_enable_secure))
> -	  && __builtin_expect (GLRO(dl_inhibit_cache) == 0, 1))
> +	  && __glibc_likely (GLRO(dl_inhibit_cache) == 0))

OK.

>  	{
>  	  /* Check the list of libraries in the file /etc/ld.so.cache,
>  	     for compatibility with Linux's ldconfig program.  */
> @@ -2254,7 +2252,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>  
>  	      /* If the loader has the DF_1_NODEFLIB flag set we must not
>  		 use a cache entry from any of these directories.  */
> -	      if (__builtin_expect (l->l_flags_1 & DF_1_NODEFLIB, 0))
> +	      if (__glibc_unlikely (l->l_flags_1 & DF_1_NODEFLIB))

OK.

>  		{
>  		  const char *dirp = system_dirs;
>  		  unsigned int cnt = 0;
> @@ -2297,7 +2295,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>        /* Finally, try the default path.  */
>        if (fd == -1
>  	  && ((l = loader ?: GL(dl_ns)[nsid]._ns_loaded) == NULL
> -	      || __builtin_expect (!(l->l_flags_1 & DF_1_NODEFLIB), 1))
> +	      || __glibc_likely (!(l->l_flags_1 & DF_1_NODEFLIB)))

OK.

>  	  && rtld_search_dirs.dirs != (void *) -1)
>  	fd = open_path (name, namelen, mode, &rtld_search_dirs,
>  			&realname, &fb, l, LA_SER_DEFAULT, &found_other_class);
> @@ -2319,7 +2317,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>  	  fd = open_verify (realname, &fb,
>  			    loader ?: GL(dl_ns)[nsid]._ns_loaded, 0, mode,
>  			    &found_other_class, true);
> -	  if (__builtin_expect (fd, 0) == -1)
> +	  if (__glibc_unlikely (fd == -1))

OK.

>  	    free (realname);
>  	}
>      }
> @@ -2333,10 +2331,10 @@ _dl_map_object (struct link_map *loader, const char *name,
>    if (mode & __RTLD_CALLMAP)
>      loader = NULL;
>  
> -  if (__builtin_expect (fd, 0) == -1)
> +  if (__glibc_unlikely (fd == -1))

OK.

>      {
>        if (trace_mode
> -	  && __builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_PRELINK, 0) == 0)
> +	  && __glibc_likely ((GLRO(dl_debug_mask) & DL_DEBUG_PRELINK) == 0))

OK.

>  	{
>  	  /* We haven't found an appropriate library.  But since we
>  	     are only interested in the list of libraries this isn't
> 

Cheers,
Carlos.
Paul Pluzhnikov March 26, 2014, 9:48 p.m. UTC | #3
On Wed, Mar 26, 2014 at 2:21 PM, Carlos O'Donell <carlos@redhat.com> wrote:

> OK, modulo one nit, see below (blank line).

Sorry about that -- I was trying to minimize assembly differences due
to line-number changes, and missed that one.

> Ondrej had some good suggestions for the other 7.

Ok to commit this one and work on the remaining 7 separately?

Thanks,
Carlos O'Donell March 26, 2014, 10:08 p.m. UTC | #4
On 03/26/2014 05:48 PM, Paul Pluzhnikov wrote:
> On Wed, Mar 26, 2014 at 2:21 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> 
>> OK, modulo one nit, see below (blank line).
> 
> Sorry about that -- I was trying to minimize assembly differences due
> to line-number changes, and missed that one.
> 
>> Ondrej had some good suggestions for the other 7.
> 
> Ok to commit this one and work on the remaining 7 separately?

Yes.

Cheers,
Carlos.

Patch
diff mbox

diff --git a/elf/dl-load.c b/elf/dl-load.c
index 9455df2..fabe025 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -280,7 +280,7 @@  is_dst (const char *start, const char *name, const char *str,
 	   && (!is_path || name[len] != ':'))
     return 0;
 
-  if (__builtin_expect (secure, 0)
+  if (__glibc_unlikely (secure)
       && ((name[len] != '\0' && name[len] != '/'
 	   && (!is_path || name[len] != ':'))
 	  || (name != start + 1 && (!is_path || name[-2] != ':'))))
@@ -381,7 +381,7 @@  _dl_dst_substitute (struct link_map *l, const char *name, char *result,
 	      /* In SUID/SGID programs, after $ORIGIN expansion the
 		 normalized path must be rooted in one of the trusted
 		 directories.  */
-	      if (__builtin_expect (check_for_trusted, false)
+	      if (__glibc_unlikely (check_for_trusted)
 		  && !is_trusted_path_normalize (last_elem, wp - last_elem))
 		wp = last_elem;
 	      else
@@ -395,7 +395,7 @@  _dl_dst_substitute (struct link_map *l, const char *name, char *result,
 
   /* In SUID/SGID programs, after $ORIGIN expansion the normalized
      path must be rooted in one of the trusted directories.  */
-  if (__builtin_expect (check_for_trusted, false)
+  if (__glibc_unlikely (check_for_trusted)
       && !is_trusted_path_normalize (last_elem, wp - last_elem))
     wp = last_elem;
 
@@ -425,7 +425,7 @@  expand_dynamic_string_token (struct link_map *l, const char *s, int is_path)
   cnt = DL_DST_COUNT (s, is_path);
 
   /* If we do not have to replace anything simply copy the string.  */
-  if (__builtin_expect (cnt, 0) == 0)
+  if (__glibc_likely (cnt == 0))
     return local_strdup (s);
 
   /* Determine the length of the substituted string.  */
@@ -513,7 +513,7 @@  fillin_rpath (char *rpath, struct r_search_path_elem **result, const char *sep,
 	cp[len++] = '/';
 
       /* Make sure we don't use untrusted directories if we run SUID.  */
-      if (__builtin_expect (check_trusted, 0) && !is_trusted_path (cp, len))
+      if (__glibc_unlikely (check_trusted) && !is_trusted_path (cp, len))
 	{
 	  free (to_free);
 	  continue;
@@ -604,7 +604,7 @@  decompose_rpath (struct r_search_path_struct *sps,
 
   /* First see whether we must forget the RUNPATH and RPATH from this
      object.  */
-  if (__builtin_expect (GLRO(dl_inhibit_rpath) != NULL, 0)
+  if (__glibc_unlikely (GLRO(dl_inhibit_rpath) != NULL)
       && !INTUSE(__libc_enable_secure))
     {
       const char *inhp = GLRO(dl_inhibit_rpath);
@@ -964,7 +964,7 @@  _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
 #ifdef SHARED
   /* When loading into a namespace other than the base one we must
      avoid loading ld.so since there can only be one copy.  Ever.  */
-  if (__builtin_expect (nsid != LM_ID_BASE, 0)
+  if (__glibc_unlikely (nsid != LM_ID_BASE)
       && ((st.st_ino == GL(dl_rtld_map).l_ino
 	   && st.st_dev == GL(dl_rtld_map).l_dev)
 	  || _dl_name_match_p (name, &GL(dl_rtld_map))))
@@ -1026,7 +1026,7 @@  _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
 #ifdef SHARED
       /* Auditing checkpoint: we are going to add new objects.  */
       if ((mode & __RTLD_AUDIT) == 0
-	  && __builtin_expect (GLRO(dl_naudit) > 0, 0))
+	  && __glibc_unlikely (GLRO(dl_naudit) > 0))
 	{
 	  struct link_map *head = GL(dl_ns)[nsid]._ns_loaded;
 	  /* Do not call the functions for any auditing object.  */
@@ -1124,14 +1124,13 @@  _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
 	case PT_LOAD:
 	  /* A load command tells us to map in part of the file.
 	     We record the load commands and process them all later.  */
-	  if (__builtin_expect ((ph->p_align & (GLRO(dl_pagesize) - 1)) != 0,
-				0))
+	  if (__glibc_unlikely ((ph->p_align & (GLRO(dl_pagesize) - 1)) != 0))
 	    {
 	      errstring = N_("ELF load command alignment not page-aligned");
 	      goto call_lose;
 	    }
-	  if (__builtin_expect (((ph->p_vaddr - ph->p_offset)
-				 & (ph->p_align - 1)) != 0, 0))
+	  if (__glibc_unlikely (((ph->p_vaddr - ph->p_offset)
+				 & (ph->p_align - 1)) != 0))
 	    {
 	      errstring
 		= N_("ELF load command address/offset not properly aligned");
@@ -1184,10 +1183,10 @@  _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
 
 	  /* If not loading the initial set of shared libraries,
 	     check whether we should permit loading a TLS segment.  */
-	  if (__builtin_expect (l->l_type == lt_library, 1)
+	  if (__glibc_likely (l->l_type == lt_library)
 	      /* If GL(dl_tls_dtv_slotinfo_list) == NULL, then rtld.c did
 		 not set up TLS data structures, so don't use them now.  */
-	      || __builtin_expect (GL(dl_tls_dtv_slotinfo_list) != NULL, 1))
+	      || __glibc_likely (GL(dl_tls_dtv_slotinfo_list) != NULL))
 	    {
 	      /* Assign the next available module ID.  */
 	      l->l_tls_modid = _dl_next_tls_modid ();
@@ -1214,9 +1213,8 @@  _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
 
 	      /* The first call allocates TLS bookkeeping data structures.
 		 Then we allocate the TCB for the initial thread.  */
-	      if (__builtin_expect (_dl_tls_setup (), 0)
-		  || __builtin_expect ((tcb = _dl_allocate_tls (NULL)) == NULL,
-				       0))
+	      if (__glibc_unlikely (_dl_tls_setup ())
+		  || __glibc_unlikely ((tcb = _dl_allocate_tls (NULL)) == NULL))
 		{
 		  errval = ENOMEM;
 		  errstring = N_("\
@@ -1430,7 +1428,7 @@  cannot allocate TLS data structures for initial thread");
 
   /* Make sure we are not dlopen'ing an object that has the
      DF_1_NOOPEN flag set.  */
-  if (__builtin_expect (l->l_flags_1 & DF_1_NOOPEN, 0)
+  if (__glibc_unlikely (l->l_flags_1 & DF_1_NOOPEN)
       && (mode & __RTLD_DLOPEN))
     {
       /* We are not supposed to load this object.  Free all resources.  */
@@ -1469,8 +1467,8 @@  cannot allocate TLS data structures for initial thread");
 
   if (__glibc_unlikely ((stack_flags &~ GL(dl_stack_flags)) & PF_X))
     {
-      if (__builtin_expect (__check_caller (RETURN_ADDRESS (0), allow_ldso),
-			    0) != 0)
+
+      if (__glibc_unlikely (__check_caller (RETURN_ADDRESS (0), allow_ldso) != 0))
 	{
 	  errstring = N_("invalid caller");
 	  goto call_lose;
@@ -1560,7 +1558,7 @@  cannot enable executable stack as shared object requires");
   /* If this object has DT_SYMBOLIC set modify now its scope.  We don't
      have to do this for the main map.  */
   if ((mode & RTLD_DEEPBIND) == 0
-      && __builtin_expect (l->l_info[DT_SYMBOLIC] != NULL, 0)
+      && __glibc_unlikely (l->l_info[DT_SYMBOLIC] != NULL)
       && &l->l_searchlist != l->l_scope[0])
     {
       /* Create an appropriate searchlist.  It contains only this map.
@@ -1586,7 +1584,7 @@  cannot enable executable stack as shared object requires");
 
   /* When we profile the SONAME might be needed for something else but
      loading.  Add it right away.  */
-  if (__builtin_expect (GLRO(dl_profile) != NULL, 0)
+  if (__glibc_unlikely (GLRO(dl_profile) != NULL)
       && l->l_info[DT_SONAME] != NULL)
     add_name_to_object (l, ((const char *) D_PTR (l, l_info[DT_STRTAB])
 			    + l->l_info[DT_SONAME]->d_un.d_val));
@@ -1600,7 +1598,7 @@  cannot enable executable stack as shared object requires");
 
 #ifdef SHARED
   /* Auditing checkpoint: we have a new object.  */
-  if (__builtin_expect (GLRO(dl_naudit) > 0, 0)
+  if (__glibc_unlikely (GLRO(dl_naudit) > 0)
       && !GL(dl_ns)[l->l_ns]._ns_loaded->l_auditing)
     {
       struct audit_ifaces *afct = GLRO(dl_audit);
@@ -1704,7 +1702,7 @@  open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
 
 #ifdef SHARED
   /* Give the auditing libraries a chance.  */
-  if (__builtin_expect (GLRO(dl_naudit) > 0, 0) && whatcode != 0
+  if (__glibc_unlikely (GLRO(dl_naudit) > 0) && whatcode != 0
       && loader->l_auditing == 0)
     {
       struct audit_ifaces *afct = GLRO(dl_audit);
@@ -1748,7 +1746,7 @@  open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
 	    break;
 	  fbp->len += retlen;
 	}
-      while (__builtin_expect (fbp->len < sizeof (ElfW(Ehdr)), 0));
+      while (__glibc_unlikely (fbp->len < sizeof (ElfW(Ehdr))));
 
       /* This is where the ELF header is loaded.  */
       ehdr = (ElfW(Ehdr) *) fbp->buf;
@@ -1830,7 +1828,7 @@  open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
 	  goto call_lose;
 	}
 
-      if (__builtin_expect (ehdr->e_version, EV_CURRENT) != EV_CURRENT)
+      if (__glibc_unlikely (ehdr->e_version != EV_CURRENT))
 	{
 	  errstring = N_("ELF file version does not match current one");
 	  goto call_lose;
@@ -1854,8 +1852,8 @@  open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
 	  errstring = N_("cannot dynamically load executable");
 	  goto call_lose;
 	}
-      else if (__builtin_expect (ehdr->e_phentsize, sizeof (ElfW(Phdr)))
-	       != sizeof (ElfW(Phdr)))
+      else if (__glibc_unlikely (ehdr->e_phentsize != sizeof (ElfW(Phdr))))
+
 	{
 	  errstring = N_("ELF file's phentsize not the expected size");
 	  goto call_lose;
@@ -1967,7 +1965,7 @@  open_path (const char *name, size_t namelen, int mode,
 
       /* If we are debugging the search for libraries print the path
 	 now if it hasn't happened now.  */
-      if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_LIBS, 0)
+      if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_LIBS)
 	  && current_what != this_dir->what)
 	{
 	  current_what = this_dir->what;
@@ -2021,7 +2019,7 @@  open_path (const char *name, size_t namelen, int mode,
 	  /* Remember whether we found any existing directory.  */
 	  here_any |= this_dir->status[cnt] != nonexisting;
 
-	  if (fd != -1 && __builtin_expect (mode & __RTLD_SECURE, 0)
+	  if (fd != -1 && __glibc_unlikely (mode & __RTLD_SECURE)
 	      && INTUSE(__libc_enable_secure))
 	    {
 	      /* This is an extra security effort to make sure nobody can
@@ -2108,14 +2106,14 @@  _dl_map_object (struct link_map *loader, const char *name,
       /* If the requested name matches the soname of a loaded object,
 	 use that object.  Elide this check for names that have not
 	 yet been opened.  */
-      if (__builtin_expect (l->l_faked, 0) != 0
+      if (__glibc_unlikely (l->l_faked != 0)
 	  || __builtin_expect (l->l_removed, 0) != 0)
 	continue;
       if (!_dl_name_match_p (name, l))
 	{
 	  const char *soname;
 
-	  if (__builtin_expect (l->l_soname_added, 1)
+	  if (__glibc_likely (l->l_soname_added)
 	      || l->l_info[DT_SONAME] == NULL)
 	    continue;
 
@@ -2134,7 +2132,7 @@  _dl_map_object (struct link_map *loader, const char *name,
     }
 
   /* Display information if we are debugging.  */
-  if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_FILES, 0)
+  if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES)
       && loader != NULL)
     _dl_debug_printf ((mode & __RTLD_CALLMAP) == 0
 		      ? "\nfile=%s [%lu];  needed by %s [%lu]\n"
@@ -2144,7 +2142,7 @@  _dl_map_object (struct link_map *loader, const char *name,
 #ifdef SHARED
   /* Give the auditing libraries a chance to change the name before we
      try anything.  */
-  if (__builtin_expect (GLRO(dl_naudit) > 0, 0)
+  if (__glibc_unlikely (GLRO(dl_naudit) > 0)
       && (loader == NULL || loader->l_auditing == 0))
     {
       struct audit_ifaces *afct = GLRO(dl_audit);
@@ -2236,7 +2234,7 @@  _dl_map_object (struct link_map *loader, const char *name,
       if (fd == -1
 	  && (__builtin_expect (! (mode & __RTLD_SECURE), 1)
 	      || ! INTUSE(__libc_enable_secure))
-	  && __builtin_expect (GLRO(dl_inhibit_cache) == 0, 1))
+	  && __glibc_likely (GLRO(dl_inhibit_cache) == 0))
 	{
 	  /* Check the list of libraries in the file /etc/ld.so.cache,
 	     for compatibility with Linux's ldconfig program.  */
@@ -2254,7 +2252,7 @@  _dl_map_object (struct link_map *loader, const char *name,
 
 	      /* If the loader has the DF_1_NODEFLIB flag set we must not
 		 use a cache entry from any of these directories.  */
-	      if (__builtin_expect (l->l_flags_1 & DF_1_NODEFLIB, 0))
+	      if (__glibc_unlikely (l->l_flags_1 & DF_1_NODEFLIB))
 		{
 		  const char *dirp = system_dirs;
 		  unsigned int cnt = 0;
@@ -2297,7 +2295,7 @@  _dl_map_object (struct link_map *loader, const char *name,
       /* Finally, try the default path.  */
       if (fd == -1
 	  && ((l = loader ?: GL(dl_ns)[nsid]._ns_loaded) == NULL
-	      || __builtin_expect (!(l->l_flags_1 & DF_1_NODEFLIB), 1))
+	      || __glibc_likely (!(l->l_flags_1 & DF_1_NODEFLIB)))
 	  && rtld_search_dirs.dirs != (void *) -1)
 	fd = open_path (name, namelen, mode, &rtld_search_dirs,
 			&realname, &fb, l, LA_SER_DEFAULT, &found_other_class);
@@ -2319,7 +2317,7 @@  _dl_map_object (struct link_map *loader, const char *name,
 	  fd = open_verify (realname, &fb,
 			    loader ?: GL(dl_ns)[nsid]._ns_loaded, 0, mode,
 			    &found_other_class, true);
-	  if (__builtin_expect (fd, 0) == -1)
+	  if (__glibc_unlikely (fd == -1))
 	    free (realname);
 	}
     }
@@ -2333,10 +2331,10 @@  _dl_map_object (struct link_map *loader, const char *name,
   if (mode & __RTLD_CALLMAP)
     loader = NULL;
 
-  if (__builtin_expect (fd, 0) == -1)
+  if (__glibc_unlikely (fd == -1))
     {
       if (trace_mode
-	  && __builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_PRELINK, 0) == 0)
+	  && __glibc_likely ((GLRO(dl_debug_mask) & DL_DEBUG_PRELINK) == 0))
 	{
 	  /* We haven't found an appropriate library.  But since we
 	     are only interested in the list of libraries this isn't