Bug 28509 - ld riscv: R_RISCV_JAL referencing a preemptible symbol should be rejected
Summary: ld riscv: R_RISCV_JAL referencing a preemptible symbol should be rejected
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Nelson Chu
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-10-28 01:00 UTC by Fangrui Song
Modified: 2022-09-12 05:24 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Fangrui Song 2021-10-28 01:00:50 UTC
j __sigsetjmp

.globl __sigsetjmp
__sigsetjmp:
  ret

riscv64-linux-gnu-as a.s -o a.o
riscv64-linux-gnu-ld -shared a.o -o a.so  # should fail


The default visibility __sigsetjmp definition is preemptible by default when linking a shared object. Such a symbol cannot be referenced by non-PLT by-GOT.


% riscv64-linux-gnu-objdump -d a.so

a.so:     file format elf64-littleriscv


Disassembly of section .text:

00000000000001b8 <__sigsetjmp-0x4>:
 1b8:   0040006f                j       1bc <__sigsetjmp>

00000000000001bc <__sigsetjmp>:
 1bc:   00008067                ret

On many architectures there is a diagnostic like R_X86_64_PC32.
Comment 1 Fangrui Song 2021-10-28 18:42:58 UTC
glibc commit https://sourceware.org/git/?p=glibc.git;a=commit;h=68389203832ab39dd0dbaabbc4059e7fff51c29b ("riscv: Fix incorrect jal with HIDDEN_JUMPTARGET") could have been avoided if such an error had been implemented.
Comment 2 Nelson Chu 2022-09-05 09:49:44 UTC
It probably worth that back to see this one, I totally forgot it...
https://sourceware.org/pipermail/binutils/2021-November/118398.html
Comment 3 cvs-commit@gcc.gnu.org 2022-09-12 03:58:29 UTC
The master branch has been updated by Nelson Chu <nelsonc1225@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ecb915b4de7569027ad78bd3e24873bb92cb8e32

commit ecb915b4de7569027ad78bd3e24873bb92cb8e32
Author: Nelson Chu <nelson@rivosinc.com>
Date:   Mon Sep 12 09:26:52 2022 +0800

    RISC-V: PR28509, the default visibility symbol cannot be referenced by R_RISCV_JAL.
    
    When generating the shared object, the default visibility symbols may bind
    externally, which means they will be exported to the dynamic symbol table,
    and are preemptible by default.  These symbols cannot be referenced by the
    non-pic R_RISCV_JAL and R_RISCV_RVC_JUMP.  However, consider that linker
    may relax the R_RISCV_CALL relocations to R_RISCV_JAL or R_RISCV_RVC_JUMP,
    if these relocations are relocated to the plt entries, then we won't report
    error for them.  Perhaps we also need the similar checks for the
    R_RISCV_BRANCH and R_RISCV_RVC_BRANCH relocations.
    
    After applying this patch, and revert the following glibc patch,
    riscv: Fix incorrect jal with HIDDEN_JUMPTARGET
    https://sourceware.org/git/?p=glibc.git;a=commit;h=68389203832ab39dd0dbaabbc4059e7fff51c29b
    
    I get the expected errors as follows,
    ld: relocation R_RISCV_RVC_JUMP against `__sigsetjmp' which may bind externally can not be used when making a shared object; recompile with -fPIC
    ld: relocation R_RISCV_JAL against `exit' which may bind externally can not be used when making a shared object; recompile with -fPIC
    
    Besides, we also have similar changes for libgcc,
    RISC-V: jal cannot refer to a default visibility symbol for shared object
    https://github.com/gcc-mirror/gcc/commit/45116f342057b7facecd3d05c2091ce3a77eda59
    
    bfd/
            pr 28509
            * elfnn-riscv.c (riscv_elf_relocate_section): Report errors when
            makeing a shard object, and the referenced symbols of R_RISCV_JAL
            relocations are default visibility.  Besides, we should handle most
            of the cases here, so don't need the unresolvable check later for
            R_RISCV_JAL and R_RISCV_RVC_JUMP.
    ld/
            pr 28509
            * testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated.
            * testsuite/ld-riscv-elf/lib-nopic-01a.s: Removed.
            * testsuite/ld-riscv-elf/lib-nopic-01b.d: Likewise.
            * testsuite/ld-riscv-elf/lib-nopic-01b.s: Likewise.
            * testsuite/ld-riscv-elf/shared-lib-nopic-01.d: New testcase.
            * testsuite/ld-riscv-elf/shared-lib-nopic-01.s: Likewise.
            * testsuite/ld-riscv-elf/shared-lib-nopic-02.d: Likewise.
            * testsuite/ld-riscv-elf/shared-lib-nopic-02.s: Likewise.
            * testsuite/ld-riscv-elf/shared-lib-nopic-03.d: Likewise.
            * testsuite/ld-riscv-elf/shared-lib-nopic-03.s: Likewise.
            * testsuite/ld-riscv-elf/shared-lib-nopic-04.d: Likewise.
            * testsuite/ld-riscv-elf/shared-lib-nopic-04.s: Likewise.
Comment 4 Nelson Chu 2022-09-12 05:24:33 UTC
Marked as RESOLVED and FIXED, since we should have fixed it in gcc, glibc, ld and lld.