Re: [PATCH 2/3] libdwfl: Add minisymtab support.

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Wed, 16 Jan 2013 15:54:13 +0100, Mark Wielaard wrote:
> diff --git a/libdwfl/dwfl_module_addrsym.c b/libdwfl/dwfl_module_addrsym.c
> index fdc95fc..0e4de9f 100644
> --- a/libdwfl/dwfl_module_addrsym.c
> +++ b/libdwfl/dwfl_module_addrsym.c
[...]
> @@ -168,17 +173,19 @@ dwfl_module_addrsym (Dwfl_Module *mod, GElf_Addr addr,
>  	}
>      }
>  
> -  /* First go through global symbols.  mod->first_global is setup by
> -     dwfl_module_getsymtab to the index of the first global symbol in
> -     the module's symbol table, or -1 when unknown.  All symbols with
> -     local binding come first in the symbol table, then all globals.  */
> -  search_table (mod->first_global < 0 ? 1 : mod->first_global, syments);
> +  /* First go through global symbols.  mod->first_global and
> +     mod->aux_first_global are setup by dwfl_module_getsymtab to the
> +     index of the first global symbol in the module's symbol table, or
> +     -1 when unknown.  All symbols with local binding come first in
> +     the symbol table, then all globals.  */
> +  int first_global = mod->first_global + mod->aux_first_global - 1;

aux_first_global is not initialized to -1 anywhere so in the normal case
without minisymtab it is left initialized as 0.  That probably does not matter
as it just needlessly searches one more symbol but I find it worth fixing for
the code clarity.


> +  search_table (first_global < 0 ? 1 : first_global, syments);
>  
>    /* If we found nothing searching the global symbols, then try the locals.
>       Unless we have a global sizeless symbol that matches exactly.  */
> -  if (closest_name == NULL && mod->first_global > 1
> +  if (closest_name == NULL && first_global > 1
>        && (sizeless_name == NULL || sizeless_sym.st_value != addr))
> -    search_table (1, mod->first_global);
> +    search_table (1, first_global);
>  
>    /* If we found no proper sized symbol to use, fall back to the best
>       candidate sizeless symbol we found, if any.  */
> diff --git a/libdwfl/dwfl_module_getdwarf.c b/libdwfl/dwfl_module_getdwarf.c
> index 025cb8a..6f6ae78 100644
> --- a/libdwfl/dwfl_module_getdwarf.c
> +++ b/libdwfl/dwfl_module_getdwarf.c
[...]
> @@ -579,6 +580,7 @@ load_symtab (struct dwfl_file *file, struct dwfl_file **symfile,
          case SHT_DYNSYM:
[...]
>  	    *symfile = file;
>  	    *strshndx = shdr->sh_link;
>  	    *syments = shdr->sh_size / shdr->sh_entsize;
> +	    *first_global = shdr->sh_info;

I have found for example /usr/bin/loginctl from systemd-44-23.fc17.x86_64:

Symbol table '.dynsym' contains 147 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
     1: 0000000000614444     0 NOTYPE  LOCAL  DEFAULT  ABS _edata
     2: 0000000000614cd0     0 NOTYPE  LOCAL  DEFAULT  ABS _end
     3: 0000000000614444     0 NOTYPE  LOCAL  DEFAULT  ABS __bss_start

So I agree now .dynsym may contain local symbols, but I still do not
understand how that happens (systemd does not rebuild for me locally).

Isn't it rather a bug of the build toolchain for systemd?


>  	    break;
>  
>  	  case SHT_SYMTAB_SHNDX:


Otherwise it looks OK to me.


Thanks,
Jan