[MC][ELF] Make linked-to symbol name part of ELFSectionKey

[MaskRay]

Summary

https://bugs.llvm.org/show_bug.cgi?id=44775

This rule has been implemented by GNU as https://sourceware.org/ml/binutils/2020-02/msg00028.html (binutils >= 2.35)

It allows us to simplify

.section .foo,"o",foo,unique,0
.section .foo,"o",bar,unique,1  # different section

as:

.section .foo,"o",foo
.section .foo,"o",bar  # different section

We consider the two .foo different even if the linked-to symbols foo and bar
are defined in the same section. This is a deliberate choice so that we don't
need to know the section where foo and bar are defined beforehand.

[merge_guards_bot]

Unit tests: unknown.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[MaskRay]

llvm/lib/MC/MCContext.cpp

322–323

As a note: this is only used by renaming .debug_* to .zdebug_*, which don't do SHF_LINK_ORDER magic. So using an empty linked-to symbol name will be fine.

As I understand it, .zdebug* are deprecated. ELF toolchain moves to SHF_COMPRESSED, so we don't need to add more support to .zdebug*.

[grimar]

It looks reasonable to me. Two minor nits are below, the rest looks probably fine.
Lets see what other people think.

llvm/include/llvm/MC/MCContext.h

217

Perhaps just:

if (UniqueID  != Other.UniqueID)
  return UniqueID < Other.UniqueID;
return LinkedToName < Other.LinkedToName;

llvm/lib/MC/MCContext.cpp

322–323

It deserves a comment in the code I think.

[MaskRay]

Add a comment

llvm/include/llvm/MC/MCContext.h

217

The order is so that it matches the field order. Name fields placing together looks better, so I want to stick with the current order.

I can do a follow-up change to use StringRef::compare for SectionName and GroupName.

[grimar]

llvm/include/llvm/MC/MCContext.h

217

Up to you. Not a big problem anyways.

[merge_guards_bot]

Unit tests: unknown.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[bd1976llvm]

I think that this patch removes the need for uniquing sections for symbols with associated symbols which are explicitly assigned to a section name e.g. via attribute((section name)).

I have updated the comment in https://reviews.llvm.org/D72194 to say that. It would be a nice follow up change to remove the UniqueID added to sections for symbols with COMDATs/associated symbols that are explicitly assigned a section name.

llvm/include/llvm/MC/MCContext.h

199

Here start using "linked_to" terminology instead of "associated". Should we stick to just one?

[peter.smith]

This is a deliberate choice so that we don't need to know the section where foo and bar are defined beforehand.

Just want to make sure this is well defined with respect to the description in ELF.

SHF_LINK_ORDER
     This flag adds special ordering requirements for link editors. The requirements apply if the sh_link field of this section's header references another section (the linked-to section). If this section is combined with other sections in the output file, it must appear in the same relative order with respect to those sections, as the linked-to section appears with respect to sections the linked-to section is combined with.

A typical use of this flag is to build a table that references text or data sections in address order.

If we have two sections S1, S2, with a SHF_LINK_ORDER on the same section D, what is their ordering relationship between S1 and S2? If foo and bar are in the same section and we don't create two sections then we don't get this problem as we'd only have one S with a sh_link to D. I think that if the author is trying to use SHF_LINK_ORDER to build a table that references text or data sections in address order then they may get surprised if S1 and S2 are not in the order of foo and bar. True that if one section where generated they'd have to ensure that the contents of S where in the same relative order to foo and bar in D but at least they would know the linker couldn't alter it.

[MaskRay]

llvm/include/llvm/MC/MCContext.h

199

"linked-to section" is used by the ELF spec. By analogy, I use "linked-to symbol" here (omitted "Sym" in "LinkedToName"). "linked-to" implies a directed edge and makes it clear its relation with "sh_link", while one can argue that "associated" means an undirected edge. I think we should rename MCSymbol related "Associated" to "LinkedToSym". The IR level "!associated" can be left unchanged. I can do that in a separate change.

[MaskRay]

In D74006#1859875, @peter.smith wrote:

This is a deliberate choice so that we don't need to know the section where foo and bar are defined beforehand.

Just want to make sure this is well defined with respect to the description in ELF.

