[PATCH] RISC-V: Update ABI to the elf_flags after parsing elf attributes.

Jim Wilson jimw@sifive.com
Wed Oct 21 23:24:12 GMT 2020


On Mon, Oct 19, 2020 at 8:39 PM Nelson Chu <nelson.chu@sifive.com> wrote:
> Originally, if the -mabi option isn't set, then assembler will set the
> abi according to the architecture string in the riscv_after_parse_args.
> But we should also check and reset the abi later since the architecture
> string may be reset by the elf attributes.  Therefore, set the abi to
> the elf_flags in the riscv_after_parse_args seems too early, we can set
> them in the riscv_elf_final_processing to make sure that the architecture
> string won't be changed.

It is OK to delay setting the ELF header flags but you are also delaying
sets to abi_xlen, float_abi, and rve_abi.  float_abi and rve_abi are
currently only used for setting ELF header flags so that is OK.  But
abi_xlen is used in some macro instructions to determine whether we emit a
lw or ld instruction.

So if I modify one of your testcases to get this

 .attribute arch,"rv64i"
.option pic
.extern foo
la a0, foo

and assemble with -march=rv32ifd and disassemble I get

   0: 00000517           auipc a0,0x0
0: R_RISCV_GOT_HI20 foo
   4: 00052503           lw a0,0(a0) # 0 <.text>
4: R_RISCV_PCREL_LO12_I .L0
4: R_RISCV_RELAX *ABS*

which is wrong because 64-bit code should use ld.  This fails both with and
without your patch.  It fails without your patch because abi_xlen is
wrongly set to 32.  And it fails with your patch because abi_xlen is now 0
when we check it.

We already have code to give an error if an attribute arch follows an
instruction.  This uses the start_assemble variable.  So it would be safe
to call riscv_set_abi_by_arch at the place where we set start_assemble to
TRUE.  If I try that, my modified testcase works.  I had to add a
declaration.  Or you could move functions around.  Anyways, I think this
all looks OK with this one change.

Jim


More information about the Binutils mailing list