Skip to content

[lld][ICF] Prevent merging two sections when they point to non-globals #136641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pranavk wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[lld][ICF] Prevent merging two sections when they point to non-globals #136641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9a3d581
[lld][ICF] Prevent merging two sections when they point to non-globals
pranavk Apr 22, 2025
4e87357
Tighter condition to prevent ICF
pranavk Apr 22, 2025
7efab7d
Peter review
pranavk Apr 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions lld/ELF/ICF.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
#include "InputFiles.h"
#include "LinkerScript.h"
#include "OutputSections.h"
#include "Relocations.h"
#include "SymbolTable.h"
#include "Symbols.h"
#include "SyntheticSections.h"
Expand Down Expand Up @@ -356,6 +357,52 @@ static SmallVector<Symbol *> getRelocTargetSyms(const InputSection *sec) {
return getReloc(sec, rel.relas);
}

// A non-trivial relocation should ideally not be split by Machine outliner
// but it's not illegal to split it in which case ICF shouldn't fold outlined
// functions containing these relocations.
static bool isTrivialRelocationType(uint16_t emachine, RelType type) {
if (emachine == EM_AARCH64) {
switch (type) {
case R_AARCH64_GOT_LD_PREL19:
case R_AARCH64_LD64_GOTOFF_LO15:
case R_AARCH64_ADR_GOT_PAGE:
case R_AARCH64_LD64_GOT_LO12_NC:
case R_AARCH64_LD64_GOTPAGE_LO15:
case R_AARCH64_TLSIE_MOVW_GOTTPREL_G1:
case R_AARCH64_TLSIE_MOVW_GOTTPREL_G0_NC:
case R_AARCH64_TLSIE_ADR_GOTTPREL_PAGE21:
case R_AARCH64_TLSIE_LD64_GOTTPREL_LO12_NC:
case R_AARCH64_TLSIE_LD_GOTTPREL_PREL19:
case R_AARCH64_TLSDESC_LD_PREL19:
case R_AARCH64_TLSDESC_ADR_PREL21:
case R_AARCH64_TLSDESC_ADR_PAGE21:
case R_AARCH64_TLSDESC_LD64_LO12:
case R_AARCH64_TLSDESC_ADD_LO12:
case R_AARCH64_TLSDESC_OFF_G1:
case R_AARCH64_TLSDESC_OFF_G0_NC:
case R_AARCH64_AUTH_MOVW_GOTOFF_G0:
case R_AARCH64_AUTH_MOVW_GOTOFF_G0_NC:
case R_AARCH64_AUTH_MOVW_GOTOFF_G1:
case R_AARCH64_AUTH_MOVW_GOTOFF_G1_NC:
case R_AARCH64_AUTH_MOVW_GOTOFF_G2:
case R_AARCH64_AUTH_MOVW_GOTOFF_G2_NC:
case R_AARCH64_AUTH_MOVW_GOTOFF_G3:
case R_AARCH64_AUTH_GOT_LD_PREL19:
case R_AARCH64_AUTH_LD64_GOTOFF_LO15:
case R_AARCH64_AUTH_ADR_GOT_PAGE:
case R_AARCH64_AUTH_LD64_GOT_LO12_NC:
case R_AARCH64_AUTH_LD64_GOTPAGE_LO15:
case R_AARCH64_AUTH_GOT_ADD_LO12_NC:
case R_AARCH64_AUTH_GOT_ADR_PREL_LO21:
case R_AARCH64_AUTH_TLSDESC_ADR_PAGE21:
case R_AARCH64_AUTH_TLSDESC_LD64_LO12:
case R_AARCH64_AUTH_TLSDESC_ADD_LO12:
return false;
}
}
return true;
}

// Compare two lists of relocations. Returns true if all pairs of
// relocations point to the same section in terms of ICF.
template <class ELFT>
Expand All @@ -375,6 +422,20 @@ bool ICF<ELFT>::variableEq(const InputSection *secA, Relocs<RelTy> ra,
auto *da = cast<Defined>(&sa);
auto *db = cast<Defined>(&sb);

// Merging sections here also means that we would mark corresponding
// relocation target symbols as equivalent, done later in ICF during section
// folding. To preserve correctness for such symbol equivalence (see
// GH#129122 for details), we also have to disable section merging here:
// 1. We don't merge local symbols into global symbols, or vice-versa. There
// are post-icf passes that assert on this behavior.
// 2. We also don't merge two local symbols together. There are post-icf
// passes that expect to see no duplicates when iterating over local
// symbols.
if ((da->isGlobal() != db->isGlobal()) &&
!isTrivialRelocationType(ctx.arg.emachine,
rai->getType(ctx.arg.isMips64EL)))
return false;

// We already dealt with absolute and non-InputSection symbols in
// constantEq, and for InputSections we have already checked everything
// except the equivalence class.
Expand Down
5 changes: 2 additions & 3 deletions lld/test/ELF/icf-ineligible.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@

# CHECK: selected section {{.*}}:(.text.f1)
# CHECK: removing identical section {{.*}}:(.text.f2)
# CHECK: removing identical section {{.*}}:(.text.f3)
# CHECK: selected section {{.*}}:(.text.f4)
# CHECK: removing identical section {{.*}}:(.text.f5)
# CHECK: redirecting 'd_alias' in symtab to 'd'
# CHECK: redirecting 'd_alias' to 'd'

.globl d, d_alias, fu, f1, f2, f3, f4, f5

Expand Down
Loading