[PATCH 1/2] readelf: Fix symbol display for RELR relocs

[Nick Clifton]

Hi Szabolcs,

> Filter symbols before binary searching for the right symbol to display
> for a given address, such that only displayable symbols are present and
> at most one per address.
> 
> The current logic does not handle multiple symbols for the same address
> well if some of them are empty, the selected symbol is not stable with
> respect to an unrelated symbol table change and on aarch64 often mapping
> symbols are displayed which is not useful.
> 
> Filtering solves these problems at the cost of a linear scan of the
> sorted symbol table.
> 
> The heuristic to select the best symbol likely could be improved, this
> patch aims to improve symbol display for RELR without complex logic
> such that the output is useful and stable for ld tests.
> ---

I like the patch, but there are a couple of things that I think should be
changed:

> +static bool
> +is_aarch64_special_symbol_name (const char *name)
> +{
> +  if (!name || name[0] != '$')
> +    return false;
> +  if (name[1] == 'x' || name[1] == 'd')
> +    return true;
> +  else if (name[1] == 'm' || name[1] == 'f' || name[1] == 'p')
> +    return true;
> +  else
> +    return false;
> +  return name[2] == 0 || name[2] == '.';
> +}

Can that last line (checking name[2]) ever be reached ?


> +  if (filedata->file_header.e_machine == EM_AARCH64)
> +    {
> +      /* Don't display mapping symbols.  */
> +      if (is_aarch64_special_symbol_name (s))
> +	return best;
> +    }

I think that you should change this to calling a generic checking
function which then runs architecture specific tests.  (Since there
are other architectures which also have special symbols).  ie:

    if (is_special_symbol (filedata, s))
      return best;

   [...]

   static bool
   is_special_symbol (Filedata * filedata, Elf_Internal_Sym * s)
   {
      switch (filedata->file_header.e_machine)
      {
      case EM_AARCh64: return is_aarch64_special_symbol (s);
   #if 0 /* Future expansion here...  */
      case EM_MIPS: return is_mips_special_symbol (s);
   #endif
      default: return false;
   }

Cheers
   Nick