SHF_LINK_ORDER
     This flag adds special ordering requirements for link editors. The requirements apply if the sh_link field of this section's header references another section (the linked-to section). If this section is combined with other sections in the output file, it must appear in the same relative order with respect to those sections, as the linked-to section appears with respect to sections the linked-to section is combined with.

A typical use of this flag is to build a table that references text or data sections in address order.

If we have two sections S1, S2, with a SHF_LINK_ORDER on the same section D, what is their ordering relationship between S1 and S2? If foo and bar are in the same section and we don't create two sections then we don't get this problem as we'd only have one S with a sh_link to D. I think that if the author is trying to use SHF_LINK_ORDER to build a table that references text or data sections in address order then they may get surprised if S1 and S2 are not in the order of foo and bar. True that if one section where generated they'd have to ensure that the contents of S where in the same relative order to foo and bar in D but at least they would know the linker couldn't alter it.

"Two sections with the same sh_link" this scenario is probably not clearly specified. I will expect a stable sort by sh_link. For the following part of the added test, after linking,

.section .foo,"o",@progbits,foo  # first in the output section .foo due to stable sort
.byte 0

.section .foo,"o",@progbits,bar  # second
.byte 1

This is what lld currently does. It will be nice to get a sign-up from GNU ld. An alternative choice is that users should not rely on the order. I'm inclined to define an order here.

[MaskRay]

Another thing is whether we should use sh_flags/sh_type to differentiate sections. In addition, whether we should use sh_entsize.
If yes, the implementation can be a bit clumsy. We may have to add many Elf*_Shdr fields to ELFSectionKey.
If no (GNU as behavior), we should add warnings. D73999 will do that.

The section group name is part of the key in both MC and GNU as. By analogy, SHF_LINK_ORDER+sh_link should probably be part of the key.

Field not specified by the .section directive (sh_addrline,sh_addr,sh_offset,sh_info) should definitely not be part of the key.

I have a slight preference for "no", though I don't currently have a particular good reason why "sh_entsize" should not be part of a key. Maybe we are fine with current .rodata.cst16 naming.

[hjl.tools]

In D74006#1860062, @MaskRay wrote:

.section .foo,"o",@progbits,foo  # first in the output section .foo due to stable sort
.byte 0

.section .foo,"o",@progbits,bar  # second
.byte 1

This is what lld currently does. It will be nice to get a sign-up from GNU ld. An alternative choice is that users should not rely on the order. I'm inclined to define an order here.

I don't believe that assembler nor linker should guarantee any orders here.

[bd1976llvm]

In D74006#1860074, @MaskRay wrote:

Another thing is whether we should use sh_flags/sh_type to differentiate sections. In addition, whether we should use sh_entsize.
If yes, the implementation can be a bit clumsy. We may have to add many Elf*_Shdr fields to ELFSectionKey.
If no (GNU as behavior), we should add warnings. D73999 will do that.

The section group name is part of the key in both MC and GNU as. By analogy, SHF_LINK_ORDER+sh_link should probably be part of the key.

Field not specified by the .section directive (sh_addrline,sh_addr,sh_offset,sh_info) should definitely not be part of the key.

I have a slight preference for "no", though I don't currently have a particular good reason why "sh_entsize" should not be part of a key. Maybe we are fine with current .rodata.cst16 naming.

