This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: PR 13897
PR 13897 is regarding a 100x slowdown in objdump -dS output for an old
dot-sym PowerpC64 binary, after a fix I made for bfd_find_nearest_line
on new PowerPC64 binaries. Nick added some caching in opd_entry_value
that certainly sped things up, but only because a bug made the cache
return the single cached function entry for all opd lookups. I fixed
the cache but then noticed that it was completely ineffective. In
fact, the cache further slowed down objdump -dS by a few percent.
My first attempt at fixing the problem removed the ineffective cache
and changed the interface to maybe_function_sym to only return true
for functions in the section of interest. That saved a loop over
sections in opd_entry_value, giving a 4x speedup. Not good enough.
We're still 25x slower than the original code which worked fine with
dot-sym binaries.
So I decided to add a cache in elf_find_function. objdump -dS calls
bfd_find_nearest_line on every instruction, and elf_find_function will
return the same function info for every instruction of a given
function. This gave around a 50x speedup, so now objdump -dS is
approximately 2x faster than it was before 2012-01-23.
PR binutils/13897
* elf.c (elf_find_function): Cache last function sym info.
(_bfd_elf_maybe_function_sym): Return function size, pass in
section of interest.
* elf-bfd.h (struct elf_backend_data <maybe_function_sym>): Likewise.
(_bfd_elf_maybe_function_sym): Likewise.
* elf64-ppc.c (ppc64_elf_maybe_function_sym): Likewise.
(opd_entry_value): Add in_code_sec param. Revert caching code.
Return -1 if in_code_sec and function found in wrong section.
Update all calls.
Index: bfd/elf-bfd.h
===================================================================
RCS file: /cvs/src/src/bfd/elf-bfd.h,v
retrieving revision 1.339
diff -u -p -r1.339 elf-bfd.h
--- bfd/elf-bfd.h 25 May 2012 01:12:19 -0000 1.339
+++ bfd/elf-bfd.h 2 Jun 2012 23:51:12 -0000
@@ -1222,10 +1222,11 @@ struct elf_backend_data
/* Return TRUE if type is a function symbol type. */
bfd_boolean (*is_function_type) (unsigned int type);
- /* Return TRUE if symbol may be a function. Set *CODE_SEC and *CODE_VAL
- to the function's entry point. */
- bfd_boolean (*maybe_function_sym) (const asymbol *sym,
- asection **code_sec, bfd_vma *code_off);
+ /* If the ELF symbol SYM might be a function in SEC, return the
+ function size and set *CODE_OFF to the function's entry point,
+ otherwise return zero. */
+ bfd_size_type (*maybe_function_sym) (const asymbol *sym, asection *sec,
+ bfd_vma *code_off);
/* Used to handle bad SHF_LINK_ORDER input. */
bfd_error_handler_type link_order_error_handler;
@@ -2207,8 +2208,8 @@ extern bfd_boolean _bfd_elf_map_sections
extern bfd_boolean _bfd_elf_is_function_type (unsigned int);
-extern bfd_boolean _bfd_elf_maybe_function_sym (const asymbol *,
- asection **, bfd_vma *);
+extern bfd_size_type _bfd_elf_maybe_function_sym (const asymbol *, asection *,
+ bfd_vma *);
extern int bfd_elf_get_default_section_type (flagword);
Index: bfd/elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.555
diff -u -p -r1.555 elf.c
--- bfd/elf.c 17 May 2012 06:29:02 -0000 1.555
+++ bfd/elf.c 2 Jun 2012 23:51:13 -0000
@@ -7407,59 +7407,74 @@ elf_find_function (bfd *abfd,
const char **filename_ptr,
const char **functionname_ptr)
{
- const char *filename;
- asymbol *func, *file;
- bfd_vma low_func;
- asymbol **p;
- /* ??? Given multiple file symbols, it is impossible to reliably
- choose the right file name for global symbols. File symbols are
- local symbols, and thus all file symbols must sort before any
- global symbols. The ELF spec may be interpreted to say that a
- file symbol must sort before other local symbols, but currently
- ld -r doesn't do this. So, for ld -r output, it is possible to
- make a better choice of file name for local symbols by ignoring
- file symbols appearing after a given local symbol. */
- enum { nothing_seen, symbol_seen, file_after_symbol_seen } state;
- const struct elf_backend_data *bed = get_elf_backend_data (abfd);
+ static asection *last_section;
+ static asymbol *func;
+ static const char *filename;
+ static bfd_size_type func_size;
if (symbols == NULL)
return FALSE;
- filename = NULL;
- func = NULL;
- file = NULL;
- low_func = 0;
- state = nothing_seen;
-
- for (p = symbols; *p != NULL; p++)
- {
- asymbol *sym = *p;
- asection *code_sec;
- bfd_vma code_off;
+ if (last_section != section
+ || func == NULL
+ || offset < func->value
+ || offset >= func->value + func_size)
+ {
+ asymbol *file;
+ bfd_vma low_func;
+ asymbol **p;
+ /* ??? Given multiple file symbols, it is impossible to reliably
+ choose the right file name for global symbols. File symbols are
+ local symbols, and thus all file symbols must sort before any
+ global symbols. The ELF spec may be interpreted to say that a
+ file symbol must sort before other local symbols, but currently
+ ld -r doesn't do this. So, for ld -r output, it is possible to
+ make a better choice of file name for local symbols by ignoring
+ file symbols appearing after a given local symbol. */
+ enum { nothing_seen, symbol_seen, file_after_symbol_seen } state;
+ const struct elf_backend_data *bed = get_elf_backend_data (abfd);
+
+ filename = NULL;
+ func = NULL;
+ file = NULL;
+ low_func = 0;
+ state = nothing_seen;
+ func_size = 0;
+ last_section = section;
- if ((sym->flags & BSF_FILE) != 0)
+ for (p = symbols; *p != NULL; p++)
{
- file = sym;
- if (state == symbol_seen)
- state = file_after_symbol_seen;
- continue;
- }
+ asymbol *sym = *p;
+ bfd_vma code_off;
+ bfd_size_type size;
- if (bed->maybe_function_sym (sym, &code_sec, &code_off)
- && code_sec == section
- && code_off >= low_func
- && code_off <= offset)
- {
- func = sym;
- low_func = code_off;
- filename = NULL;
- if (file != NULL
- && ((sym->flags & BSF_LOCAL) != 0
- || state != file_after_symbol_seen))
- filename = bfd_asymbol_name (file);
+ if ((sym->flags & BSF_FILE) != 0)
+ {
+ file = sym;
+ if (state == symbol_seen)
+ state = file_after_symbol_seen;
+ continue;
+ }
+
+ size = bed->maybe_function_sym (sym, section, &code_off);
+ if (size != 0
+ && code_off <= offset
+ && (code_off > low_func
+ || (code_off == low_func
+ && size > func_size)))
+ {
+ func = sym;
+ func_size = size;
+ low_func = code_off;
+ filename = NULL;
+ if (file != NULL
+ && ((sym->flags & BSF_LOCAL) != 0
+ || state != file_after_symbol_seen))
+ filename = bfd_asymbol_name (file);
+ }
+ if (state == nothing_seen)
+ state = symbol_seen;
}
- if (state == nothing_seen)
- state = symbol_seen;
}
if (func == NULL)
@@ -9714,18 +9729,26 @@ _bfd_elf_is_function_type (unsigned int
|| type == STT_GNU_IFUNC);
}
-/* Return TRUE iff the ELF symbol SYM might be a function. Set *CODE_SEC
- and *CODE_OFF to the function's entry point. */
-
-bfd_boolean
-_bfd_elf_maybe_function_sym (const asymbol *sym,
- asection **code_sec, bfd_vma *code_off)
+/* If the ELF symbol SYM might be a function in SEC, return the
+ function size and set *CODE_OFF to the function's entry point,
+ otherwise return zero. */
+
+bfd_size_type
+_bfd_elf_maybe_function_sym (const asymbol *sym, asection *sec,
+ bfd_vma *code_off)
{
+ bfd_size_type size;
+
if ((sym->flags & (BSF_SECTION_SYM | BSF_FILE | BSF_OBJECT
- | BSF_THREAD_LOCAL | BSF_RELC | BSF_SRELC)) != 0)
- return FALSE;
+ | BSF_THREAD_LOCAL | BSF_RELC | BSF_SRELC)) != 0
+ || sym->section != sec)
+ return 0;
- *code_sec = sym->section;
*code_off = sym->value;
- return TRUE;
+ size = 0;
+ if (!(sym->flags & BSF_SYNTHETIC))
+ size = ((elf_symbol_type *) sym)->internal_elf_sym.st_size;
+ if (size == 0)
+ size = 1;
+ return size;
}
Index: bfd/elf64-ppc.c
===================================================================
RCS file: /cvs/src/src/bfd/elf64-ppc.c,v
retrieving revision 1.386
diff -u -p -r1.386 elf64-ppc.c
--- bfd/elf64-ppc.c 1 Jun 2012 12:26:55 -0000 1.386
+++ bfd/elf64-ppc.c 3 Jun 2012 03:39:12 -0000
@@ -55,7 +55,7 @@ static bfd_reloc_status_type ppc64_elf_t
static bfd_reloc_status_type ppc64_elf_unhandled_reloc
(bfd *, arelent *, asymbol *, void *, asection *, bfd *, char **);
static bfd_vma opd_entry_value
- (asection *, bfd_vma, asection **, bfd_vma *);
+ (asection *, bfd_vma, asection **, bfd_vma *, bfd_boolean);
#define TARGET_LITTLE_SYM bfd_elf64_powerpcle_vec
#define TARGET_LITTLE_NAME "elf64-powerpcle"
@@ -2347,7 +2347,7 @@ ppc64_elf_branch_reloc (bfd *abfd, arele
{
bfd_vma dest = opd_entry_value (symbol->section,
symbol->value + reloc_entry->addend,
- NULL, NULL);
+ NULL, NULL, FALSE);
if (dest != (bfd_vma) -1)
reloc_entry->addend = dest - (symbol->value
+ symbol->section->output_section->vma
@@ -5522,7 +5522,8 @@ static bfd_vma
opd_entry_value (asection *opd_sec,
bfd_vma offset,
asection **code_sec,
- bfd_vma *code_off)
+ bfd_vma *code_off,
+ bfd_boolean in_code_sec)
{
bfd *opd_bfd = opd_sec->owner;
Elf_Internal_Rela *relocs;
@@ -5533,43 +5534,39 @@ opd_entry_value (asection *opd_sec,
at a final linked executable with addr2line or somesuch. */
if (opd_sec->reloc_count == 0)
{
- static asection *last_opd_sec, *last_code_sec;
- static bfd_vma last_opd_off, last_entry_vma;
- static bfd_boolean sec_search_done;
-
- if (last_opd_sec != opd_sec
- || last_opd_off != offset
- || (code_sec != NULL && !sec_search_done))
- {
- char buf[8];
-
- if (!bfd_get_section_contents (opd_bfd, opd_sec, buf, offset, 8))
- return (bfd_vma) -1;
-
- last_opd_sec = opd_sec;
- last_opd_off = offset;
- last_entry_vma = bfd_get_64 (opd_bfd, buf);
- sec_search_done = FALSE;
- if (code_sec != NULL)
- {
- asection *sec;
+ char buf[8];
+
+ if (!bfd_get_section_contents (opd_bfd, opd_sec, buf, offset, 8))
+ return (bfd_vma) -1;
- sec_search_done = TRUE;
- last_code_sec = NULL;
- for (sec = opd_bfd->sections; sec != NULL; sec = sec->next)
- if (sec->vma <= last_entry_vma
- && (sec->flags & SEC_LOAD) != 0
- && (sec->flags & SEC_ALLOC) != 0)
- last_code_sec = sec;
- }
- }
- if (code_sec != NULL && last_code_sec != NULL)
- {
- *code_sec = last_code_sec;
- if (code_off != NULL)
- *code_off = last_entry_vma - last_code_sec->vma;
+ val = bfd_get_64 (opd_bfd, buf);
+ if (code_sec != NULL)
+ {
+ asection *sec, *likely = NULL;
+
+ if (in_code_sec)
+ {
+ sec = *code_sec;
+ if (sec->vma <= val
+ && val < sec->vma + sec->size)
+ likely = sec;
+ else
+ val = -1;
+ }
+ else
+ for (sec = opd_bfd->sections; sec != NULL; sec = sec->next)
+ if (sec->vma <= val
+ && (sec->flags & SEC_LOAD) != 0
+ && (sec->flags & SEC_ALLOC) != 0)
+ likely = sec;
+ if (likely != NULL)
+ {
+ *code_sec = likely;
+ if (code_off != NULL)
+ *code_off = val - likely->vma;
+ }
}
- return last_entry_vma;
+ return val;
}
BFD_ASSERT (is_ppc64_elf (opd_bfd));
@@ -5640,7 +5637,12 @@ opd_entry_value (asection *opd_sec,
if (code_off != NULL)
*code_off = val;
if (code_sec != NULL)
- *code_sec = sec;
+ {
+ if (in_code_sec && *code_sec != sec)
+ return -1;
+ else
+ *code_sec = sec;
+ }
if (sec != NULL && sec->output_section != NULL)
val += sec->output_section->vma + sec->output_offset;
}
@@ -5651,20 +5653,53 @@ opd_entry_value (asection *opd_sec,
return val;
}
-/* Return TRUE iff the ELF symbol SYM might be a function. Set *CODE_SEC
- and *CODE_OFF to the function's entry point. */
-
-static bfd_boolean
-ppc64_elf_maybe_function_sym (const asymbol *sym,
- asection **code_sec, bfd_vma *code_off)
+/* If the ELF symbol SYM might be a function in SEC, return the
+ function size and set *CODE_OFF to the function's entry point,
+ otherwise return zero. */
+
+static bfd_size_type
+ppc64_elf_maybe_function_sym (const asymbol *sym, asection *sec,
+ bfd_vma *code_off)
{
- if (_bfd_elf_maybe_function_sym (sym, code_sec, code_off))
+ bfd_size_type size;
+
+ if ((sym->flags & (BSF_SECTION_SYM | BSF_FILE | BSF_OBJECT
+ | BSF_THREAD_LOCAL | BSF_RELC | BSF_SRELC)) != 0)
+ return 0;
+
+ size = 0;
+ if (!(sym->flags & BSF_SYNTHETIC))
+ size = ((elf_symbol_type *) sym)->internal_elf_sym.st_size;
+
+ if (strcmp (sym->section->name, ".opd") == 0)
{
- if (strcmp (sym->section->name, ".opd") == 0)
- opd_entry_value (sym->section, sym->value, code_sec, code_off);
- return TRUE;
+ if (opd_entry_value (sym->section, sym->value,
+ &sec, code_off, TRUE) == (bfd_vma) -1)
+ return 0;
+ /* An old ABI binary with dot-syms has a size of 24 on the .opd
+ symbol. This size has nothing to do with the code size of the
+ function, which is what we're supposed to return, but the
+ code size isn't available without looking up the dot-sym.
+ However, doing that would be a waste of time particularly
+ since elf_find_function will look at the dot-sym anyway.
+ Now, elf_find_function will keep the largest size of any
+ function sym found at the code address of interest, so return
+ 1 here to avoid it incorrectly caching a larger function size
+ for a small function. This does mean we return the wrong
+ size for a new-ABI function of size 24, but all that does is
+ disable caching for such functions. */
+ if (size == 24)
+ size = 1;
}
- return FALSE;
+ else
+ {
+ if (sym->section != sec)
+ return 0;
+ *code_off = sym->value;
+ }
+ if (size == 0)
+ size = 1;
+ return size;
}
/* Return true if symbol is defined in a regular object file. */
@@ -5744,7 +5779,7 @@ ppc64_elf_gc_keep (struct bfd_link_info
else if (get_opd_info (eh->elf.root.u.def.section) != NULL
&& opd_entry_value (eh->elf.root.u.def.section,
eh->elf.root.u.def.value,
- &sec, NULL) != (bfd_vma) -1)
+ &sec, NULL, FALSE) != (bfd_vma) -1)
sec->flags |= SEC_KEEP;
sec = eh->elf.root.u.def.section;
@@ -5795,7 +5830,7 @@ ppc64_elf_gc_mark_dynamic_ref (struct el
else if (get_opd_info (eh->elf.root.u.def.section) != NULL
&& opd_entry_value (eh->elf.root.u.def.section,
eh->elf.root.u.def.value,
- &code_sec, NULL) != (bfd_vma) -1)
+ &code_sec, NULL, FALSE) != (bfd_vma) -1)
code_sec->flags |= SEC_KEEP;
}
@@ -5855,7 +5890,7 @@ ppc64_elf_gc_mark_hook (asection *sec,
else if (get_opd_info (eh->elf.root.u.def.section) != NULL
&& opd_entry_value (eh->elf.root.u.def.section,
eh->elf.root.u.def.value,
- &rsec, NULL) != (bfd_vma) -1)
+ &rsec, NULL, FALSE) != (bfd_vma) -1)
eh->elf.root.u.def.section->gc_mark = 1;
else
rsec = h->root.u.def.section;
@@ -6324,7 +6359,7 @@ func_desc_adjust (struct elf_link_hash_e
&& opd_entry_value (fdh->elf.root.u.def.section,
fdh->elf.root.u.def.value,
&fh->elf.root.u.def.section,
- &fh->elf.root.u.def.value) != (bfd_vma) -1)
+ &fh->elf.root.u.def.value, FALSE) != (bfd_vma) -1)
{
fh->elf.root.type = fdh->elf.root.type;
fh->elf.forced_local = 1;
@@ -10915,7 +10950,8 @@ toc_adjusting_stub_needed (struct bfd_li
sym_value += adjust;
}
- dest = opd_entry_value (sym_sec, sym_value, &sym_sec, NULL);
+ dest = opd_entry_value (sym_sec, sym_value,
+ &sym_sec, NULL, FALSE);
if (dest == (bfd_vma) -1)
continue;
}
@@ -11490,7 +11526,7 @@ ppc64_elf_size_stubs (struct bfd_link_in
sym_value += adjust;
}
dest = opd_entry_value (sym_sec, sym_value,
- &code_sec, &code_value);
+ &code_sec, &code_value, FALSE);
if (dest != (bfd_vma) -1)
{
destination = dest;
@@ -12904,7 +12940,7 @@ ppc64_elf_relocate_section (bfd *output_
bfd_vma off = (relocation + addend
- sec->output_section->vma
- sec->output_offset);
- bfd_vma dest = opd_entry_value (sec, off, NULL, NULL);
+ bfd_vma dest = opd_entry_value (sec, off, NULL, NULL, FALSE);
if (dest != (bfd_vma) -1)
{
relocation = dest;
--
Alan Modra
Australia Development Lab, IBM