[Bug ld/31318] RISC-V 32-bit relocations truncated silently

[palmer at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31318

Palmer Dabbelt <palmer at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |palmer at gcc dot gnu.org
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2024-01-30

--- Comment #1 from Palmer Dabbelt <palmer at gcc dot gnu.org> ---
(In reply to Joseph Faulls from comment #0)
> Hi,
> 
> It seems handling relocations of type R_RISCV_32 does not error if there is
> a truncation. This does not seem right to me.
> 
> This was discovered when placing a program at very high addresses (>
> max_int), and the relocations emitted by the (clang) compiler in a jump
> table (from an optimised switch-case) in the data section were truncated.
> This resulted in a run-time failure. Supplying -mcmodel=medany solves this.
> 
> Reproducing test case:
> 
> test.s:
>         .macro  DATA symbol
>         .word   \symbol
>         .endm
>         DATA    abs
> 
> $ as test.s -march=rv64i -mabi=lp64 -defsym __abs__=1 -o test.o
> $ ld -melf64lriscv -Ttext 0x8000 --defsym _start=0x0 --defsym
> abs=0xc00005000 --defsym abs_local=0x200 test.o -o test.elf
> $ objdump -dr test.elf
> 
> Disassembly of section .text:
> 
> 0000000000008000 <__DATA_BEGIN__-0x1004>:
>     8000:       00005000                .word   0x00005000
> 
> Should this not error as we're truncating the address?

Seems reasonable to me.  It's also what x86 does for a similar test case

    $ cat test.s
    .set symbol, 0x100000000
    .data
    .word symbol

    .text
    .global main
    main:
    $ gcc test.s -o test
    test.s: Assembler messages:
    test.s:3: Warning: value 0x100000000 truncated to 0x0
    ...
    ... spew about executable stacks
    ...

> I've spent a while debugging, but this is the best fix I can come up with:
> 
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index 8b27e3b8d6a..91a552a3dc2 100644
> --- a/bfd/elfnn-riscv.c
> +++ b/bfd/elfnn-riscv.c
> @@ -1866,6 +1866,9 @@ perform_relocation (const reloc_howto_type *howto,
> 
>      case R_RISCV_32:
>      case R_RISCV_64:
> +      if (value & ~howto->dst_mask)
> +        return bfd_reloc_overflow;
> +      break;
>      case R_RISCV_ADD8:
>      case R_RISCV_ADD16:
>      case R_RISCV_ADD32:
> 
> However, I don't really know if this is a reasonable fix. What do people
> think?

It seems generally OK to me.  We're not always the best about {src,dst}_mask
being accurate, but it looks fine for these.  So probably best to send a patch.
 Unfortunately we just released, but it should be an easy backport.

> 
> Thanks,
> Joe

-- 
You are receiving this mail because:
You are on the CC list for the bug.