An alternative design to this patch is for the assembler to *never* create more than one section with a given name (apart from for COMDATs because that is established behaviour we can't change) unless the ,unique, N, extension is used. However, the assembler can give warnings as in D73999. This is really simple for users to understand - the rule is you have to choose a unique name for your sections.

llvm/include/llvm/MC/MCContext.h

199

SGTM.

[MaskRay]

llvm/include/llvm/MC/MCContext.h

199

Created D74082 to rename Associated to LinkedToSym

[MaskRay]

Rebase after D74082 was committed

My current thoughts regarding (.section directives with the same name but different fields)
are similar to binutils' (eventual) state:

• sh_flags or sh_type: warn
• sh_link/group name: no warning. produce separate sections
• sh_entsize due to SHF_MERGE: still controversial. I am slightly inclined to require ,unique

[MaskRay]

In D73999#1866680, @psmith wrote:

In D73999#1866168, @MaskRay wrote:

Address review comments.

I tend to agree with Alan Modra's arguement in
https://sourceware.org/ml/binutils/2020-02/msg00093.html

I don't think so. User assembly often gets section attributes wrong
or leaves them off entirely for special sections known to the
assembler. ie. the first .section .foo above is a built-in rather
than user input.

Add more folks for feedback.

Please see my binutils post. For a .section directive with the same name but a different field. If the field is:

• sh_flags or sh_type: warn
• sh_link due to SHF_LINK_ORDER: no warning. produce separate sections
• sh_entsize due to SHF_MERGE: still controversial

I agree with warning for sh_flags, I think there is too much legacy code out there that would behave differently.
I agree with sh_link producing separate sections. In an ideal world no-one writes this by hand in assembly.
For sh_entsize SHF_MERGE, this is informing the linker that it can produce an optimisation, which it is permitted to ignore. I tend towards an error message if it is implementable as it is a strong message to fix the assembler. The next best thing is a warning then clearing SHF_MERGE and setting sh_entsize to 0. I don't think a warning on its own is safe.

Does this patch look good? The consensus is that we should make sh_link producing separate sections.

[MaskRay]

Update 4 __patchable_function_entries tests.

The case no-function-sections is very similar to function-sections case now.
A future clean-up can delete ,unique from the tests but that will require some interface changes.

[psmith]

I'm happy with this patch, I think it is a fair reflection of the consensus, I'll let others approve when they have had a chance to read through in the US timezone.

[efriedma]

It seems like there's some bad interaction between this, ARM exception handling, and asm parsing. I'm seeing link errors like error: EXIDX sections .ARM.exidx(10) and .ARM.exidx(4) both link to text section.text(2) if I build with "-via-file-asm", and link with binutils ld. Reverting this patch seems to fix the issue.

You can produce an object file with multiple .ARM.exidx sections using something like echo "void h();void f(){}void g(){}" | clang -x c - -o armeh.o -c -via-file-asm --target=arm-eabi

I suspect that ARMELFStreamer::SwitchToEHSection is somehow triggering this code, but I'm not sure I follow how this is supposed to work. And I'm not sure why -via-file-asm is relevant.

[MaskRay]

In D74006#1890487, @efriedma wrote:

It seems like there's some bad interaction between this, ARM exception handling, and asm parsing. I'm seeing link errors like error: EXIDX sections .ARM.exidx(10) and .ARM.exidx(4) both link to text section.text(2) if I build with "-via-file-asm", and link with binutils ld. Reverting this patch seems to fix the issue.

You can produce an object file with multiple .ARM.exidx sections using something like echo "void h();void f(){}void g(){}" | clang -x c - -o armeh.o -c -via-file-asm --target=arm-eabi

I suspect that ARMELFStreamer::SwitchToEHSection is somehow triggering this code, but I'm not sure I follow how this is supposed to work. And I'm not sure why -via-file-asm is relevant.

It seems that .ARM.exidx should be special cased.

edit: clang test/MC/ARM/eh-directive-section-multiple-func.s -o a.o -c --target=arm-eabi; readelf -S a.o a.o has multiple .ARM.exidx sections.

llvm-mc runs Ctx.setUseNamesOnTempLabels(false);, so FnStart will have an empty name. Multiple .ARM.exidx are considered the same.

In clang cc1as, UseNamesOnTempLabels is true. FnStart will get different names for different functions. Multiple .ARM.exidx are considered different.

Will send a fix soon.

[MaskRay]

@efriedma Either D75095 or D75097 can fix the .ARM.exidx issue. I think both make sense. Unfortunately neither can be easily tested :/

[maxim-kuvyrkov]

@MaskRay , hi. This patch miscompiles 471.omnetpp and 453.povray on arm-linux-gnueabihf. Would you please investigate?

Let me know if you need help reproducing the problem.

[MaskRay]

In D74006#1907258, @maxim-kuvyrkov wrote:

@MaskRay , hi. This patch miscompiles 471.omnetpp and 453.povray on arm-linux-gnueabihf. Would you please investigate?

Let me know if you need help reproducing the problem.

I just cloned llvm-test-suite. Do you mean the following files? I have no experience configuring SPEC. Can you give more detailed instructions?

% fd -u omnetpp
External/SPEC/CINT2006/471.omnetpp
External/SPEC/CINT2006/471.omnetpp/471.omnetpp.reference_output
External/SPEC/CINT2017rate/520.omnetpp_r
External/SPEC/CINT2017speed/620.omnetpp_s
LNTBased/speccpu2006/int/471.omnetpp

lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp had one bug which was fixed now: D75095.

Are you using the latest llvm-project?

[efriedma]

Cross-compiling the LLVM testsuite is a bit painful; I wouldn't recommend trying to set it up unless you really need it. But see http://llvm.org/docs/TestSuiteGuide.html describes SPEC and cross-compiling, if you really want to,

My team's internal buildbot indicates SPEC2006 is passing for arm-linux-gnueabi. So I think more details are necessary. What compile flags are you using? Are you using "test" or "ref" input? Is there a compiler error, or is it failing at runtime?

[maxim-kuvyrkov]

It's runtime failure and occurs on "ref" input. Compilation flags are
-O3 -marm --target=armv7a-linux-gnueabihf --sysroot=/path/to/build/sysroots/arm-linux-gnueabihf
and linking flags are (note that I'm linking with LLD).
-Wl,--build-id -Wl,-dynamic-linker=/path/to/run/sysroot/lib/ld-2.31.9000.so -Wl,-rpath=/path/to/run/sysroot/lib -Wl,-rpath=/path/to/run/sysroot/usr/lib -O3 -marm --target=armv7a-linux-gnueabihf --sysroot=/path/to/build/sysroots/arm-linux-gnueabihf -fuse-ld=lld

Same failure occurs with -Os/-Oz with -mthumb.

I'll check whether linking with BFD linker avoids the problem.

Omnetpp fails with:
1583131428.18: 471.omnetpp: copy 0 non-zero return code (exit code=6, signal=0)
1583131428.18: ********
1583131428.18: Contents of omnetpp.err
1583131428.18: ********
1583131428.18: terminate called after throwing an instance of 'cTerminationException*'
1583131428.18: Aborted
1583131428.18: ********

and povray fails with
1583130534.05: 453.povray: copy 0 non-zero return code (exit code=6, signal=0)

[efriedma]

I briefly made a couple attempts to reproduce SPEC2006 failures, but failed; all the tests pass for me, with both binutils ld and lld. My setup isn't hardfloat, but that seems unlikely to be relevant.

Are you using latest master? (In particular, more recent than D75095?)

It would be helpful if you could try to figure out what's actually different about the executables before/after this patch, using objdump. I mean, the failures are consistent with something going wrong generating the unwind info section, but it's not clear what specifically is going wrong.

llvm/include/llvm/MC/MCContext.h

private:
     /// differences between temporary and non-temporary labels (primarily on
     /// Darwin).
     bool AllowTemporaryLabels = true;
     bool UseNamesOnTempLabels = true;
 
     /// The Compile Unit ID that we are currently processing.
     unsigned DwarfCompileUnitID = 0;
 
+    // Sections are differentiated by the quadruple (section_name, group_name,
+    // unique_id, link_to_symbol_name). Sections sharing the same quadruple are
+    // combined into one section.
     struct ELFSectionKey {
       std::string SectionName;
       StringRef GroupName;
+      StringRef LinkedToName;
       unsigned UniqueID;
 
       ELFSectionKey(StringRef SectionName, StringRef GroupName,
-                    unsigned UniqueID)
-          : SectionName(SectionName), GroupName(GroupName), UniqueID(UniqueID) {
-      }
+                    StringRef LinkedToName, unsigned UniqueID)
+          : SectionName(SectionName), GroupName(GroupName),
+            LinkedToName(LinkedToName), UniqueID(UniqueID) {}
 
       bool operator<(const ELFSectionKey &Other) const {
         if (SectionName != Other.SectionName)
           return SectionName < Other.SectionName;
         if (GroupName != Other.GroupName)
           return GroupName < Other.GroupName;
+        if (int O = LinkedToName.compare(Other.LinkedToName))
+          return O < 0;
         return UniqueID < Other.UniqueID;
       }
     };
 
     struct COFFSectionKey {
       std::string SectionName;
       StringRef GroupName;
       int SelectionKey;
       unsigned UniqueID;

llvm/lib/MC/MCContext.cpp

return Entry = new (MachOAllocator.Allocate()) MCSectionMachO(
              Segment, Section, TypeAndAttributes, Reserved2, Kind, Begin);
 }
 
 void MCContext::renameELFSection(MCSectionELF *Section, StringRef Name) {
   StringRef GroupName;
   if (const MCSymbol *Group = Section->getGroup())
     GroupName = Group->getName();
 
+  // This function is only used by .debug*, which should not have the
+  // SHF_LINK_ORDER flag.
   unsigned UniqueID = Section->getUniqueID();
   ELFUniquingMap.erase(
-      ELFSectionKey{Section->getSectionName(), GroupName, UniqueID});
-  auto I = ELFUniquingMap.insert(std::make_pair(
-                                     ELFSectionKey{Name, GroupName, UniqueID},
-                                     Section))
+      ELFSectionKey{Section->getSectionName(), GroupName, "", UniqueID});
+  auto I = ELFUniquingMap
+               .insert(std::make_pair(
+                   ELFSectionKey{Name, GroupName, "", UniqueID}, Section))
                .first;
   StringRef CachedName = I->first.SectionName;
   const_cast<MCSectionELF *>(Section)->setSectionName(CachedName);
 }
 
 MCSectionELF *MCContext::createELFSectionImpl(StringRef Section, unsigned Type,
                                               unsigned Flags, SectionKind K,
                                               unsigned EntrySize,
MCSectionELF *MCContext::getELFSection(const Twine &Section, unsigned Type,
                                        unsigned Flags, unsigned EntrySize,
                                        const MCSymbolELF *GroupSym,
                                        unsigned UniqueID,
                                        const MCSymbolELF *LinkedToSym) {
   StringRef Group = "";
   if (GroupSym)
     Group = GroupSym->getName();
   // Do the lookup, if we have a hit, return it.
-  auto IterBool = ELFUniquingMap.insert(
-      std::make_pair(ELFSectionKey{Section.str(), Group, UniqueID}, nullptr));
+  auto IterBool = ELFUniquingMap.insert(std::make_pair(
+      ELFSectionKey{Section.str(), Group,
+                    LinkedToSym ? LinkedToSym->getName() : "", UniqueID},
+      nullptr));
   auto &Entry = *IterBool.first;
   if (!IterBool.second)
     return Entry.second;
 
   StringRef CachedName = Entry.first.SectionName;
 
   SectionKind Kind;
   if (Flags & ELF::SHF_ARM_PURECODE)

llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll

 ; CHECK-NEXT: .Lfunc_begin2:
 ; CHECK-NEXT: .cfi_startproc
 ; CHECK-NEXT: // %bb.0:
 ; CHECK-NEXT:  hint #34
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  ret
 ; CHECK:      .Lfunc_end2:
 ; CHECK-NEXT: .size f2_1, .Lfunc_end2-f2_1
-; CHECK:      .section __patchable_function_entries,"awo",@progbits,f1,unique,0
+; CHECK:      .section __patchable_function_entries,"awo",@progbits,f2_1,unique,0
 ; CHECK-NEXT: .p2align 3
 ; CHECK-NEXT: .xword .Ltmp0
   ret void
 }
 
 ;; -fpatchable-function-entry=1 -mbranch-protection=bti
 ;; For M=0, don't create .Lpatch0 if the initial instruction is not BTI,
 ;; even if other basic blocks may have BTI.
 define internal void @f1i(i64 %v) "patchable-function-entry"="1" "branch-target-enforcement" {
 ; CHECK-LABEL: f1i:
 ; CHECK-NEXT: .Lfunc_begin3:
 ; CHECK:      // %bb.0:
 ; CHECK-NEXT:  nop
 ;; Other basic blocks have BTI, but they don't affect our decision to not create .Lpatch0
 ; CHECK:      .LBB{{.+}} // %sw.bb1
 ; CHECK-NEXT:  hint #36
-; CHECK:      .section __patchable_function_entries,"awo",@progbits,f1,unique,0
+; CHECK:      .section __patchable_function_entries,"awo",@progbits,f1i,unique,0
 ; CHECK-NEXT: .p2align 3
 ; CHECK-NEXT: .xword .Lfunc_begin3
 entry:
   switch i64 %v, label %sw.bb0 [
     i64 1, label %sw.bb1
     i64 2, label %sw.bb2
     i64 3, label %sw.bb3
     i64 4, label %sw.bb4

llvm/test/CodeGen/AArch64/patchable-function-entry.ll

 ;; They share the __patchable_function_entries section.
 ;; With -function-sections, f1 and f2 are in different text sections.
 ;; Use separate __patchable_function_entries.
 define void @f2() "patchable-function-entry"="2" {
 ; CHECK-LABEL: f2:
 ; CHECK-NEXT: .Lfunc_begin2:
 ; CHECK-COUNT-2: nop
 ; CHECK-NEXT:  ret
-; NOFSECT:     .section __patchable_function_entries,"awo",@progbits,f1,unique,0
+; NOFSECT:     .section __patchable_function_entries,"awo",@progbits,f2,unique,0
 ; FSECT:       .section __patchable_function_entries,"awo",@progbits,f2,unique,1
 ; CHECK-NEXT:  .p2align 3
 ; CHECK-NEXT:  .xword .Lfunc_begin2
   ret void
 }
 
 $f3 = comdat any
 define void @f3() "patchable-function-entry"="3" comdat {

 ; CHECK-NEXT:  nop
 ; CHECK-NEXT: f3_2:
 ; CHECK:      // %bb.0:
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  ret
 ;; .size does not include the prefix.
 ; CHECK:      .Lfunc_end5:
 ; CHECK-NEXT: .size f3_2, .Lfunc_end5-f3_2
-; NOFSECT     .section __patchable_function_entries,"awo",@progbits,f1,unique,0
+; NOFSECT:    .section __patchable_function_entries,"awo",@progbits,f3_2,unique,0
 ; FSECT:      .section __patchable_function_entries,"awo",@progbits,f3_2,unique,4
 ; CHECK:      .p2align 3
 ; CHECK-NEXT: .xword .Ltmp1
   ret void
 }
 
 ;; When prefix data is used, arbitrarily place NOPs after prefix data.
 define void @prefix() "patchable-function-entry"="0" "patchable-function-prefix"="1" prefix i32 1 {

llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll

 ; CHECK-NEXT: .cfi_startproc
 ; CHECK-NEXT: # %bb.0:
 ; 32-NEXT:     endbr32
 ; 64-NEXT:     endbr64
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  ret
 ; CHECK:      .Lfunc_end2:
 ; CHECK-NEXT: .size f2_1, .Lfunc_end2-f2_1
-; CHECK:      .section __patchable_function_entries,"awo",@progbits,f1,unique,0
+; CHECK:      .section __patchable_function_entries,"awo",@progbits,f2_1,unique,0
 ; 32-NEXT:    .p2align 2
 ; 32-NEXT:    .long .Ltmp0
 ; 64-NEXT:    .p2align 3
 ; 64-NEXT:    .quad .Ltmp0
   ret void
 }
 
 ;; -fpatchable-function-entry=1 -fcf-protection=branch
 ;; For M=0, don't create .Lpatch0 if the initial instruction is not ENDBR,
 ;; even if other basic blocks may have ENDBR.
 @buf = internal global [5 x i8*] zeroinitializer
 declare i32 @llvm.eh.sjlj.setjmp(i8*)
 
 define internal void @f1i() "patchable-function-entry"="1" {
 ; CHECK-LABEL: f1i:
 ; CHECK-NEXT: .Lfunc_begin3:
 ; CHECK-NEXT: .cfi_startproc
 ; CHECK-NEXT: # %bb.0:
 ; CHECK-NEXT:  nop
 ; CHECK-NOT:  .Lpatch0:
 ;; Another basic block has ENDBR, but it doesn't affect our decision to not create .Lpatch0
 ; CHECK:       endbr
-; CHECK:      .section __patchable_function_entries,"awo",@progbits,f1,unique,0
+; CHECK:      .section __patchable_function_entries,"awo",@progbits,f1i,unique,0
 ; 32-NEXT:    .p2align 2
 ; 32-NEXT:    .long .Lfunc_begin3
 ; 64-NEXT:    .p2align 3
 ; 64-NEXT:    .quad .Lfunc_begin3
 entry:
   tail call i32 @llvm.eh.sjlj.setjmp(i8* bitcast ([5 x i8*]* @buf to i8*))
   ret void
 }
 
 !llvm.module.flags = !{!0}
 
 !0 = !{i32 4, !"cf-protection-branch", i32 1}

llvm/test/CodeGen/X86/patchable-function-entry.ll

 ;; With -function-sections, f1 and f2 are in different text sections.
 ;; Use separate __patchable_function_entries.
 define void @f2() "patchable-function-entry"="2" {
 ; CHECK-LABEL: f2:
 ; CHECK-NEXT: .Lfunc_begin2:
 ; 32-COUNT-2:  nop
 ; 64:          xchgw %ax, %ax
 ; CHECK-NEXT:  ret
-; NOFSECT:     .section __patchable_function_entries,"awo",@progbits,f1,unique,0
+; NOFSECT:     .section __patchable_function_entries,"awo",@progbits,f2,unique,0
 ; FSECT:       .section __patchable_function_entries,"awo",@progbits,f2,unique,1
 ; 32:          .p2align 2
 ; 32-NEXT:     .long .Lfunc_begin2
 ; 64:          .p2align 3
 ; 64-NEXT:     .quad .Lfunc_begin2
   ret void
 }
 

 ; CHECK-NEXT:  nop
 ; CHECK-NEXT: f3_2:
 ; CHECK:      # %bb.0:
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  ret
 ;; .size does not include the prefix.
 ; CHECK:      .Lfunc_end5:
 ; CHECK-NEXT: .size f3_2, .Lfunc_end5-f3_2
-; NOFSECT     .section __patchable_function_entries,"awo",@progbits,f0,unique,0
+; NOFSECT:    .section __patchable_function_entries,"awo",@progbits,f3_2,unique,0
 ; FSECT:      .section __patchable_function_entries,"awo",@progbits,f3_2,unique,4
 ; 32:         .p2align 2
 ; 32-NEXT:    .long .Ltmp0
 ; 64:         .p2align 3
 ; 64-NEXT:    .quad .Ltmp0
   %frame = alloca i8, i32 16
   ret void
 }

llvm/test/MC/ELF/comdat-dup-group-name.s

-// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - | llvm-readobj -S --symbols | FileCheck %s
-
-// Test that we produce two foo sections, each in separate groups
-
-// CHECK: Index: 3
-// CHECK-NEXT: Name: .group
-
-// CHECK: Index: 4
-// CHECK-NEXT: Name: .foo
-
-// CHECK: Index: 5
-// CHECK-NEXT: Name: .group
-
-// CHECK: Index: 6
-// CHECK-NEXT: Name: .foo
-
-// CHECK: Symbols [
-
-// CHECK: Name: f1
-// CHECK-NOT: }
-// CHECK: Section: .group (0x3)
-
-// CHECK: Name: f2
-// CHECK-NOT: }
-// CHECK: Section: .group (0x5)
-
-  .section  .foo,"axG",@progbits,f1,comdat
-        nop
-
-  .section  .foo,"axG",@progbits,f2,comdat
-        nop
-

llvm/test/MC/ELF/section-combine.s

+## Sections are differentiated by the quadruple
+## (section_name, group_name, unique_id, link_to_symbol_name).
+## Sections sharing the same quadruple are combined into one section.
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t
+# RUN: llvm-readelf -x .foo %t | FileCheck %s
+
+# CHECK:      Hex dump of section '.foo':
+# CHECK-NEXT: 0x00000000 00
+# CHECK:      Hex dump of section '.foo':
+# CHECK-NEXT: 0x00000000 0102
+# CHECK:      Hex dump of section '.foo':
+# CHECK-NEXT: 0x00000000 03
+# CHECK:      Hex dump of section '.foo':
+# CHECK-NEXT: 0x00000000 0405
+# CHECK:      Hex dump of section '.foo':
+# CHECK-NEXT: 0x00000000 06
+# CHECK:      Hex dump of section '.foo':
+# CHECK-NEXT: 0x00000000 0708
+
+foo:
+bar:
+
+## foo and bar are in the same section. However, a section referencing foo
+## is considered different from a section referencing bar.
+.section .foo,"o",@progbits,foo
+.byte 0
+
+.section .foo,"o",@progbits,bar
+.byte 1
+.section .foo,"o",@progbits,bar
+.byte 2
+
+.section .foo,"o",@progbits,bar,unique,0
+.byte 3
+
+.section .foo,"o",@progbits,bar,unique,1
+.byte 4
+.section .foo,"o",@progbits,bar,unique,1
+.byte 5
+
+.section .foo,"Go",@progbits,comdat0,comdat,bar,unique,1
+.byte 6
+
+.section .foo,"Go",@progbits,comdat1,comdat,bar,unique,1
+.byte 7
+.section .foo,"Go",@progbits,comdat1,comdat,bar,unique,1
+.byte 8