[lld] [lld-macho] Mark local personality functions as `INDIRECT_SYMBOL_LOCAL` (PR #95171)
Daniel Bertalan via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 11 14:00:23 PDT 2024
https://github.com/BertalanD created https://github.com/llvm/llvm-project/pull/95171
This expands on the fix in 4e572db. The issue is pretty similar: we might put symbols in the GOT which don't need run-time binding, locally defined personality symbols in this case. We should set their indirect symbol table entries to `INDIRECT_SYMBOL_LOCAL` to help `strip` remove these local names from the symbol table.
Checking if the symbol is private-extern doesn't cover all cases; it can also be a non-weak extern function too, for instance; use the `needsBinding()` helper to determine it. This was the case for the personality function in statically linked Rust executables.
The extra non-`LOCAL` symbols triggered a bug in Apple's `strip` implementation. As the indirect value for the personality function was not set to the flag, but the symbol didn't require binding, it tried to make the symbol local, overwriting the GOT entry with the function's address in the process. This normally wouldn't be a problem, but if chained fixups are used, the fixup also encodes the offset to the next fixup, and it effectively zeroed this offset out, causing the remaining relocations on the page to not be performed by dyld.
This caused the crash in https://issues.chromium.org/issues/325410295
The change in tests is a bit ugly, as a lot of symbol information is now removed by turning more symbols `LOCAL`.
>From c4a94eb4cf6de88ac373172a67a7840a66fd0163 Mon Sep 17 00:00:00 2001
From: Daniel Bertalan <dani at danielbertalan.dev>
Date: Mon, 10 Jun 2024 22:34:54 +0200
Subject: [PATCH] [lld-macho] Mark local personality functions as
`INDIRECT_SYMBOL_LOCAL`
This expands on the fix in 4e572db. The issue is pretty similar: we
might put symbols in the GOT which don't need run-time binding, locally
defined personality symbols in this case. We should set their indirect
symbol table entries to `INDIRECT_SYMBOL_LOCAL` to help `strip` remove
these local names from the symbol table.
Checking if the symbol is private-extern doesn't cover all cases; it can
also be a non-weak extern function too, for instance; use the
`needsBinding()` helper to determine it. This was the case for the
personality function in statically linked Rust executables.
The extra non-`LOCAL` symbols triggered a bug in Apple's `strip`
implementation. As the indirect value for the personality function was
not set to the flag, but the symbol didn't require binding, it tried to
make the symbol local, overwriting the GOT entry with the function's
address in the process. This normally wouldn't be a problem, but if
chained fixups are used, the fixup also encodes the offset to the next
fixup, and it effectively zeroed this offset out, causing the remaining
relocations on the page to not be performed by dyld.
This caused the crash in https://issues.chromium.org/issues/325410295
The change in tests is a bit ugly, as a lot of symbol information is now
removed by turning more symbols `LOCAL`.
---
lld/MachO/SyntheticSections.cpp | 5 +----
lld/test/MachO/arm64-reloc-pointer-to-got.s | 4 ++--
...-unwind-both-local-and-dylib-personality.s | 22 +++++++++----------
lld/test/MachO/compact-unwind.s | 4 ++--
lld/test/MachO/dead-strip.s | 2 +-
.../invalid/compact-unwind-personalities.s | 6 ++---
6 files changed, 20 insertions(+), 23 deletions(-)
diff --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp
index b3fe223938bf5..6a7284fefcf29 100644
--- a/lld/MachO/SyntheticSections.cpp
+++ b/lld/MachO/SyntheticSections.cpp
@@ -1478,11 +1478,8 @@ void IndirectSymtabSection::finalizeContents() {
}
static uint32_t indirectValue(const Symbol *sym) {
- if (sym->symtabIndex == UINT32_MAX)
+ if (sym->symtabIndex == UINT32_MAX || !needsBinding(sym))
return INDIRECT_SYMBOL_LOCAL;
- if (auto *defined = dyn_cast<Defined>(sym))
- if (defined->privateExtern)
- return INDIRECT_SYMBOL_LOCAL;
return sym->symtabIndex;
}
diff --git a/lld/test/MachO/arm64-reloc-pointer-to-got.s b/lld/test/MachO/arm64-reloc-pointer-to-got.s
index 3d9eba0555c8a..2551d71256f4c 100644
--- a/lld/test/MachO/arm64-reloc-pointer-to-got.s
+++ b/lld/test/MachO/arm64-reloc-pointer-to-got.s
@@ -8,8 +8,8 @@
## POINTER_TO_GOT, we should still be able to relax this GOT_LOAD reference to
## it.
# CHECK: _main:
-# CHECK-NEXT: adrp x8, [[#]] ;
-# CHECK-NEXT: ldr x8, [x8] ; literal pool symbol address: _foo
+# CHECK-NEXT: adrp x8, [[#]] ; 0x100004000
+# CHECK-NEXT: ldr x8, [x8]
# CHECK-NEXT: ret
# CHECK: Idx Name Size VMA Type
diff --git a/lld/test/MachO/compact-unwind-both-local-and-dylib-personality.s b/lld/test/MachO/compact-unwind-both-local-and-dylib-personality.s
index 35f39ba5fb1e2..caf774c7c2a19 100644
--- a/lld/test/MachO/compact-unwind-both-local-and-dylib-personality.s
+++ b/lld/test/MachO/compact-unwind-both-local-and-dylib-personality.s
@@ -45,16 +45,16 @@
# A: Indirect symbols for (__DATA_CONST,__got) 4 entries
# A-NEXT: address index name
# A: 0x[[#%x,GXX_PERSONALITY_LO:]] [[#]] ___gxx_personality_v0
-# A: 0x[[#%x,PERSONALITY_1:]] [[#]] _personality_1
-# A: 0x[[#%x,PERSONALITY_2:]] [[#]] _personality_2
-# A: 0x[[#%x,GXX_PERSONALITY_HI:]] [[#]] ___gxx_personality_v0
+# A: 0x[[#%x,PERSONALITY_1:]] LOCAL
+# A: 0x[[#%x,PERSONALITY_2:]] LOCAL
+# A: 0x[[#%x,GXX_PERSONALITY_HI:]] LOCAL
# BC: Indirect symbols for (__DATA_CONST,__got)
# BC-NEXT: address index name
# BC: 0x[[#%x,GXX_PERSONALITY_LO:]] LOCAL
-# C: 0x[[#%x,GXX_PERSONALITY_HI:]] [[#]] ___gxx_personality_v0
-# BC: 0x[[#%x,PERSONALITY_1:]] [[#]] _personality_1
-# BC: 0x[[#%x,PERSONALITY_2:]] [[#]] _personality_2
+# C: 0x[[#%x,GXX_PERSONALITY_HI:]] LOCAL
+# BC: 0x[[#%x,PERSONALITY_1:]] LOCAL
+# BC: 0x[[#%x,PERSONALITY_2:]] LOCAL
# BC-EMPTY:
# CHECK: Personality functions: (count = 3)
@@ -70,11 +70,11 @@
# D: Indirect symbols for (__DATA_CONST,__got) 6 entries
# D-NEXT: address index name
# D: 0x[[#%x,GXX_PERSONALITY_HI:]] [[#]] ___gxx_personality_v0
-# D: 0x[[#%x,PERSONALITY_1:]] [[#]] _personality_1
-# D: 0x[[#%x,PERSONALITY_2:]] [[#]] _personality_2
-# D: 0x[[#%x,PERSONALITY_3:]] [[#]] _personality_3
-# D: 0x[[#%x,PERSONALITY_4:]] [[#]] _personality_4
-# D: 0x[[#%x,GXX_PERSONALITY_LO:]] [[#]] ___gxx_personality_v0
+# D: 0x[[#%x,PERSONALITY_1:]] LOCAL
+# D: 0x[[#%x,PERSONALITY_2:]] LOCAL
+# D: 0x[[#%x,PERSONALITY_3:]] LOCAL
+# D: 0x[[#%x,PERSONALITY_4:]] LOCAL
+# D: 0x[[#%x,GXX_PERSONALITY_LO:]] LOCAL
# D: Contents of __unwind_info section:
# D: Personality functions: (count = 1)
diff --git a/lld/test/MachO/compact-unwind.s b/lld/test/MachO/compact-unwind.s
index 27e4b44dc0b09..6516f7162e290 100644
--- a/lld/test/MachO/compact-unwind.s
+++ b/lld/test/MachO/compact-unwind.s
@@ -29,12 +29,12 @@
# FIRST: Indirect symbols for (__DATA_CONST,__got)
# FIRST-NEXT: address index name
# FIRST-DAG: 0x[[#%x,GXX_PERSONALITY:]] [[#]] ___gxx_personality_v0
-# FIRST-DAG: 0x[[#%x,MY_PERSONALITY:]]
+# FIRST-DAG: 0x[[#%x,MY_PERSONALITY:]] LOCAL
# SECOND: Indirect symbols for (__DATA_CONST,__got)
# SECOND-NEXT: address index name
# SECOND-DAG: 0x[[#%x,GXX_PERSONALITY:]] [[#]] ___gxx_personality_v0
-# SECOND-DAG: 0x[[#%x,MY_PERSONALITY:]] [[#]] _my_personality
+# SECOND-DAG: 0x[[#%x,MY_PERSONALITY:]] LOCAL
# CHECK: SYMBOL TABLE:
# CHECK-DAG: [[#%x,MAIN:]] g F __TEXT,__text _main
diff --git a/lld/test/MachO/dead-strip.s b/lld/test/MachO/dead-strip.s
index f60305a4ec494..d4350154695ff 100644
--- a/lld/test/MachO/dead-strip.s
+++ b/lld/test/MachO/dead-strip.s
@@ -35,7 +35,7 @@
# EXEC-DAG: g {{.*}} __mh_execute_header
# EXECDATA-LABEL: Indirect symbols
# EXECDATA-NEXT: name
-# EXECDATA-NEXT: _ref_com
+# EXECDATA-NEXT: LOCAL
# EXECDATA-LABEL: Contents of (__DATA,__ref_section) section
# EXECDATA-NEXT: 04 00 00 00 00 00 00 00 05 00 00 00 00 00 00 00
# EXECDATA-LABEL: Exports trie:
diff --git a/lld/test/MachO/invalid/compact-unwind-personalities.s b/lld/test/MachO/invalid/compact-unwind-personalities.s
index 25ee4bd17929f..9cc474de83638 100644
--- a/lld/test/MachO/invalid/compact-unwind-personalities.s
+++ b/lld/test/MachO/invalid/compact-unwind-personalities.s
@@ -14,9 +14,9 @@
# DWARF: Indirect symbols
# DWARF: address index name
# DWARF: 0x[[#%x,GXX_PERSONALITY:]] {{.*}} ___gxx_personality_v0
-# DWARF: 0x[[#%x,PERSONALITY_1:]] {{.*}} _personality_1
-# DWARF: 0x[[#%x,PERSONALITY_2:]] {{.*}} _personality_2
-# DWARF: 0x[[#%x,PERSONALITY_3:]] {{.*}} _personality_3
+# DWARF: 0x[[#%x,PERSONALITY_1:]] LOCAL
+# DWARF: 0x[[#%x,PERSONALITY_2:]] LOCAL
+# DWARF: 0x[[#%x,PERSONALITY_3:]] LOCAL
# DWARF: .eh_frame contents:
# DWARF: Personality Address: [[#%.16x,GXX_PERSONALITY]]
# DWARF: Personality Address: [[#%.16x,PERSONALITY_1]]
More information about the llvm-commits
mailing list