[PATCH v2 2/2] RISC-V: Re-define mapping symbol $x to the file elf architecture attribute

[Nelson Chu]

To dump the objects from llvm, and fix the conflict with psABI, committed.

Thanks
Nelson

On Mon, Feb 24, 2025 at 3:38 PM Nelson Chu <nelson@rivosinc.com> wrote:

> From: Andrew Oates <andrew@andrewoates.com>
>
> The mapping symbol "$x" without an ISA string "means using ISA
> configuration from ELF attribute."[1].  Currently the code does not
> reset the subset_list.  This means that a previous mapping symbol that
> overrides the ISA string will continue to be used, rather than the
> default string set in the ELF file's .riscv.attributes section.  This
> can cause incorrect or failed instruction decodings.
>
> In practice, this causes problems when disassembling code generated by
> LLVM, which (unlike gas) does not emit explicit mapping symbols at the
> start of each section.
>
> This change stores the default architecture string seen at the beginning
> of disassembly in the global parse data struct, and restores that to
> subset_list whenever a bare "$x" symbol is seen.
>
> [1]
> https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc#mapping-symbol
>
> Before this patch, the mapping-x.s was dumped as,
>
> 00000000 <.text>:
>    0:   00000013                nop
>    4:   0001                    .insn   2, 0x0001
>    6:   0001                    .insn   2, 0x0001
>
> Which is caused by the definiation of $x was conflict with the psABI.
> ---
>  gas/testsuite/gas/riscv/mapping-x.d | 13 +++++++++++++
>  gas/testsuite/gas/riscv/mapping-x.s |  6 ++++++
>  opcodes/riscv-dis.c                 | 19 +++++++++++++------
>  3 files changed, 32 insertions(+), 6 deletions(-)
>  create mode 100644 gas/testsuite/gas/riscv/mapping-x.d
>  create mode 100644 gas/testsuite/gas/riscv/mapping-x.s
>
> diff --git a/gas/testsuite/gas/riscv/mapping-x.d
> b/gas/testsuite/gas/riscv/mapping-x.d
> new file mode 100644
> index 00000000000..ccf9ef762e7
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/mapping-x.d
> @@ -0,0 +1,13 @@
> +#as:
> +#source: mapping-x.s
> +#objdump: -d
> +
> +.*:[   ]+file format .*
> +
> +
> +Disassembly of section .text:
> +
> +0+000 <.text>:
> +[      ]+[0-9a-f]+:[   ]+00000013[     ]+nop
> +[      ]+[0-9a-f]+:[   ]+0001[         ]+nop
> +[      ]+[0-9a-f]+:[   ]+0001[         ]+nop
> diff --git a/gas/testsuite/gas/riscv/mapping-x.s
> b/gas/testsuite/gas/riscv/mapping-x.s
> new file mode 100644
> index 00000000000..f17cf2295ca
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/mapping-x.s
> @@ -0,0 +1,6 @@
> +.attribute arch, "rv32ic"
> +.option arch, -c
> +.insn 4, 0x00000013
> +$x:
> +.insn 2, 0x0001
> +.insn 2, 0x0001
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index 84c6deef7b6..be110a684b6 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -52,6 +52,9 @@ struct riscv_private_data
>    enum riscv_spec_class default_priv_spec;
>    /* Used for architecture parser.  */
>    riscv_parse_subset_t riscv_rps_dis;
> +  /* Default architecture string for the object file.  It will be changed
> once
> +     elf architecture attribute exits.  This is used for mapping symbol
> $x.  */
> +  const char* default_arch;
>    /* Used for mapping symbols.  */
>    int last_map_symbol;
>    bfd_vma last_stop_offset;
> @@ -1065,10 +1068,14 @@ riscv_update_map_state (int n,
>      return;
>
>    name = bfd_asymbol_name(info->symtab[n]);
> -  if (strcmp (name, "$x") == 0)
> -    *state = MAP_INSN;
> -  else if (strcmp (name, "$d") == 0)
> +  if (strcmp (name, "$d") == 0)
>      *state = MAP_DATA;
> +  else if (strcmp (name, "$x") == 0)
> +    {
> +      *state = MAP_INSN;
> +      riscv_release_subset_list (pd->riscv_rps_dis.subset_list);
> +      riscv_parse_subset (&pd->riscv_rps_dis, pd->default_arch);
> +    }
>    else if (strncmp (name, "$xrv", 4) == 0)
>      {
>        *state = MAP_INSN;
> @@ -1372,7 +1379,7 @@ riscv_init_disasm_info (struct disassemble_info
> *info)
>    pd->riscv_rps_dis.xlen = &pd->xlen;
>    pd->riscv_rps_dis.isa_spec = &pd->default_isa_spec;
>    pd->riscv_rps_dis.check_unknown_prefixed_ext = false;
> -  const char *default_arch = "rv64gc";
> +  pd->default_arch = "rv64gc";
>    if (info->section != NULL)
>      {
>        bfd *abfd = info->section->owner;
> @@ -1390,12 +1397,12 @@ riscv_init_disasm_info (struct disassemble_info
> *info)
>                                                       attr[Tag_b].i,
>                                                       attr[Tag_c].i,
>
> &pd->default_priv_spec);
> -             default_arch = attr[Tag_RISCV_arch].s;
> +             pd->default_arch = attr[Tag_RISCV_arch].s;
>             }
>         }
>      }
>    riscv_release_subset_list (pd->riscv_rps_dis.subset_list);
> -  riscv_parse_subset (&pd->riscv_rps_dis, default_arch);
> +  riscv_parse_subset (&pd->riscv_rps_dis, pd->default_arch);
>
>    pd->last_map_symbol = -1;
>    pd->last_stop_offset = 0;
> --
> 2.39.3 (Apple Git-146)
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20250303/a967948f/attachment.htm>