[PATCH] RISC-V: Fix BFD_RELOC_RISCV_PCREL_LO12_S patch issue

[Nelson Chu]

Hi,

Looks good, thanks.

If you are willing, please add the "Signed-off-by: your name <your email>"
in the commit message,
https://sourceware.org/pipermail/binutils/2022-October/123680.html

Or you can get the GNU copyright access from here,

https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=tree;f=doc/Copyright

You can fill one of them, and then send it to assign@gnu.org, to get your
copyright assignment.


Then we can commit your patch ;)


Thanks

Nelson

On Wed, May 29, 2024 at 4:00 PM Sun Sunny <sunny.sun@corelabtech.com> wrote:

> In commit dff565fcca8137954d6ad571ef39f6aec5c0429c, the fixups
> for PCREL_LO12_I and PCREL_LO12_S were mixed, so the "IMM"
> field were applied to incorrect position, this caused incorrect
> src registers to be encoded.
>
> gas/
>     * config/tc-riscv.c (md_apply_fix): Fix PCREL_LO12_S issue.
> gas/
>     * testsuite/gas/riscv/fixup-pcrel*: New tests.
> ---
>  gas/config/tc-riscv.c                 |  5 +++-
>  gas/testsuite/gas/riscv/fixup-pcrel.d | 41 +++++++++++++++++++++++++++
>  gas/testsuite/gas/riscv/fixup-pcrel.s | 11 +++++++
>  3 files changed, 56 insertions(+), 1 deletion(-)
>  create mode 100644 gas/testsuite/gas/riscv/fixup-pcrel.d
>  create mode 100644 gas/testsuite/gas/riscv/fixup-pcrel.s
>
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index 8d749581c1d..0acbd154b2a 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -4675,7 +4675,10 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg
> ATTRIBUTE_UNUSED)
>    {
>      bfd_vma target = entry->target;
>      bfd_vma value = target - entry->address;
> -    bfd_putl32 (bfd_getl32 (buf) | ENCODE_ITYPE_IMM (value), buf);
> +    if (fixP->fx_r_type == BFD_RELOC_RISCV_PCREL_LO12_S)
> +      bfd_putl32 (bfd_getl32 (buf) | ENCODE_STYPE_IMM (value), buf);
> +    else
> +      bfd_putl32 (bfd_getl32 (buf) | ENCODE_ITYPE_IMM (value), buf);
>      /* Relaxations should never be enabled by `.option relax'.  */
>      if (!riscv_opts.relax)
>        fixP->fx_done = 1;
> diff --git a/gas/testsuite/gas/riscv/fixup-pcrel.d
> b/gas/testsuite/gas/riscv/fixup-pcrel.d
> new file mode 100644
> index 00000000000..8ce71ddc0bb
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/fixup-pcrel.d
> @@ -0,0 +1,41 @@
> +#as: -march=rv32i -mabi=ilp32
> +#source: fixup-pcrel.s
> +#objdump: -dr
> +
> +.*:[ ]+file format elf32-littleriscv
> +
> +
> +Disassembly of section .text:
> +
> +00000000 <main>:
> +   0: 00000517           auipc a0,0x0
> + 0: R_RISCV_PCREL_HI20 insn
> + 0: R_RISCV_RELAX *ABS*
> +   4: 00051503           lh a0,0(a0) # 0 <main>
> + 4: R_RISCV_PCREL_LO12_I .L0
> + 4: R_RISCV_RELAX *ABS*
> +   8: 00000597           auipc a1,0x0
> + 8: R_RISCV_PCREL_HI20 insn+0x2
> + 8: R_RISCV_RELAX *ABS*+0x2
> +   c: 00059583           lh a1,0(a1) # 8 <main+0x8>
> + c: R_RISCV_PCREL_LO12_I .L0
> + c: R_RISCV_RELAX *ABS*
> +  10: 00000297           auipc t0,0x0
> + 10: R_RISCV_PCREL_HI20 .L1^B1
> + 10: R_RISCV_RELAX *ABS*
> +  14: 00a29823           sh a0,16(t0) # 20 <.L1^B1>
> + 14: R_RISCV_PCREL_LO12_S .L0
> + 14: R_RISCV_RELAX *ABS*
> +  18: 00000297           auipc t0,0x0
> + 18: R_RISCV_PCREL_HI20 .L1^B1+0x2
> + 18: R_RISCV_RELAX *ABS*+0x2
> +  1c: 00b29523           sh a1,10(t0) # 22 <.L1^B1+0x2>
> + 1c: R_RISCV_PCREL_LO12_S .L0
> + 1c: R_RISCV_RELAX *ABS*
> +
> +00000020 <.L1^B1>:
> +  20: 0de68693           addi a3,a3,222
> +  24: 00008067           ret
> +
> +00000028 <insn>:
> +  28: 14d68693           addi a3,a3,333
> diff --git a/gas/testsuite/gas/riscv/fixup-pcrel.s
> b/gas/testsuite/gas/riscv/fixup-pcrel.s
> new file mode 100644
> index 00000000000..0cb1f6e1eb1
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/fixup-pcrel.s
> @@ -0,0 +1,11 @@
> +.global main
> +main:
> +        lh a0, insn
> +        lh a1, insn+2
> +        sh a0, 1f, t0
> +        sh a1, 1f+2, t0
> +1:
> +        addi a3, a3, 222
> +        ret
> +insn:
> +        addi a3, a3, 333
> --
> 2.34.1
>

I think we can just update the fixup-local testcase to verify this patch.
Add sw after lw, and the above lh/sh should be enough.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20240530/cab57f7c/attachment-0001.htm>