<div dir="ltr"><div dir="ltr">There is a similar (n >= last_map_symbol) check when we need to backtrace searching, <a href="https://github.com/bminor/binutils-gdb/blob/master/opcodes/riscv-dis.c#L1109">https://github.com/bminor/binutils-gdb/blob/master/opcodes/riscv-dis.c#L1109</a>, should we also need to fix this?</div><div dir="ltr"><div><br><div>diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c                                                                    <br>index 684098d386a..e6596c47423 100644                                                                                     <br>--- a/opcodes/riscv-dis.c                                                                                                 <br>+++ b/opcodes/riscv-dis.c                                                                                                 <br>@@ -1076,11 +1076,9 @@ riscv_search_mapping_symbol (bfd_vma memaddr,                                                      <br>   from_last_map_symbol = (last_map_symbol >= 0<br>                          && info->stop_offset == last_stop_offset);<br><br>-  /* Start scanning at the start of the function, or wherever                                                            <br>-     we finished last time.  */                                                                                          <br>-  n = info->symtab_pos + 1;                                                                                              <br>-  if (from_last_map_symbol && n >= last_map_symbol)                                                                      <br>-    n = last_map_symbol;                                                                                                 <br>+  /* Start scanning from wherever we finished last time, or the start                                                    <br>+     of the function.  */                                                                                                <br>+  n = from_last_map_symbol ? last_map_symbol : info->symtab_pos + 1;                                                     <br><br>   /* Find the suitable mapping symbol to dump.  */<br>   for (; n < info->symtab_size; n++)<br>@@ -1105,9 +1103,7 @@ riscv_search_mapping_symbol (bfd_vma memaddr,                                                       <br>      can pick up a text mapping symbol of a preceeding section.  */<br>   if (!found)<br>     {<br>-      n = info->symtab_pos;                                                                                              <br>-      if (from_last_map_symbol && n >= last_map_symbol)                                                                  <br>-       n = last_map_symbol;                                                                                              <br>+      n = from_last_map_symbol ? last_map_symbol : info->symtab_pos;                                                     <br><br>       for (; n >= 0; n--)<br></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, May 6, 2024 at 11:12 PM Palmer Dabbelt <<a href="mailto:palmer@dabbelt.com">palmer@dabbelt.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Fri, 03 May 2024 05:56:47 PDT (-0700), <a href="mailto:Joseph.Faulls@imgtec.com" target="_blank">Joseph.Faulls@imgtec.com</a> wrote:<br>
> Ping. I know it's a small patch, but it sped up disassembly of my use case from 5 minutes to 5 seconds.<br>
> ________________________________<br>
> From: Joseph Faulls<br>
> Sent: Thursday, April 18, 2024 11:48 AM<br>
> To: <a href="mailto:binutils@sourceware.org" target="_blank">binutils@sourceware.org</a> <<a href="mailto:binutils@sourceware.org" target="_blank">binutils@sourceware.org</a>><br>
> Cc: <a href="mailto:nelson@rivosinc.com" target="_blank">nelson@rivosinc.com</a> <<a href="mailto:nelson@rivosinc.com" target="_blank">nelson@rivosinc.com</a>><br>
> Subject: [PATCH] RISC-V: Search for mapping symbols from the last one found<br>
><br>
> With previous behaviour, multiple mapping symbols within the same<br>
> function would result in all the mapping symbols being searched.<br>
> This could slow down disassembly dramatically.<br>
><br>
> Multiple mapping symbols within a function can be a result of encoding<br>
> instructions as data, like sometimes seen in random instruction<br>
> generators.<br>
><br>
> opcodes/ChangeLog:<br>
><br>
>         * riscv-dis.c (riscv_search_mapping_symbol): Use last mapping<br>
>           symbol if it exists.<br>
><br>
> Note for reviewers:<br>
> 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!<br>
<br>
Sorry for being slow here.  Nelson and I were talking a bit right before <br>
the weekend and it looks like this was just a bug, but it's very similar <br>
to what the Arm code does.<br></blockquote><div><br></div><div>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, <a href="https://github.com/bminor/binutils-gdb/commit/51457761649bab6868343b3da2258d73a62901f7">https://github.com/bminor/binutils-gdb/commit/51457761649bab6868343b3da2258d73a62901f7</a>.  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.</div><div><br></div><div>Nelson</div></div></div>