[llvm] d8162a7 - [MC] .addrsig_sym: ignore unregistered symbols
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 11 15:07:23 PDT 2022
Author: Fangrui Song
Date: 2022-10-11T15:07:14-07:00
New Revision: d8162a7196b3916d3bb51b2597c9e417e4132150
URL: https://github.com/llvm/llvm-project/commit/d8162a7196b3916d3bb51b2597c9e417e4132150
DIFF: https://github.com/llvm/llvm-project/commit/d8162a7196b3916d3bb51b2597c9e417e4132150.diff
LOG: [MC] .addrsig_sym: ignore unregistered symbols
.addrsig_sym forces registering the symbol regardless whether it is otherwise
registered. This creates an undefined symbol which is inconvenient/undesired:
* `extern int x; void f() { (void)x; }` has inconsistent behavior whether `x` is emitted as an undefined symbol.
`-O0 -faddrsig` makes `x` undefined while other -O levels and -fno-addrsig eliminate the symbol.
* In ThinLTO, after a non-prevailing linkonce_odr definition is converted to available_externally, and then a declaration,
the addrsig code emits a symbol while the symbol is otherwise unseen.
D135427 fixed a bug that a non-prevailing `__cxx_global_var_init` was
incorrectly retained. However, the IR declaration causes an undesired
`.addrsig_sym __cxx_global_var_init`. This can be addressed in a way similar
to D101512 (`isTransitiveUsedByMetadataOnly`) but the increased
`OutStreamer->emitAddrsigSym(getSymbol(&GV));` complexity makes me nervous.
Just ignoring unregistered symbols circumvents the problem.
Reviewed By: rnk
Differential Revision: https://reviews.llvm.org/D135642
Added:
Modified:
llvm/docs/Extensions.rst
llvm/lib/MC/ELFObjectWriter.cpp
llvm/lib/MC/MCObjectStreamer.cpp
llvm/lib/MC/MachObjectWriter.cpp
llvm/lib/MC/WinCOFFObjectWriter.cpp
llvm/test/MC/COFF/addrsig.s
llvm/test/MC/ELF/addrsig.s
llvm/test/MC/MachO/addrsig.s
Removed:
llvm/test/MC/ELF/addrsig-error.s
################################################################################
diff --git a/llvm/docs/Extensions.rst b/llvm/docs/Extensions.rst
index 4534ba38093ed..b4ea741fdfe7b 100644
--- a/llvm/docs/Extensions.rst
+++ b/llvm/docs/Extensions.rst
@@ -377,7 +377,8 @@ this directive, all symbols are considered address-significant.
.addrsig_sym sym
-This marks ``sym`` as address-significant.
+If ``sym`` is not otherwise referenced or defined anywhere else in the file,
+this directive is a no-op. Otherwise, mark ``sym`` as address-significant.
``SHT_LLVM_SYMPART`` Section (symbol partition specification)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/llvm/lib/MC/ELFObjectWriter.cpp b/llvm/lib/MC/ELFObjectWriter.cpp
index 6f54f5033f6c4..945b82c8eaabb 100644
--- a/llvm/lib/MC/ELFObjectWriter.cpp
+++ b/llvm/lib/MC/ELFObjectWriter.cpp
@@ -786,7 +786,8 @@ void ELFWriter::computeSymbolTable(
void ELFWriter::writeAddrsigSection() {
for (const MCSymbol *Sym : OWriter.AddrsigSyms)
- encodeULEB128(Sym->getIndex(), W.OS);
+ if (Sym->getIndex() != 0)
+ encodeULEB128(Sym->getIndex(), W.OS);
}
MCSectionELF *ELFWriter::createRelocationSection(MCContext &Ctx,
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index 0c4ed201a0c51..739d1cb9b1bf1 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -912,7 +912,6 @@ void MCObjectStreamer::emitAddrsig() {
}
void MCObjectStreamer::emitAddrsigSym(const MCSymbol *Sym) {
- getAssembler().registerSymbol(*Sym);
getAssembler().getWriter().addAddrsigSymbol(Sym);
}
diff --git a/llvm/lib/MC/MachObjectWriter.cpp b/llvm/lib/MC/MachObjectWriter.cpp
index 038433cb24fac..dafae3a130d12 100644
--- a/llvm/lib/MC/MachObjectWriter.cpp
+++ b/llvm/lib/MC/MachObjectWriter.cpp
@@ -758,6 +758,8 @@ void MachObjectWriter::populateAddrSigSection(MCAssembler &Asm) {
Asm.getContext().getObjectFileInfo()->getAddrSigSection();
unsigned Log2Size = is64Bit() ? 3 : 2;
for (const MCSymbol *S : getAddrsigSyms()) {
+ if (!S->isRegistered())
+ continue;
MachO::any_relocation_info MRE;
MRE.r_word0 = 0;
MRE.r_word1 = (Log2Size << 25) | (MachO::GENERIC_RELOC_VANILLA << 28);
diff --git a/llvm/lib/MC/WinCOFFObjectWriter.cpp b/llvm/lib/MC/WinCOFFObjectWriter.cpp
index 809ac37c3442f..e2d1a5d7b8ee5 100644
--- a/llvm/lib/MC/WinCOFFObjectWriter.cpp
+++ b/llvm/lib/MC/WinCOFFObjectWriter.cpp
@@ -1098,6 +1098,8 @@ uint64_t WinCOFFObjectWriter::writeObject(MCAssembler &Asm,
Frag->setLayoutOrder(0);
raw_svector_ostream OS(Frag->getContents());
for (const MCSymbol *S : AddrsigSyms) {
+ if (!S->isRegistered())
+ continue;
if (!S->isTemporary()) {
encodeULEB128(S->getIndex(), OS);
continue;
diff --git a/llvm/test/MC/COFF/addrsig.s b/llvm/test/MC/COFF/addrsig.s
index 7d0f354e7cdfa..7c3b25d52edae 100644
--- a/llvm/test/MC/COFF/addrsig.s
+++ b/llvm/test/MC/COFF/addrsig.s
@@ -14,38 +14,53 @@
// CHECK-NEXT: IMAGE_SCN_LNK_REMOVE (0x800)
// CHECK-NEXT: ]
// CHECK-NEXT: SectionData (
-// CHECK-NEXT: 0000: 080A0B02
+// CHECK-NEXT: 0000: 080B0A02
// CHECK-NEXT: )
// CHECK: Symbols [
-// CHECK: Name: .text
+// CHECK: Name:
+// CHECK-SAME: {{^}} .text
// CHECK: AuxSectionDef
-// CHECK: Name: .data
+// CHECK: Name:
+// CHECK-SAME: {{^}} .data
// CHECK: AuxSectionDef
-// CHECK: Name: .bss
+// CHECK: Name:
+// CHECK-SAME: {{^}} .bss
// CHECK: AuxSectionDef
-// CHECK: Name: .llvm_addrsig
+// CHECK: Name:
+// CHECK-SAME: {{^}} .llvm_addrsig
// CHECK: AuxSectionDef
-// CHECK: Name: g1
-// CHECK: Name: g2
-// CHECK: Name: g3
-// CHECK: Name: local
+// CHECK: Name:
+// CHECK-SAME: {{^}} g1
+// CHECK: Name:
+// CHECK-SAME: {{^}} g2
+// CHECK: Name:
+// CHECK-SAME: {{^}} local
+// CHECK: Name:
+// CHECK-SAME: {{^}} g3
+// CHECK-NOT: Name:
+// CHECK: }
// CHECK: Addrsig [
// CHECK-NEXT: Sym: g1 (8)
-// CHECK-NEXT: Sym: g3 (10)
-// CHECK-NEXT: Sym: local (11)
+// CHECK-NEXT: Sym: g3 (11)
+// CHECK-NEXT: Sym: local (10)
// CHECK-NEXT: Sym: .data (2)
// CHECK-NEXT: ]
+.globl g1
+
.addrsig
.addrsig_sym g1
.globl g2
.addrsig_sym g3
.addrsig_sym local
.addrsig_sym .Llocal
+.addrsig_sym .Lunseen
+.addrsig_sym unseen
local:
+.globl g3
.data
.Llocal:
diff --git a/llvm/test/MC/ELF/addrsig-error.s b/llvm/test/MC/ELF/addrsig-error.s
deleted file mode 100644
index 3383de54641c1..0000000000000
--- a/llvm/test/MC/ELF/addrsig-error.s
+++ /dev/null
@@ -1,5 +0,0 @@
-// RUN: not llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - 2>&1 | FileCheck %s
-// CHECK: Undefined temporary symbol
-
-.addrsig
-.addrsig_sym .Lundef
diff --git a/llvm/test/MC/ELF/addrsig.s b/llvm/test/MC/ELF/addrsig.s
index fb0895a1a3ae8..2aadba7f9486f 100644
--- a/llvm/test/MC/ELF/addrsig.s
+++ b/llvm/test/MC/ELF/addrsig.s
@@ -62,6 +62,7 @@
// CHECK-NEXT: }
// CHECK-NEXT: Symbol {
// CHECK-NEXT: Name: g3
+// CHECK-NOT: Symbol {
// CHECK: Addrsig [
// CHECK-NEXT: Sym: g1 (3)
@@ -70,6 +71,8 @@
// CHECK-NEXT: Sym: (1)
// CHECK-NEXT: ]
+.globl g1
+
// ASM: .addrsig
// ASM-NEXT: .addrsig_sym g1
.addrsig
@@ -78,11 +81,17 @@
// ASM: .addrsig_sym g3
// ASM-NEXT: .addrsig_sym local
// ASM-NEXT: .addrsig_sym .Llocal
+// ASM-NEXT: .addrsig_sym .Lunseen
+// ASM-NEXT: .addrsig_sym unseen
.addrsig_sym g3
.addrsig_sym local
.addrsig_sym .Llocal
+.addrsig_sym .Lunseen
+.addrsig_sym unseen
local:
.Llocal:
+.globl g3
+
// DWO-NOT: .llvm_addrsig
diff --git a/llvm/test/MC/MachO/addrsig.s b/llvm/test/MC/MachO/addrsig.s
index c03518f2fd113..49ec3564b459f 100644
--- a/llvm/test/MC/MachO/addrsig.s
+++ b/llvm/test/MC/MachO/addrsig.s
@@ -15,10 +15,10 @@
# CHECK: Symbol {
# CHECK-NEXT: Name: local
# CHECK: Symbol {
-# CHECK-NEXT: Name: .Llocal
-# CHECK: Symbol {
# CHECK-NEXT: Name: ltmp1
# CHECK: Symbol {
+# CHECK-NEXT: Name: .Llocal
+# CHECK: Symbol {
# CHECK-NEXT: Name: g1
# CHECK: Symbol {
# CHECK-NEXT: Name: g2
@@ -35,6 +35,8 @@
.addrsig_sym g3
.addrsig_sym local
.addrsig_sym .Llocal
+.addrsig_sym .Lunseen
+.addrsig_sym unseen
local:
nop
More information about the llvm-commits
mailing list