This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] binutils: Improve symbols lookup
On Sun, Dec 15, 2019 at 06:48:54PM +0100, Borislav Petkov wrote:
> On Thu, Dec 12, 2019 at 04:07:35PM +1030, Alan Modra wrote:
> > The right way to prefer function and object symbols is to modify
> > compare_symbols.
>
> Well, the problem there is, is that the section pointer comparison comes
> before we even get to look at the flags and we exit there early:
>
> A: in_suspend, sect: 0x55c40bd0d630
> B: __smp_locks_end, sect: 0x55c40bd0d500
> a sec > b sec
> A: __nosave_begin, sect: 0x55c40bd0d630
> B: __smp_locks_end, sect: 0x55c40bd0d500
> a sec > b sec
> A: __nosave_begin, sect: 0x55c40bd0d630
> B: in_suspend, sect: 0x55c40bd0d630
>
> leading to this sort order:
>
> 779278: 0xffffffff9b230000 __smp_locks_end
> 779279: 0xffffffff9b230000 in_suspend
> 779280: 0xffffffff9b230000 __nosave_begin
>
> because __smp_locks_end is in the section with the smaller address. It
> is placed earlier in the binary:
>
> 718847: ffffffff9b230000 0 NOTYPE GLOBAL DEFAULT 65 __nosave_begin
> 779597: ffffffff9b230000 4 OBJECT GLOBAL DEFAULT 65 in_suspend
> 792880: ffffffff9b230000 0 NOTYPE GLOBAL DEFAULT 63 __smp_locks_end
>
> It is in section 63 while the other two are in 65. Which leads to the
> first symbol to be selected in this case.
I see. Would you mind testing the following patch? I think it should
do as you want for OBJECT symbols, and fixes some other bugs I noticed.
There is almost certainly going to be some testsuite fallout for some
targets (mips-sgi-irix for one). I'll fix that with a followup patch.
* objdump.c (compare_section): New static var.
(compare_symbols): Sort by current section only. Don't access
symbol name out of bounds when checking for file symbols.
Sort section symbols and object symbols.
(find_symbol_for_address): Remove bogus debugging and section
symbol test.
(disassemble_data): Move symbol sort from here..
(disassemble_section): ..to here. Set compare_section.
diff --git a/binutils/objdump.c b/binutils/objdump.c
index c10136edc3..332b12167b 100644
--- a/binutils/objdump.c
+++ b/binutils/objdump.c
@@ -803,6 +803,8 @@ remove_useless_symbols (asymbol **symbols, long count)
return out_ptr - symbols;
}
+static const asection *compare_section;
+
/* Sort symbols into value order. */
static int
@@ -814,8 +816,7 @@ compare_symbols (const void *ap, const void *bp)
const char *bn;
size_t anl;
size_t bnl;
- bfd_boolean af;
- bfd_boolean bf;
+ bfd_boolean as, af, bs, bf;
flagword aflags;
flagword bflags;
@@ -824,10 +825,16 @@ compare_symbols (const void *ap, const void *bp)
else if (bfd_asymbol_value (a) < bfd_asymbol_value (b))
return -1;
- if (a->section > b->section)
- return 1;
- else if (a->section < b->section)
+ /* 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;
+ if (as && !bs)
return -1;
+ if (!as && bs)
+ return 1;
an = bfd_asymbol_name (a);
bn = bfd_asymbol_name (b);
@@ -853,7 +860,8 @@ compare_symbols (const void *ap, const void *bp)
#define file_symbol(s, sn, snl) \
(((s)->flags & BSF_FILE) != 0 \
- || ((sn)[(snl) - 2] == '.' \
+ || ((snl) > 2 \
+ && (sn)[(snl) - 2] == '.' \
&& ((sn)[(snl) - 1] == 'o' \
|| (sn)[(snl) - 1] == 'a')))
@@ -865,8 +873,8 @@ compare_symbols (const void *ap, const void *bp)
if (! af && bf)
return -1;
- /* Try to sort global symbols before local symbols before function
- symbols before debugging symbols. */
+ /* Sort function and object symbols before global symbols before
+ local symbols before section symbols before debugging symbols. */
aflags = a->flags;
bflags = b->flags;
@@ -878,6 +886,13 @@ compare_symbols (const void *ap, const void *bp)
else
return -1;
}
+ if ((aflags & BSF_SECTION_SYM) != (bflags & BSF_SECTION_SYM))
+ {
+ if ((aflags & BSF_SECTION_SYM) != 0)
+ return 1;
+ else
+ return -1;
+ }
if ((aflags & BSF_FUNCTION) != (bflags & BSF_FUNCTION))
{
if ((aflags & BSF_FUNCTION) != 0)
@@ -885,6 +900,13 @@ compare_symbols (const void *ap, const void *bp)
else
return 1;
}
+ if ((aflags & BSF_OBJECT) != (bflags & BSF_OBJECT))
+ {
+ if ((aflags & BSF_OBJECT) != 0)
+ return -1;
+ else
+ return 1;
+ }
if ((aflags & BSF_LOCAL) != (bflags & BSF_LOCAL))
{
if ((aflags & BSF_LOCAL) != 0)
@@ -1102,14 +1124,11 @@ find_symbol_for_address (bfd_vma vma,
/* The symbol we want is now in min, the low end of the range we
were searching. If there are several symbols with the same
- value, we want the first (non-section/non-debugging) one. */
+ value, we want the first one. */
thisplace = min;
while (thisplace > 0
&& (bfd_asymbol_value (sorted_syms[thisplace])
- == bfd_asymbol_value (sorted_syms[thisplace - 1]))
- && ((sorted_syms[thisplace - 1]->flags
- & (BSF_SECTION_SYM | BSF_DEBUGGING)) == 0)
- )
+ == bfd_asymbol_value (sorted_syms[thisplace - 1])))
--thisplace;
/* Prefer a symbol in the current section if we have multple symbols
@@ -2389,6 +2408,10 @@ disassemble_section (bfd *abfd, asection *section, void *inf)
pinfo->buffer_length = datasize;
pinfo->section = section;
+ /* Sort the symbols into value and section order. */
+ compare_section = section;
+ qsort (sorted_syms, sorted_symcount, sizeof (asymbol *), compare_symbols);
+
/* Skip over the relocs belonging to addresses below the
start address. */
while (rel_pp < rel_ppend
@@ -2632,9 +2655,6 @@ disassemble_data (bfd *abfd)
++sorted_symcount;
}
- /* Sort the symbols into section and symbol order. */
- qsort (sorted_syms, sorted_symcount, sizeof (asymbol *), compare_symbols);
-
init_disassemble_info (&disasm_info, stdout, (fprintf_ftype) fprintf);
disasm_info.application_data = (void *) &aux;
--
Alan Modra
Australia Development Lab, IBM