[lld] [lld][ELF] Merge equivalent symbols found during ICF (PR #139493)

[Pranav Kant via llvm-commits]

pranavk wrote:

> I typically prioritize correctness too, but exposing OffsetGetter and needsGot from Relocations.cpp is completely unacceptable.
The distinction between local and non-local symbols is also troubling.
The local versus non-local symbol distinction is also problematic, as it keeps potential correctness pitfalls.

Thanks for detailing your concerns. So it seems you are okay if we reland this PR with only first two commits (faa11ed33f123e30f26116ff1c968fb249d72416 and e4041b71b7c2f0abf2bc0baa926cccd0ab74bf55). Let me know if I am reading you incorrectly. I can simplify the condition in the second commit further as you pointed out in a thread above. This would be very similar to [earlier merged PR]((https://github.com/llvm/llvm-project/pull/134342/files)) with minor changes on top of it.

It appears it's the handling of non-global symbols mostly that's the cause of concern along with introduction of `TrivialRelocation()`. For Google's internal needs, for now, we are okay with just handling this for global symbols and leaving local symbols out but this doesn't really address concerns @jyknight brought [here](https://github.com/llvm/llvm-project/pull/134342#discussion_r2044177042) to fix the problem as a whole. Since it has been a point of contention, I think it's worthwhile to address those concerns separately in a different PR (although tbh I am not sure if we can do that successfully).

If you confirm this, I can change this PR accordingly to only include first two commits.







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