[PATCH v7 1/5] RISC-V: Print highest address on disassembler

[Nelson Chu]

Hi Tsukasa,

I think there are three dis-assembler issues here,

1. PR29342
2. The target address of jump
3. Should we show the target address when it is -1

Therefore, three separate patches looks reasonable.  But I would suggest
that we should add test cases for each patch as possible as we can, rather
than add them together at last.  That is because it is really hard to make
sure the correctness of the separate patches.

This patch refers to issue 3.  So according to the source code, we won't
show the final target address when pd->print_addr is -1, which means
the address is exactly -1 or it is the default value.  I think it is really
rare to jump or refer to the symbol whose value is -1.  Besides that, not
showing the target address when it's value is -1 doesn't look wrong to me,
so personally I would like to keep the original behavior.  Unless there are
other users who really want to change the behavior, or llvm are doing
something different.  However, the change of gp makes sense, so it looks to
me.

Thanks
Nelson

On Wed, Aug 24, 2022 at 9:28 AM Tsukasa OI via Binutils <
binutils@sourceware.org> wrote:

> This patch makes possible to print the highest address (0xffffffff on RV32,
> 0xffffffff_ffffffff on RV64).  This is particularly useful if the highest
> address space is used for I/O registers and corresponding symbols
> are defined.
>
> opcodes/ChangeLog:
>
>         * riscv-dis.c (struct riscv_private_data): Add `to_print_addr' and
>         `has_gp' to enable printing the highest address.
>         (maybe_print_address): Utilize `to_print_addr' and `has_gp'.
>         (riscv_disassemble_insn): Likewise.
> ---
>  opcodes/riscv-dis.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index 164fd209dbd..c6d80c3ba49 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -52,6 +52,8 @@ struct riscv_private_data
>    bfd_vma gp;
>    bfd_vma print_addr;
>    bfd_vma hi_addr[OP_MASK_RD + 1];
> +  bool to_print_addr;
> +  bool has_gp;
>  };
>
>  /* Used for mapping symbols.  */
> @@ -177,10 +179,13 @@ maybe_print_address (struct riscv_private_data *pd,
> int base_reg, int offset,
>        pd->print_addr = (base_reg != 0 ? pd->hi_addr[base_reg] : 0) +
> offset;
>        pd->hi_addr[base_reg] = -1;
>      }
> -  else if (base_reg == X_GP && pd->gp != (bfd_vma)-1)
> +  else if (base_reg == X_GP && pd->has_gp)
>      pd->print_addr = pd->gp + offset;
>    else if (base_reg == X_TP || base_reg == 0)
>      pd->print_addr = offset;
> +  else
> +    return;
> +  pd->to_print_addr = true;
>
>    /* Sign-extend a 32-bit value to a 64-bit value.  */
>    if (wide)
> @@ -595,14 +600,19 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t
> word, disassemble_info *info)
>        int i;
>
>        pd = info->private_data = xcalloc (1, sizeof (struct
> riscv_private_data));
> -      pd->gp = -1;
> -      pd->print_addr = -1;
> +      pd->gp = 0;
> +      pd->print_addr = 0;
>        for (i = 0; i < (int)ARRAY_SIZE (pd->hi_addr); i++)
>         pd->hi_addr[i] = -1;
> +      pd->to_print_addr = false;
> +      pd->has_gp = false;
>
>        for (i = 0; i < info->symtab_size; i++)
>         if (strcmp (bfd_asymbol_name (info->symtab[i]), RISCV_GP_SYMBOL)
> == 0)
> -         pd->gp = bfd_asymbol_value (info->symtab[i]);
> +         {
> +           pd->gp = bfd_asymbol_value (info->symtab[i]);
> +           pd->has_gp = true;
> +         }
>      }
>    else
>      pd = info->private_data;
> @@ -662,13 +672,13 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t
> word, disassemble_info *info)
>           print_insn_args (op->args, word, memaddr, info);
>
>           /* Try to disassemble multi-instruction addressing sequences.  */
> -         if (pd->print_addr != (bfd_vma)-1)
> +         if (pd->to_print_addr)
>             {
>               info->target = pd->print_addr;
>               (*info->fprintf_styled_func)
>                 (info->stream, dis_style_comment_start, " # ");
>               (*info->print_address_func) (info->target, info);
> -             pd->print_addr = -1;
> +             pd->to_print_addr = false;
>             }
>
>           /* Finish filling out insn_info fields.  */
> --
> 2.34.1
>
>