[PATCH] RISC-V: Search for mapping symbols from the last one found

[Nelson Chu]

There is a similar (n >= last_map_symbol) check when we need to backtrace
searching,
https://github.com/bminor/binutils-gdb/blob/master/opcodes/riscv-dis.c#L1109,
should we also need to fix this?

diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c

index 684098d386a..e6596c47423 100644

--- a/opcodes/riscv-dis.c

+++ b/opcodes/riscv-dis.c

@@ -1076,11 +1076,9 @@ riscv_search_mapping_symbol (bfd_vma memaddr,

   from_last_map_symbol = (last_map_symbol >= 0
                          && info->stop_offset == last_stop_offset);

-  /* Start scanning at the start of the function, or wherever

-     we finished last time.  */

-  n = info->symtab_pos + 1;

-  if (from_last_map_symbol && n >= last_map_symbol)

-    n = last_map_symbol;

+  /* Start scanning from wherever we finished last time, or the start

+     of the function.  */

+  n = from_last_map_symbol ? last_map_symbol : info->symtab_pos + 1;


   /* Find the suitable mapping symbol to dump.  */
   for (; n < info->symtab_size; n++)
@@ -1105,9 +1103,7 @@ riscv_search_mapping_symbol (bfd_vma memaddr,

      can pick up a text mapping symbol of a preceeding section.  */
   if (!found)
     {
-      n = info->symtab_pos;

-      if (from_last_map_symbol && n >= last_map_symbol)

-       n = last_map_symbol;

+      n = from_last_map_symbol ? last_map_symbol : info->symtab_pos;


       for (; n >= 0; n--)

On Mon, May 6, 2024 at 11:12 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:

> On Fri, 03 May 2024 05:56:47 PDT (-0700), Joseph.Faulls@imgtec.com wrote:
> > Ping. I know it's a small patch, but it sped up disassembly of my use
> case from 5 minutes to 5 seconds.
> > ________________________________
> > From: Joseph Faulls
> > Sent: Thursday, April 18, 2024 11:48 AM
> > To: binutils@sourceware.org <binutils@sourceware.org>
> > Cc: nelson@rivosinc.com <nelson@rivosinc.com>
> > Subject: [PATCH] RISC-V: Search for mapping symbols from the last one
> found
> >
> > With previous behaviour, multiple mapping symbols within the same
> > function would result in all the mapping symbols being searched.
> > This could slow down disassembly dramatically.
> >
> > Multiple mapping symbols within a function can be a result of encoding
> > instructions as data, like sometimes seen in random instruction
> > generators.
> >
> > opcodes/ChangeLog:
> >
> >         * riscv-dis.c (riscv_search_mapping_symbol): Use last mapping
> >           symbol if it exists.
> >
> > Note for reviewers:
> > I don't see why the previous check 'n >= last_map_symbol exists'. Why do
> we force starting from the start of the function if the last mapping symbol
> was found after it? If I'm missing something, please let me know!
>
> Sorry for being slow here.  Nelson and I were talking a bit right before
> the weekend and it looks like this was just a bug, but it's very similar
> to what the Arm code does.
>

I guess that is because we ported the code from aarch64 at the beginning,
and then kept their code to fix the stuff probably this one,
https://github.com/bminor/binutils-gdb/commit/51457761649bab6868343b3da2258d73a62901f7.
But even applying Joseph's patch, the testcase from the above commit also
looks good, and I also passed the riscv-gnu-toolchain for the above changes
(two places use n >= last_map_symbol).  So it seems no reason to keep the
(n >= last_map_symbol) checks for now.

Nelson
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20240514/5e503c4a/attachment.htm>