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

[Rafael Auler via llvm-commits]

rafaelauler wrote:

Thanks for the context, @yota9. I missed the check that guards on the symbol having no address, which makes this code much more restricted than I previously thought. But I wonder which symbols would have no address, and still be strong definitions (non-weak)? I was trying to understand why is this if() necessary, specially after the fix in this patch.

Another thing I was discussing with Maksim yesterday: Maksim pointed out that we should be skipping processing static relocations in places that also have dynamic relocations, and this weak ref case is an example that _should_ have a dynamic relocation. After all, the weak symbol pattern is precisely one in which the runtime can modify it by resolving the weak undef, so BOLT has no business in updating a reference that will be resolved at runtime. But if we look at https://github.com/llvm/llvm-project/blob/main/bolt/lib/Rewrite/RewriteInstance.cpp#L2451 we only do this for non-AArch64, because of https://reviews.llvm.org/D122100.

Now, we have this exception that AArch64 processes static relocs in places that have dynamic relocs in the same place because AArch64 uses R_AARCH64_RELATIVE in .rela.dyn (dynamic) to fixup the load address in constant islands. So we're forced to read static relocs that also have dynamic relocs in the same offset, for this specific case, for AArch64. But that seems to be too big of a hammer to fix this problem. Perhaps we should only process those static relocations if they have the dynamic R_AARCH64_RELATIVE in the same offset, but not other kinds of dynamic relocs [that might mean that the runtime will completely recompute the address at that location].

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