[PATCH 0/2] s390: Avoid relocation overflows on undefined weak symbols

[Jens Remus]

Hello Fangrui,

thank you for the feedback and hints at how the issue is dealt with by 
other architectures!

Am 26.06.2024 um 20:26 schrieb Fangrui Song:
> On Wed, Jun 26, 2024 at 8:20 AM Jens Remus <jremus@linux.ibm.com> wrote:
>> Patch 1 corrects commit 896a639babe2 ("s390: Avoid reloc overflows on
>> undefined weak symbols") not to replace Branch Relative on Count High
>> (brcth) referencing an undefined weak symbol definitively resolving to
>> zero by a trap, as it is not guaranteed that the conditional branch is
>> taken in any case.
>>
>> Patch 2 complements commit 896a639babe2 ("s390: Avoid reloc overflows
>> on undefined weak symbols") by applying similar replacements of
>> instructions referencing undefined weak symbols that definitively
>> resolve to zero. This time for PLT32DBL relocations.
...
>> Jens Remus (2):
>>    s390: Do not replace brcth referencing undefined weak symbol
>>    s390: Avoid reloc overflows on undefined weak symbols (cont)
>>
>>   bfd/elf64-s390.c                    | 42 ++++++++++++++++++++++++++---
>>   ld/testsuite/ld-s390/s390.exp       |  5 +++-
>>   ld/testsuite/ld-s390/weakundef-1.dd |  6 ++---
>>   ld/testsuite/ld-s390/weakundef-1.s  |  1 -
>>   ld/testsuite/ld-s390/weakundef-2.dd | 17 ++++++++++++
>>   ld/testsuite/ld-s390/weakundef-2.s  | 17 ++++++++++++
>>   6 files changed, 80 insertions(+), 8 deletions(-)
>>   create mode 100644 ld/testsuite/ld-s390/weakundef-2.dd
>>   create mode 100644 ld/testsuite/ld-s390/weakundef-2.s

> Other ports have simpler strategies for unresolved undefined weak symbols
> https://reviews.llvm.org/D103001
> 
> aarch64: GNU ld: rewrite the instruction to a NOP; ld.lld: branch to
> the next instruction
> mips: GNU ld: branch to the start of the text segment (?); ld.lld:
> branch to zero
> ppc32: GNU ld: rewrite the instruction to a NOP; ld.lld: branch to the
> current instruction
> ppc64: GNU ld: rewrite the instruction to a NOP; ld.lld: branch to the
> current instruction
> riscv: GNU ld: branch to the absolute zero address (with instruction rewriting)
> i386/x86_64: GNU ld/ld.lld: branch to the link-time zero address
> 
> I haven't checked the patch closely, but it seems to add quite a few lines.

I had a quick look at bfd.ld AArch64 and PPC64. As you listed they both 
replace branches with NOPs. Yet it is unclear to me how/where they deal 
with (function) pointers resolving to zero.

On s390x using a section start address greater than 4GB at link-time 
causes PC32DBL and PLT32DBL PC-relative relocations for undefined weak 
symbols that definitively resolve to zero to overflow.

So far we have identified three types of PC-relative instructions and 
decided to rewrite them as follows to not change the general behavior:
- PC-relative load address of zero are replaced by a non-PC-relative
   load address of zero. This ensures correctness of run-time checks
   for undef weak symbols that resolved to zero.
- PC-relative branches to zero are replaced by a trap. A branch to zero
   would result in an exception at run-time and so does a trap.
- PC-relative instructions that can be considered optional, such as 
PC-relative prefetch of zero, are replaced by a NOP.

While technically it might be ok to rewrite subject branches as a NOP, 
we think that using a trap is better for debugging, as a NOP might go 
undetected at run-time. Also the complexity of the logic to do so does 
not change whether to rewrite as NOP or trap.

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303) and z/VSE Support
+49-7031-16-1128 Office
jremus@de.ibm.com

IBM

IBM Deutschland Research & Development GmbH; Vorsitzender des 
Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der 
Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/