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

[Pranav Kant via llvm-commits]

================
@@ -352,6 +407,15 @@ bool ICF<ELFT>::variableEq(const InputSection *secA, Relocs<RelTy> ra,
     auto *da = cast<Defined>(&sa);
     auto *db = cast<Defined>(&sb);
 
+    // Prevent sections containing local symbols from merging into sections with
+    // global symbols, or vice-versa. This is to prevent local-global symbols
+    // getting merged into each other (done later in ICF). We do this as
+    // post-ICF passes cannot handle duplicates when iterating over local
+    // symbols. There are also assertions that prevent this.
+    if ((!da->isGlobal() || !db->isGlobal()) &&
----------------
pranavk wrote:

I will add the test for this.

Regarding why we need this, the issue was brought to the attention first here in this comment: https://github.com/llvm/llvm-project/pull/131630#issuecomment-2770780719

Quoting part of James' comment:

> I also added a new check that ensures local symbols aren't merged with global symbols or vice-versa, otherwise assert(b->isLocal() && "should have been caught in initializeSymbols()"); is triggered by lld/test/ELF/icf-ineligible.s. (Instead, that test now fails because the sections referring to a local symbol aren't merged with those referring to the equivalent global symbol, as the test expects, but maybe that's OK.)

So we are not merging sections (and that automatically implies not merging their symbols) when there are local symbols involved. Note that we are only avoiding merging of such sections when it's a non-trivial relocation as that's when it will cause the original 'GOT merging' problem that motivated this series of changes.

Regarding deleting `isTrivialRelocation()`, I am currently using it in two places -- here and previous block. Both usages try to merge sections when only trivial relocations are involved as our original 'GOT merging' problem only occurs with non-trivial relocation.

Basically, the invariant we are trying to maintain here is: 
*When there's a non-trivial relocation, we shouldn't merge the sections unless their symbols can be merged*

We can try to satisfy this invariant broadly by removing `!isTrivialRelocation()` check but that will fold less sections. I believe the current code satisfies the above invariant as tightly as possible to maximize section folding without causing 'GOT merging' problem.

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