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

[Alan Modra]

On Thu, Feb 27, 2025 at 08:13:05AM +0100, Jan Beulich wrote:
> On 27.02.2025 00:21, Alan Modra wrote:
> > On Wed, Feb 26, 2025 at 12:58:55PM +0100, Jan Beulich wrote:
> >> Multiple sections of the same name may exist (32-bit HPPA for example
> >> looks to be keeping sections separate when combining object files with
> >> ld -r). And really there's no reason to go by section name: We can
> >> simply compare section pointers, as only a match with the section being
> >> disassembled is of interest.
> >> ---
> >> Tests put in place by subsequent patches will cover this, on HPPA 32-bit
> >> ELF.
> >>
> >> --- a/binutils/objdump.c
> >> +++ b/binutils/objdump.c
> >> @@ -1133,8 +1133,8 @@ compare_symbols (const void *ap, const v
> >>       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;
> >> +  as = compare_section == a->section;
> >> +  bs = compare_section == b->section;
> >>    if (as && !bs)
> >>      return -1;
> >>    if (!as && bs)
> > 
> > The sym_ok comment says:
> >       /* 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.  */
> 
> Hmm, but then what do we do? Comparing names clearly is wrong, as the
> testcases added in the subsequent patch demonstrate.
> 
> Plus I'm in trouble with you saying "could have come from a separate debug
> info file": What we disassemble are sections from "real" object files. In
> particular compare_section isn't going to reference a debug info file. And
> that's what we mean to special case here, as per the first sentence of the
> comment (just out of context): "Prefer symbols from the section currently
> being disassembled." (sym_ok() actually does both - first compare pointers
> if the file is the same, and only then compare by name. I can't conclude
> yet whether the latter is entirely correct. Nevertheless maybe you're
> suggesting we similarly need to compare both here?)

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.

> What I need to do is drop the last sentence of this same comment.
> 
> Jan

-- 
Alan Modra