[llvm] [Bolt] fix a wrong relocation update issue with weak references (PR #69136)

Rafael Auler via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 5 18:18:40 PST 2023


rafaelauler wrote:

Regarding the formatting/descriptions on the PR:

Capitalize BOLT.  As a suggestion, I would also make the title shorter (like 50/72 rule format). Although we don't explicitly enforce this, a lot of devs use this because it looks nicer on git log. In BOLT, most of diffs are formatted like that.

For example:

```
[BOLT] Don't resolve weak undefs in data to PLT

In AArch64, RISCV, BOLT resolved weak undefs in data sections to a PLT
entry. Even though code refs to weak syms can go to a PLT entry, data
section should remain zero to support the pattern if (sym) sym() used
with weak syms. BOLT was breaking this. Fix it.
```

The first line of the commit message should be the title of the PR, while the remaining lines formatted to 72 columns are the the description of the PR.

BTW I was re-reading https://reviews.llvm.org/D118088 on why are we redirecting these relocs to PLT entries in the first place, and I can't really tell why. @yota9 do you remember why? The problem with that is that we could be pessimizing some .data references if the linker concluded they should go straight to the functions and is bypassing the PLT. By doing this we go to these optimized references and convert them back to go through the PLT again. In some instances this might change the intended behavior too if the application was linked with the intent to make a given symbol non-preemptible, but we go there and make it preemptible again by redirecting the reference to a PLT entry. We can't blindly redirect everything to a PLT entry, and this weak-undef case is just one example why doing so is incorrect.

https://github.com/llvm/llvm-project/pull/69136


More information about the llvm-commits mailing list