[PowerPC][ELF] Place .toc in the same COMDAT group as the target object

[MaskRay]

Summary

.text.foo (SHF_GROUP) references .toc
.toc references .rodata.foo (SHF_GROUP)
If .text.foo and .rodata.foo are discarded by the section group rule =>
.toc referencing .rodata.foo (STB_LOCAL) is disallowed by the ELF spec (http://www.sco.com/developers/gabi/latest/ch4.sheader.html):

A symbol table entry with STB_LOCAL binding that is defined relative to one of a group's sections, and that is contained in a symbol table section that is not part of the group, must be discarded if the group members are discarded. References to this symbol table entry from outside the group are not allowed.

lld .toc hacks (rL363126 and rL364391) allow this, but we should
eventually get rid of the hacks.

New PowerPC/ppc32-pic-large.ll tests check that we don't place .got2 in a COMDAT GROUP =>
lwz 3, .LC0-.LTOC(30) is not representable if .LC0 and .LTOC are in different .got2 sections

[MaskRay]

Make a LARGE-SECUREPLT: line generic

[Bdragon28]

Still working on testing this one.

I'm not sure if this is an artifact of my attempts to force clang to be an external toolchain or if this is a break yet.

--- all_subdir_lib/libdevdctl ---
ld: error: relocation refers to a discarded section: .toc
>>> defined in event.pico
>>> referenced by event.cc:268 (/usr/src/lib/libdevdctl/event.cc:268)
>>>               event.pico:(DevdCtl::Event::GetTimestamp() const)
>>> referenced by event.cc:268 (/usr/src/lib/libdevdctl/event.cc:268)
>>>               event.pico:(DevdCtl::Event::GetTimestamp() const)
ld: error: relocation refers to a discarded section: .toc
>>> defined in exception.pico
>>> referenced by exception.cc:94 (/usr/src/lib/libdevdctl/exception.cc:94)
>>>               exception.pico:(DevdCtl::ParseException::ParseException(DevdCtl::ParseException::Type, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long))
>>> referenced by exception.cc:94 (/usr/src/lib/libdevdctl/exception.cc:94)
>>>               exception.pico:(DevdCtl::ParseException::ParseException(DevdCtl::ParseException::Type, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long))
ld: error: relocation refers to a discarded section: .toc
>>> defined in exception.pico
>>> referenced by sstream:911 (/usr/obj/usr/src/powerpc.powerpc64/tmp/usr/include/c++/v1/sstream:911)
>>>               exception.pico:(DevdCtl::ParseException::ParseException(DevdCtl::ParseException::Type, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long))
>>> referenced by sstream:911 (/usr/obj/usr/src/powerpc.powerpc64/tmp/usr/include/c++/v1/sstream:911)
>>>               exception.pico:(DevdCtl::ParseException::ParseException(DevdCtl::ParseException::Type, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long))
ld: error: relocation refers to a discarded section: .toc
>>> defined in exception.pico
>>> referenced by istream:191 (/usr/obj/usr/src/powerpc.powerpc64/tmp/usr/include/c++/v1/istream:191)
>>>               exception.pico:(DevdCtl::ParseException::ParseException(DevdCtl::ParseException::Type, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long))
>>> referenced by istream:191 (/usr/obj/usr/src/powerpc.powerpc64/tmp/usr/include/c++/v1/istream:191)
>>>               exception.pico:(DevdCtl::ParseException::ParseException(DevdCtl::ParseException::Type, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long))
ld: error: relocation refers to a discarded section: .toc
>>> defined in exception.pico
>>> referenced by sstream:244 (/usr/obj/usr/src/powerpc.powerpc64/tmp/usr/include/c++/v1/sstream:244)
>>>               exception.pico:(DevdCtl::ParseException::ParseException(DevdCtl::ParseException::Type, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long))
>>> referenced by sstream:244 (/usr/obj/usr/src/powerpc.powerpc64/tmp/usr/include/c++/v1/sstream:244)
>>>               exception.pico:(DevdCtl::ParseException::ParseException(DevdCtl::ParseException::Type, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long))
ld: error: relocation refers to a discarded section: .toc
>>> defined in exception.pico
>>> referenced by iosfwd:139 (/usr/obj/usr/src/powerpc.powerpc64/tmp/usr/include/c++/v1/iosfwd:139)
>>>               exception.pico:(DevdCtl::ParseException::ParseException(DevdCtl::ParseException::Type, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long))
>>> referenced by iosfwd:139 (/usr/obj/usr/src/powerpc.powerpc64/tmp/usr/include/c++/v1/iosfwd:139)
>>>               exception.pico:(DevdCtl::ParseException::ParseException(DevdCtl::ParseException::Type, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long))
>>> referenced by exception.cc:0 (/usr/src/lib/libdevdctl/exception.cc:0)
>>>               exception.pico:(DevdCtl::ParseException::ParseException(DevdCtl::ParseException::Type, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long))
>>> referenced by exception.cc:0 (/usr/src/lib/libdevdctl/exception.cc:0)
>>>               exception.pico:(DevdCtl::ParseException::ParseException(DevdCtl::ParseException::Type, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long))
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

[MaskRay]

In D75416#1900178, @Bdragon28 wrote:

Still working on testing this one.

I'm not sure if this is an artifact of my attempts to force clang to be an external toolchain or if this is a break yet.

--- all_subdir_lib/libdevdctl ---
ld: error: relocation refers to a discarded section: .toc

ld: error: relocation refers to a discarded section: .toc was worked around in lld by rL363126.

[Bdragon28]

Right, which is why it was so surprising to see it again in HEAD. I'm still trying to get a reproduce.

[Bdragon28]

I'm pretty sure I'm just tripping over c++ weirdness related to trying to use an external llvm HEAD compiler as a toolchain for the same platform. I will retry from my cross build machine.

[Bdragon28]

OK, I reinterpreted 9569a1472ee7fee37f7f991d34634c5d8d1f3559 (as a prereq to reduce the churn) and this patch in the context of llvm10 (mainly re-uppercasing function names so that stuff applies and converting the MCSection::NonUniqueID bit back into ~0) so I can run it in-tree for a FreeBSD buildworld, and I hit the exact same failure, so it looks like there's an actual problem here.

[Bdragon28]

Here's the LLD_REPRODUCE using the cross toolchain (since I had it already and I hit the same thing on the backport, this one is probably the best one to look at)

libdevdctl.tar6 MBDownload

[MaskRay]

This is a fundamental problem of .toc . It cannot be fixed, oh, well, without some ELF extensions.

For @Bdragon28's reproduce file,

A .text (not in a section group) references foo defined in .data.rel.ro.foo (in a section group) via .toc .

.text
addis 3,2,.LC0@toc@ha

.section .data.rel.ro.foo,"aGw",foo,comdat
.globl foo
foo:

.section .toc,"aGw",@progbits,foo,comdat  # caused by this patch
.LC0:
.tc foo[TC],foo

This patch emits .section .toc,"aGw",foo,comdat.
When .data.rel.ro.foo and the .toc are discarded due to the section group rule, the relocation from .text to the .toc becomes dangling.

A symbol table entry with STB_LOCAL binding that is defined relative to one of a group's sections, and that is contained in a symbol table section that is not part of the group, must be discarded if the group members are discarded. References to this symbol table entry from outside the group are not allowed.

One thought is to change the .toc reference to a STB_GLOBAL:

.text
addis 3,2,foo.toc@toc@ha

.section .toc,"aGw",@progbits,foo,comdat
.globl foo.toc
foo.toc:
.tc foo[TC],foo

Say, both a.o and b.o have the code sequence above. If a.o(.toc) is the prevailing definition, the symbolic reference add 3,2,foo.toc@toc@ha from b.o will bind to a.o(.toc) (like an external reference).

Everything works fine except one thing: foo.toc is left in .symtab (unless --strip-all) and probably .dynsym. If we set the visibility of foo.toc to STB_HIDDEN or STB_INTERNAL, foo.toc will be omitted from .dynsym, but we cannot omit it from .symtab.
Unless we extend ELF and say STB_INTERNAL symbols are omitted from .symtab.

I have a feeling that the .tc directive was misdesigned for ELF. If we let foo@toc@ha create a TOC entry implicitly (like a GOT-generating relocation on other architectures), we would not need multiple .toc sections in the first place. Additionally, toc-indirect to toc-relative relaxation would become easier to implement. Note, explicit .section .toc,"aw",@progbits can still be allowed if we want to emit some non-symbol entries. They should still be explicit.

It is sad that .toc is prettier than ppc32 .got2, but that is the pot calling the kettle black.

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

void PPCLinuxAsmPrinter::emitEndOfAsmFile(Module &M) {
 
   bool isPPC64 = DL.getPointerSizeInBits() == 64;
 
   PPCTargetStreamer &TS =
       static_cast<PPCTargetStreamer &>(*OutStreamer->getTargetStreamer());
 
   if (!TOC.empty()) {
     const char *Name = isPPC64 ? ".toc" : ".got2";
-    MCSectionELF *Section = OutContext.getELFSection(
-        Name, ELF::SHT_PROGBITS, ELF::SHF_WRITE | ELF::SHF_ALLOC);
-    OutStreamer->SwitchSection(Section);
-    if (!isPPC64)
-      OutStreamer->emitValueToAlignment(4);
-
+    MCSectionELF *CurSection = nullptr, *Section;
     for (const auto &TOCMapPair : TOC) {
       const MCSymbol *const TOCEntryTarget = TOCMapPair.first;
       MCSymbol *const TOCEntryLabel = TOCMapPair.second;
+      const MCSymbolELF *Group = nullptr;
+      if (isPPC64 && TOCEntryTarget->isInSection())
+        Group = cast<MCSectionELF>(TOCEntryTarget->getSection()).getGroup();
+      Section = OutStreamer->getContext().getELFSection(
+          Name, ELF::SHT_PROGBITS,
+          ELF::SHF_WRITE | ELF::SHF_ALLOC | (Group ? ELF::SHF_GROUP : 0), 0,
+          Group, MCSection::NonUniqueID, nullptr);
+      if (Section != CurSection) {
+        OutStreamer->SwitchSection(Section);
+        CurSection = Section;
+        if (!isPPC64)
+          OutStreamer->emitValueToAlignment(4);
+      }
 
       OutStreamer->emitLabel(TOCEntryLabel);
       if (isPPC64)
         TS.emitTCEntry(*TOCEntryTarget);
       else
         OutStreamer->emitSymbolValue(TOCEntryTarget, 4);
     }
   }

llvm/test/CodeGen/PowerPC/mcm-1.ll

 ; RUN: llc -verify-machineinstrs -mcpu=pwr7 -O0 -code-model=medium <%s | FileCheck %s
 ; RUN: llc -verify-machineinstrs -mcpu=pwr7 -O0 -code-model=large <%s | FileCheck %s
 
 ; Test correct code generation for medium and large code model
 ; for loading and storing an external variable.
 
 target datalayout = "E-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-f128:128:128-v128:128:128-n32:64"
 target triple = "powerpc64-unknown-linux-gnu"
 
+$fi = comdat any
+$gi = comdat any
+
 @ei = external global i32
+@fi = global i32 0, align 4, comdat
+@gi = global i32 0, align 4, comdat
 
 define signext i32 @test_external() nounwind {
 entry:
   %0 = load i32, i32* @ei, align 4
   %inc = add nsw i32 %0, 1
   store i32 %inc, i32* @ei, align 4
   ret i32 %0
 }
 
+define i32 @load() {
+entry:
+  %0 = load i32, i32* @gi, align 4
+  %1 = load i32, i32* @fi, align 4
+  %2 = add i32 %0, %1
+  ret i32 %2
+}
+
 ; CHECK-LABEL: test_external:
 ; CHECK: addis [[REG1:[0-9]+]], 2, .LC[[TOCNUM:[0-9]+]]@toc@ha
 ; CHECK: ld [[REG2:[0-9]+]], .LC[[TOCNUM]]@toc@l([[REG1]])
 ; CHECK: lwz {{[0-9]+}}, 0([[REG2]])
 ; CHECK: stw {{[0-9]+}}, 0([[REG2]])
-; CHECK: .section .toc
-; CHECK: .LC[[TOCNUM]]:
-; CHECK: .tc {{[a-z0-9A-Z_.]+}}[TC],{{[a-z0-9A-Z_.]+}}
+
+
+; CHECK:      .section .toc,"aw",@progbits
+; CHECK-NEXT: .LC0:
+; CHECK-NEXT:  .tc ei[TC],ei
+; CHECK-NEXT: .section .toc,"aGw",@progbits,fi,comdat
+; CHECK-NEXT: .LC1:
+; CHECK-NEXT:  .tc fi[TC],fi
+; CHECK-NEXT: .section .toc,"aGw",@progbits,gi,comdat
+; CHECK-NEXT: .LC2:
+; CHECK-NEXT:  .tc gi[TC],gi

llvm/test/CodeGen/PowerPC/ppc32-pic-large.ll

-; RUN: llc < %s -mtriple=powerpc-unknown-linux-gnu -relocation-model=pic | FileCheck -check-prefix=LARGE-BSS %s
-; RUN: llc < %s -mtriple=powerpc-unknown-linux-gnu -mattr=+secure-plt -relocation-model=pic | FileCheck -check-prefix=LARGE-SECUREPLT %s
+; RUN: llc < %s -mtriple=powerpc-unknown-linux-gnu -relocation-model=pic | FileCheck --check-prefixes=LARGE,LARGE-BSS %s
+; RUN: llc < %s -mtriple=powerpc-unknown-linux-gnu -mattr=+secure-plt -relocation-model=pic | FileCheck --check-prefixes=LARGE,LARGE-SECUREPLT %s
 ; RUN: llc < %s -mtriple=powerpc-unknown-netbsd -mattr=+secure-plt -relocation-model=pic | FileCheck -check-prefix=LARGE-SECUREPLT %s
 ; RUN: llc < %s -mtriple=powerpc-unknown-netbsd -relocation-model=pic | FileCheck -check-prefix=LARGE-SECUREPLT %s
 ; RUN: llc < %s -mtriple=powerpc-unknown-openbsd -mattr=+secure-plt -relocation-model=pic | FileCheck -check-prefix=LARGE-SECUREPLT %s
 ; RUN: llc < %s -mtriple=powerpc-unknown-openbsd -relocation-model=pic | FileCheck -check-prefix=LARGE-SECUREPLT %s
 ; RUN: llc < %s -mtriple=powerpc-linux-musl -mattr=+secure-plt -relocation-model=pic | FileCheck -check-prefix=LARGE-SECUREPLT %s
 ; RUN: llc < %s -mtriple=powerpc-linux-musl -relocation-model=pic | FileCheck -check-prefix=LARGE-SECUREPLT %s
+$bar1 = comdat any
+$bar2 = comdat any
+
 @bar = common global i32 0, align 4
+@bar1 = global i32 0, align 4, comdat
+@bar2 = global i32 0, align 4, comdat
 
 declare i32 @call_foo(i32, ...)
 
 define i32 @foo() {
 entry:
   %0 = load i32, i32* @bar, align 4
   %call = call i32 (i32, ...) @call_foo(i32 %0, i32 0, i32 1, i32 2, i32 4, i32 8, i32 16, i32 32, i32 64)
   ret i32 %0
 }
 
+define i32 @load() {
+entry:
+  %0 = load i32, i32* @bar1
+  %1 = load i32, i32* @bar2
+  %2 = add i32 %0, %1
+  ret i32 %2
+}
+
 !llvm.module.flags = !{!0}
 !0 = !{i32 1, !"PIC Level", i32 2}
 ; LARGE-BSS:       [[POFF:\.L[0-9]+\$poff]]:
 ; LARGE-BSS-NEXT:    .long .LTOC-[[PB:\.L[0-9]+\$pb]]
 ; LARGE-BSS-NEXT:  foo:
 ; LARGE-BSS:         stwu 1, -32(1)
 ; LARGE-BSS:         stw 30, 24(1)
 ; LARGE-BSS:         bl [[PB]]
 ; LARGE-BSS-NEXT:  [[PB]]:
 ; LARGE-BSS:         mflr 30
 ; LARGE-BSS:         lwz [[REG:[0-9]+]], [[POFF]]-[[PB]](30)
 ; LARGE-BSS-NEXT:    add 30, [[REG]], 30
 ; LARGE-BSS-DAG:     lwz [[VREG:[0-9]+]], [[VREF:\.LC[0-9]+]]-.LTOC(30)
 ; LARGE-BSS-DAG:     lwz {{[0-9]+}}, 0([[VREG]])
 ; LARGE-BSS-DAG:     stw {{[0-9]+}}, 8(1)
 ; LARGE-BSS:         lwz 30, 24(1)
-; LARGE-BSS:       .section .got2,"aw",@progbits
-; LARGE-BSS-NEXT:    .p2align 2
-; LARGE-BSS-NEXT:  [[VREF]]:
-; LARGE-BSS-NEXT:    .long bar
 ; LARGE-SECUREPLT:   addis 30, 30, .LTOC-.L0$pb@ha
 ; LARGE-SECUREPLT:   addi 30, 30, .LTOC-.L0$pb@l
 ; LARGE-SECUREPLT:   bl call_foo@PLT+32768
+
+; LARGE:      .section .bss.bar1,"aGw",@nobits,bar1,comdat
+; LARGE:      .section .bss.bar2,"aGw",@nobits,bar2,comdat
+; LARGE:      .section .got2,"aw",@progbits
+; LARGE-NEXT: .p2align 2
+; LARGE-NEXT: .LC0:
+; LARGE-NEXT:  .long bar
+; LARGE-NEXT: .LC1:
+; LARGE-NEXT:  .long bar1
+; LARGE-NEXT: .LC2:
+; LARGE-NEXT:  .long bar2