[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

diff --git a/llvm/docs/Extensions.rst b/llvm/docs/Extensions.rst
index 4534ba3..b4ea741 100644 (file)
--- 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 6f54f50..945b82c 100644 (file)
--- 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 0c4ed20..739d1cb 100644 (file)
--- 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 038433c..dafae3a 100644 (file)
--- 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 809ac37..e2d1a5d 100644 (file)
--- 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 7d0f354..7c3b25d 100644 (file)
--- 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 (file)
index 3383de5..0000000
--- 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 fb0895a..2aadba7 100644 (file)
--- 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 c03518f..49ec356 100644 (file)
--- 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