[PATCH] D150220: [lld]: Fix RISC-V relaxation bug

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

> When multiple symbol names alias to the same symbol (which is the case when we use --wrap),

This is the uncommon in `wrap.s`, where `foo` and `__wrap_foo` are defined in one file. This is not how `--wrap` is used in the wild, but if we identify a bug, we should fix it.

> the RISC-V relaxation steps are executed multiple times for the same symbol, causing erroneous addresses being assigned to symbols.

A relocatable object file (`%t2`) indeed has multiple instances of the same symbol. Can you describe how this leads to incorrect addresses?

For `riscv-wrap.s`, the `%t3` is identical without or with this patch.



================
Comment at: lld/test/ELF/Inputs/riscv-wrap.s:3
+.weak __wrap_foo 
+.protected __wrap_foo
+.global __real_foo
----------------
Omit `__real_foo`. Having a `.globl` for an undefined symbol is a no-op, but not conventional.

Avoid `.protected` if STV_PROTECTED doesn't lead to behavior differences.

Add a comment before `.align 5`.

`nop` below is not indented.


================
Comment at: lld/test/ELF/riscv-wrap.s:2
+# REQUIRES: riscv 
+## Test RISCV's handling of --wrap when R_RISCV_ALIGN is present 
+# RUN: rm -rf %t && mkdir %t && cd %t 
----------------
The test case copied from `wrap.s` needs some polishing. You may find some recent tests that use a IMHO cleaner style:

* `.o` for relocatable object files. `%t.o` and `%t` to make the primary relocatable object file name match the linker output file name.
* prefer `split-file` when it makes the test more readable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150220/new/

https://reviews.llvm.org/D150220