From 792c206e2b63c16075d759d3abc3eb5399ed74fe Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Mon, 26 Jul 2021 09:05:18 -0700 Subject: [PATCH] [llvm-objcopy] Drop GRP_COMDAT if the group signature is localized See [GRP_COMDAT group with STB_LOCAL signature](https://groups.google.com/g/generic-abi/c/2X6mR-s2zoc) objcopy PR: https://sourceware.org/bugzilla/show_bug.cgi?id=27931 GRP_COMDAT deduplication is purely based on the signature symbol name in ld.lld/GNU ld/gold. The local/global status is not part of the equation. If the signature symbol is localized by --localize-hidden or --keep-global-symbol, the intention is likely to make the group fully localized. Drop GRP_COMDAT to suppress deduplication. Reviewed By: jhenderson Differential Revision: https://reviews.llvm.org/D106782 --- .../tools/llvm-objcopy/ELF/group-reorder.test | 1 + llvm/test/tools/llvm-objcopy/ELF/group.test | 49 +++++++++++++++++++ .../ELF/remove-section-in-group.test | 1 + .../llvm-objcopy/ELF/strip-dwo-groups.test | 10 ++-- llvm/tools/llvm-objcopy/ELF/Object.cpp | 6 +++ 5 files changed, 63 insertions(+), 4 deletions(-) diff --git a/llvm/test/tools/llvm-objcopy/ELF/group-reorder.test b/llvm/test/tools/llvm-objcopy/ELF/group-reorder.test index ce163da90c10..db2f733fbf8a 100644 --- a/llvm/test/tools/llvm-objcopy/ELF/group-reorder.test +++ b/llvm/test/tools/llvm-objcopy/ELF/group-reorder.test @@ -61,4 +61,5 @@ Symbols: Section: .bar - Name: bar Type: STT_FUNC + Binding: STB_GLOBAL Section: .foo diff --git a/llvm/test/tools/llvm-objcopy/ELF/group.test b/llvm/test/tools/llvm-objcopy/ELF/group.test index 806653d64440..64c97b17d185 100644 --- a/llvm/test/tools/llvm-objcopy/ELF/group.test +++ b/llvm/test/tools/llvm-objcopy/ELF/group.test @@ -78,3 +78,52 @@ Symbols: ## sh_info fields. # RUN: llvm-objcopy --allow-broken-links -R .symtab %t3 %t4 # RUN: cmp %t3 %t4 + +## The signature symbol becomes local. Assume the intention is to localize the group. +## Drop GRP_COMDAT so that the linker will suppress deduplication. +# RUN: llvm-objcopy --keep-global-symbol=bar %t %t5 +# RUN: llvm-readelf -s --section-groups %t5 | FileCheck %s --check-prefix=LOCAL-SIG + +# LOCAL-SIG: LOCAL DEFAULT [[#]] foo +# LOCAL-SIG: (unknown) group section [ 1] `.group' [foo] contains 1 sections: + +## The signature symbol remains non-local. Keep GRP_COMDAT. +# RUN: llvm-readelf -s --section-groups %t1 | FileCheck %s --check-prefix=WEAK-SIG + +# WEAK-SIG: WEAK DEFAULT [[#]] foo +# WEAK-SIG: COMDAT group section [ 1] `.group' [foo] contains 1 sections: + +# RUN: llvm-objcopy --globalize-symbol=foo %t %t6 +# RUN: llvm-readelf -s --section-groups %t6 | FileCheck %s --check-prefix=GLOBAL-SIG + +# GLOBAL-SIG: GLOBAL DEFAULT [[#]] foo +# GLOBAL-SIG: COMDAT group section [ 1] `.group' [foo] contains 1 sections: + +## If the signature is initially local and no operation has been performed to +## specifically localize it, it isn't clear whether we should drop GRP_COMDAT. +## The current convention is that compilers should not produce such input, so +## our choice does not matter. +# RUN: yaml2obj --docnum=2 %s -o %t.localsig +# RUN: llvm-objcopy %t.localsig %t.localsig.out +# RUN: llvm-readelf -s --section-groups %t.localsig.out | FileCheck %s --check-prefix=LOCAL-SIG + +--- !ELF +FileHeader: + Class: ELFCLASS64 + Data: ELFDATA2LSB + Type: ET_REL + Machine: EM_X86_64 +Sections: + - Name: .group + Type: SHT_GROUP + Info: foo + Members: + - SectionOrType: GRP_COMDAT + - SectionOrType: .text.foo + - Name: .text.foo + Type: SHT_PROGBITS + Flags: [ SHF_GROUP ] +Symbols: + - Name: foo + Type: STT_FUNC + Section: .text.foo diff --git a/llvm/test/tools/llvm-objcopy/ELF/remove-section-in-group.test b/llvm/test/tools/llvm-objcopy/ELF/remove-section-in-group.test index 6b2e43d68033..9e683b9f68c9 100644 --- a/llvm/test/tools/llvm-objcopy/ELF/remove-section-in-group.test +++ b/llvm/test/tools/llvm-objcopy/ELF/remove-section-in-group.test @@ -34,3 +34,4 @@ Sections: Symbols: - Name: foo_bar_grp Section: .group + Binding: STB_GLOBAL diff --git a/llvm/test/tools/llvm-objcopy/ELF/strip-dwo-groups.test b/llvm/test/tools/llvm-objcopy/ELF/strip-dwo-groups.test index 77727ca94222..93c35117fa8f 100644 --- a/llvm/test/tools/llvm-objcopy/ELF/strip-dwo-groups.test +++ b/llvm/test/tools/llvm-objcopy/ELF/strip-dwo-groups.test @@ -75,9 +75,11 @@ Sections: Symbols: - Name: debug_before Section: .debug_before.dwo - - Name: group2 - Section: .text.group2 - - Name: group1 - Section: .text.group1 - Name: debug_after Section: .debug_after.dwo + - Name: group2 + Section: .text.group2 + Binding: STB_WEAK + - Name: group1 + Section: .text.group1 + Binding: STB_WEAK diff --git a/llvm/tools/llvm-objcopy/ELF/Object.cpp b/llvm/tools/llvm-objcopy/ELF/Object.cpp index 0255fdaec5d4..91a73f2111ae 100644 --- a/llvm/tools/llvm-objcopy/ELF/Object.cpp +++ b/llvm/tools/llvm-objcopy/ELF/Object.cpp @@ -1069,6 +1069,12 @@ Error Section::removeSectionReferences( void GroupSection::finalize() { this->Info = Sym ? Sym->Index : 0; this->Link = SymTab ? SymTab->Index : 0; + // Linker deduplication for GRP_COMDAT is based on Sym->Name. The local/global + // status is not part of the equation. If Sym is localized, the intention is + // likely to make the group fully localized. Drop GRP_COMDAT to suppress + // deduplication. See https://groups.google.com/g/generic-abi/c/2X6mR-s2zoc + if ((FlagWord & GRP_COMDAT) && Sym && Sym->Binding == STB_LOCAL) + this->FlagWord &= ~GRP_COMDAT; } Error GroupSection::removeSectionReferences(