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

[via llvm-commits]

linsinan1995 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.
Will do. Thanks a ton for your detailed guide!

> 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.

Thats a really good point.

`func at plt` symbol is synthetic and they are not in symtab, thus it brings difficulty when analyzing some relocs, such as R_AARCH64_ADR_PREL_PG_HI21. D118088 could help for such cases. I think a better solution for this issue is that BOLT should only replace the address with the plt symbol for those plt related relocations, so non-preemptible cases will not be touched and and so weak undef cases as well. What do you think?


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