From 0fbe2215438b6598b4bc54c64812cab792ba5862 Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Tue, 4 Feb 2020 13:52:10 -0800 Subject: [PATCH] [MC][ELF] Make linked-to symbol name part of ELFSectionKey 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 ``` to ``` .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. Differential Revision: https://reviews.llvm.org/D74006 --- llvm/include/llvm/MC/MCContext.h | 12 ++++-- llvm/lib/MC/MCContext.cpp | 16 +++++--- .../AArch64/patchable-function-entry-bti.ll | 4 +- .../CodeGen/AArch64/patchable-function-entry.ll | 4 +- .../CodeGen/X86/patchable-function-entry-ibt.ll | 4 +- llvm/test/CodeGen/X86/patchable-function-entry.ll | 4 +- llvm/test/MC/ELF/comdat-dup-group-name.s | 32 --------------- llvm/test/MC/ELF/section-combine.s | 48 ++++++++++++++++++++++ 8 files changed, 75 insertions(+), 49 deletions(-) delete mode 100644 llvm/test/MC/ELF/comdat-dup-group-name.s create mode 100644 llvm/test/MC/ELF/section-combine.s diff --git a/llvm/include/llvm/MC/MCContext.h b/llvm/include/llvm/MC/MCContext.h index 7e8c233..e516ec8 100644 --- a/llvm/include/llvm/MC/MCContext.h +++ b/llvm/include/llvm/MC/MCContext.h @@ -193,21 +193,27 @@ namespace llvm { /// 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; } }; diff --git a/llvm/lib/MC/MCContext.cpp b/llvm/lib/MC/MCContext.cpp index cbd823e..8b3caf7 100644 --- a/llvm/lib/MC/MCContext.cpp +++ b/llvm/lib/MC/MCContext.cpp @@ -316,12 +316,14 @@ void MCContext::renameELFSection(MCSectionELF *Section, StringRef Name) { 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(Section)->setSectionName(CachedName); @@ -404,8 +406,10 @@ MCSectionELF *MCContext::getELFSection(const Twine &Section, unsigned Type, 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; diff --git a/llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll b/llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll index 19386e9..e86ffe0 100644 --- a/llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll +++ b/llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll @@ -41,7 +41,7 @@ define void @f2_1() "patchable-function-entry"="1" "patchable-function-prefix"=" ; 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 @@ -58,7 +58,7 @@ define internal void @f1i(i64 %v) "patchable-function-entry"="1" "branch-target- ;; 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: diff --git a/llvm/test/CodeGen/AArch64/patchable-function-entry.ll b/llvm/test/CodeGen/AArch64/patchable-function-entry.ll index 5ae9d88..71b3f67 100644 --- a/llvm/test/CodeGen/AArch64/patchable-function-entry.ll +++ b/llvm/test/CodeGen/AArch64/patchable-function-entry.ll @@ -34,7 +34,7 @@ define void @f2() "patchable-function-entry"="2" { ; 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 @@ -82,7 +82,7 @@ define void @f3_2() "patchable-function-entry"="1" "patchable-function-prefix"=" ;; .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 diff --git a/llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll b/llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll index 3def5a3..e74e87f 100644 --- a/llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll +++ b/llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll @@ -50,7 +50,7 @@ define void @f2_1() "patchable-function-entry"="1" "patchable-function-prefix"=" ; 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 @@ -73,7 +73,7 @@ define internal void @f1i() "patchable-function-entry"="1" { ; 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 diff --git a/llvm/test/CodeGen/X86/patchable-function-entry.ll b/llvm/test/CodeGen/X86/patchable-function-entry.ll index dcf14df..0047687 100644 --- a/llvm/test/CodeGen/X86/patchable-function-entry.ll +++ b/llvm/test/CodeGen/X86/patchable-function-entry.ll @@ -34,7 +34,7 @@ define void @f2() "patchable-function-entry"="2" { ; 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 @@ -91,7 +91,7 @@ define void @f3_2() "patchable-function-entry"="1" "patchable-function-prefix"=" ;; .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 diff --git a/llvm/test/MC/ELF/comdat-dup-group-name.s b/llvm/test/MC/ELF/comdat-dup-group-name.s deleted file mode 100644 index ffa8b2b..0000000 --- a/llvm/test/MC/ELF/comdat-dup-group-name.s +++ /dev/null @@ -1,32 +0,0 @@ -// 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 - diff --git a/llvm/test/MC/ELF/section-combine.s b/llvm/test/MC/ELF/section-combine.s new file mode 100644 index 0000000..b68eaa3 --- /dev/null +++ b/llvm/test/MC/ELF/section-combine.s @@ -0,0 +1,48 @@ +## 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 -- 2.7.4