[PATCH 2/4] objdump: don't compare sections by name

[Alan Modra]

On Thu, Mar 06, 2025 at 06:00:05PM +0100, Jan Beulich wrote:
> On 28.02.2025 09:14, Alan Modra wrote:
> > On Fri, Feb 28, 2025 at 09:00:27AM +1030, Alan Modra wrote:
> >> Yes, the commit 660df28acfa1 change to compare_symbols should have
> >> used the same logic as that in sym_ok.  Ideally both places would not
> >> compare section names.  Exactly how to match a section in the real
> >> object file with a section in a debug file might be a little tricky,
> >> which is probably why commit 39f0547e554d used names.  Binutils
> >> objcopy tries to keep as much section info intact as possible when
> >> creating debug-only files, for example vma and size match for
> >> corresponding sections.
> > 
> > Like this, which works for your testcase.
> > 
> > diff --git a/binutils/objdump.c b/binutils/objdump.c
> > index 5ab29a719bc..e31da826c44 100644
> > --- a/binutils/objdump.c
> > +++ b/binutils/objdump.c
> > @@ -1107,6 +1107,28 @@ remove_useless_symbols (asymbol **symbols, long count)
> >    return out_ptr - symbols;
> >  }
> >  
> > +/* Return true iff SEC1 and SEC2 are the same section.
> > +   This would just be a simple pointer comparison except that one of
> > +   the sections might be from a separate debug info file.  */
> > +
> > +static bool
> > +is_same_section (const asection *sec1, const asection *sec2)
> > +{
> > +  if (sec1 == sec2)
> > +    return true;
> > +  if (sec1->owner == sec2->owner
> > +      || sec1->owner == NULL
> > +      || sec2->owner == NULL)
> > +    return false;
> 
> For my own education: In which situation may ->owner be NULL?

When the section is any of the _bfd_std_section entries, which can
happen for example with sec2 from an *ABS* symbol.  There isn't really
any reason for the NULL checks as the function now is written, but
originally I was going to try to match sections in the debug bfd
without comparing names.

> Other than that I'd like to integrate this into my patch then, mentioning
> you as co-author (perhaps even the main one). Provided that's okay with
> you.
> 
> Jan
> 
> > +  /* OK, so we have one section in a debug info file.  (Or they both
> > +     are, but the way this function is currently used sec1 will be in
> > +     a normal object.)  Compare names, vma and size.  This ought to
> > +     cover all the usual cases.  */
> > +  return (sec1->vma == sec2->vma
> > +	  && sec1->size == sec2->size
> > +	  && strcmp (sec1->name, sec2->name) == 0);
> > +}
> > +
> >  static const asection *compare_section;
> >  
> >  /* Sort symbols into value order.  */
> > @@ -1131,10 +1153,9 @@ compare_symbols (const void *ap, const void *bp)
> >  
> >    /* Prefer symbols from the section currently being disassembled.
> >       Don't sort symbols from other sections by section, since there
> > -     isn't much reason to prefer one section over another otherwise.
> > -     See sym_ok comment for why we compare by section name.  */
> > -  as = strcmp (compare_section->name, a->section->name) == 0;
> > -  bs = strcmp (compare_section->name, b->section->name) == 0;
> > +     isn't much reason to prefer one section over another otherwise.  */
> > +  as = is_same_section (compare_section, a->section);
> > +  bs = is_same_section (compare_section, b->section);
> >    if (as && !bs)
> >      return -1;
> >    if (!as && bs)
> > @@ -1353,26 +1374,8 @@ sym_ok (bool want_section,
> >  	asection *sec,
> >  	struct disassemble_info *inf)
> >  {
> > -  if (want_section)
> > -    {
> > -      /* NB: An object file can have different sections with the same
> > -	 section name.  Compare compare section pointers if they have
> > -	 the same owner.  */
> > -      if (sorted_syms[place]->section->owner == sec->owner
> > -	  && sorted_syms[place]->section != sec)
> > -	return false;
> > -
> > -      /* Note - we cannot just compare section pointers because they could
> > -	 be different, but the same...  Ie the symbol that we are trying to
> > -	 find could have come from a separate debug info file.  Under such
> > -	 circumstances the symbol will be associated with a section in the
> > -	 debug info file, whilst the section we want is in a normal file.
> > -	 So the section pointers will be different, but the section names
> > -	 will be the same.  */
> > -      if (strcmp (bfd_section_name (sorted_syms[place]->section),
> > -		  bfd_section_name (sec)) != 0)
> > -	return false;
> > -    }
> > +  if (want_section && !is_same_section (sec, sorted_syms[place]->section))
> > +    return false;
> >  
> >    return inf->symbol_is_valid (sorted_syms[place], inf);
> >  }
> > 

-- 
Alan Modra