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

[Mark Wielaard <mjw at redhat dot com>]

On Wed, 2013-01-16 at 16:56 +0100, Jan Kratochvil wrote:
> On Wed, 16 Jan 2013 15:54:13 +0100, Mark Wielaard wrote:
> > +  /* All local symbols should come before all global symbols.  If we
> > +     have an auxiliary table make sure all the main locals come first
> > +     and all main globals come last.  And skip the auxiliary table
> > +     zero undefined entry.  */
> >    GElf_Word shndx;
> > -  sym = gelf_getsymshndx (mod->symdata, mod->symxndxdata, ndx, sym, &shndx);
> > +  int tndx = ndx;
> > +  struct dwfl_file *file;
> > +  Elf_Data *symdata;
> > +  Elf_Data *symxndxdata;
> > +  Elf_Data *symstrdata;
> > +  if (mod->aux_symdata == NULL
> > +      || ndx < mod->first_global)
> > +    {
> > +      /* main symbol table (locals).  */
> > +      tndx = ndx;
> > +      file = mod->symfile;
> > +      symdata = mod->symdata;
> > +      symxndxdata = mod->symxndxdata;
> > +      symstrdata = mod->symstrdata;
> > +    }
> > +  else if ((size_t) ndx < mod->first_global + mod->aux_syments)
> > +    {
> > +      /* aux symbol table.  */
> > +      tndx = ndx - mod->first_global + 1;
> > +      file = &mod->aux_sym;
> > +      symdata = mod->aux_symdata;
> > +      symxndxdata = mod->aux_symxndxdata;
> > +      symstrdata = mod->aux_symstrdata;
> > +    }
> > +  else
> > +    {
> > +      /* main symbol table (globals).  */
> > +      tndx = ndx - mod->aux_syments;
> > +      file = mod->symfile;
> > +      symdata = mod->symdata;
> > +      symxndxdata = mod->symxndxdata;
> > +      symstrdata = mod->symstrdata;
> > +    }
> 
> As elfutils is already careful with choosing symbols order in
> dwfl_module_addrsym.c couldn't the aux-symtab local symbols have the same
> preference to the main local symbols like the global symbols have?
> 
> I do not find breaking the symbols order is worth the small simplification of
> the code.

I am not sure the order of the globals really matters much, and I like
smaller simpler code since it contains less bugs :) But it is easy to
switch around. Just have to double check I didn't introduce yet another
off by one by updating the tests too. I assume you would like something
like this change?

diff --git a/libdwfl/dwfl_module_getsym.c b/libdwfl/dwfl_module_getsym.c
index 6479e26..3e4d9f6 100644
--- a/libdwfl/dwfl_module_getsym.c
+++ b/libdwfl/dwfl_module_getsym.c
@@ -43,9 +43,9 @@ dwfl_module_getsym (Dwfl_Module *mod, int ndx,
     }
 
   /* All local symbols should come before all global symbols.  If we
-     have an auxiliary table make sure all the main locals come first
-     and all main globals come last.  And skip the auxiliary table
-     zero undefined entry.  */
+     have an auxiliary table make sure all the main locals come first,
+     then all aux locals, then all main globals and finally all aux globals.
+     And skip the auxiliary table zero undefined entry.  */
   GElf_Word shndx;
   int tndx = ndx;
   struct dwfl_file *file;
@@ -62,24 +62,33 @@ dwfl_module_getsym (Dwfl_Module *mod, int ndx,
       symxndxdata = mod->symxndxdata;
       symstrdata = mod->symstrdata;
     }
-  else if ((size_t) ndx < mod->first_global + mod->aux_syments)
+  else if (ndx < mod->first_global + mod->aux_first_global - 1)
     {
-      /* aux symbol table.  */
+      /* aux symbol table (locals).  */
       tndx = ndx - mod->first_global + 1;
       file = &mod->aux_sym;
       symdata = mod->aux_symdata;
       symxndxdata = mod->aux_symxndxdata;
       symstrdata = mod->aux_symstrdata;
     }
-  else
+  else if ((size_t) ndx < mod->syments + mod->aux_first_global - 1)
     {
       /* main symbol table (globals).  */
-      tndx = ndx - mod->aux_syments;
+      tndx = ndx - mod->aux_first_global + 1;
       file = mod->symfile;
       symdata = mod->symdata;
       symxndxdata = mod->symxndxdata;
       symstrdata = mod->symstrdata;
     }
+  else
+    {
+      /* aux symbol table (globals).  */
+      tndx = ndx - mod->syments + 1;
+      file = &mod->aux_sym;
+      symdata = mod->aux_symdata;
+      symxndxdata = mod->aux_symxndxdata;
+      symstrdata = mod->aux_symstrdata;
+    }
   sym = gelf_getsymshndx (symdata, symxndxdata, tndx, sym, &shndx);
 
   if (unlikely (sym == NULL))