* [PATCH] x86: Keep __patchable_function_entries sections with --gc-sections @ 2020-02-01 16:26 H.J. Lu 2020-02-01 17:19 ` H.J. Lu 2020-02-01 17:34 ` Fangrui Song 0 siblings, 2 replies; 10+ messages in thread From: H.J. Lu @ 2020-02-01 16:26 UTC (permalink / raw) To: binutils After all text sections have been garbage collected, if a __patchable_function_entries section references a section which wasn't marked, mark it with SEC_EXCLUDE and return NULL. Otherwise, keep it. Should it be handled in _bfd_elf_gc_mark_extra_sections? H.J. --- bfd/ PR ld/25490 * elfxx-x86.c (_bfd_x86_elf_gc_mark_hook): Handle __patchable_function_entries sections. (_bfd_x86_elf_gc_mark_extra_sections): New function. * elfxx-x86.h (_bfd_x86_elf_gc_mark_extra_sections): New. (elf_backend_gc_mark_extra_sections): Likewise. ld/ PR ld/25490 * testsuite/ld-i386/i386.exp: Run PR ld/25490 tests. * testsuite/ld-x86-64/x86-64.exp: Likewise. * testsuite/ld-i386/pr25490-1.d: Likewise. * testsuite/ld-i386/pr25490-2a.d: Likewise. * testsuite/ld-i386/pr25490-2b.d: Likewise. * testsuite/ld-x86-64/pr25490-1-x32.d: Likewise. * testsuite/ld-x86-64/pr25490-1.d: Likewise. * testsuite/ld-x86-64/pr25490-1.s: Likewise. * testsuite/ld-x86-64/pr25490-2.s: Likewise. * testsuite/ld-x86-64/pr25490-2a-x32.d: Likewise. * testsuite/ld-x86-64/pr25490-2a.d: Likewise. * testsuite/ld-x86-64/pr25490-2b-x32.d: Likewise. * testsuite/ld-x86-64/pr25490-2b.d: Likewise. --- bfd/elfxx-x86.c | 44 +++++++++++++++++++++++++ bfd/elfxx-x86.h | 5 +++ ld/testsuite/ld-i386/i386.exp | 3 ++ ld/testsuite/ld-i386/pr25490-1.d | 8 +++++ ld/testsuite/ld-i386/pr25490-2a.d | 8 +++++ ld/testsuite/ld-i386/pr25490-2b.d | 9 +++++ ld/testsuite/ld-x86-64/pr25490-1-x32.d | 8 +++++ ld/testsuite/ld-x86-64/pr25490-1.d | 7 ++++ ld/testsuite/ld-x86-64/pr25490-1.s | 13 ++++++++ ld/testsuite/ld-x86-64/pr25490-2.s | 26 +++++++++++++++ ld/testsuite/ld-x86-64/pr25490-2a-x32.d | 8 +++++ ld/testsuite/ld-x86-64/pr25490-2a.d | 8 +++++ ld/testsuite/ld-x86-64/pr25490-2b-x32.d | 9 +++++ ld/testsuite/ld-x86-64/pr25490-2b.d | 9 +++++ ld/testsuite/ld-x86-64/x86-64.exp | 6 ++++ 15 files changed, 171 insertions(+) create mode 100644 ld/testsuite/ld-i386/pr25490-1.d create mode 100644 ld/testsuite/ld-i386/pr25490-2a.d create mode 100644 ld/testsuite/ld-i386/pr25490-2b.d create mode 100644 ld/testsuite/ld-x86-64/pr25490-1-x32.d create mode 100644 ld/testsuite/ld-x86-64/pr25490-1.d create mode 100644 ld/testsuite/ld-x86-64/pr25490-1.s create mode 100644 ld/testsuite/ld-x86-64/pr25490-2.s create mode 100644 ld/testsuite/ld-x86-64/pr25490-2a-x32.d create mode 100644 ld/testsuite/ld-x86-64/pr25490-2a.d create mode 100644 ld/testsuite/ld-x86-64/pr25490-2b-x32.d create mode 100644 ld/testsuite/ld-x86-64/pr25490-2b.d diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c index fc783b0e988..ec2ab0ce72b 100644 --- a/bfd/elfxx-x86.c +++ b/bfd/elfxx-x86.c @@ -2110,10 +2110,54 @@ _bfd_x86_elf_gc_mark_hook (asection *sec, case R_X86_64_GNU_VTENTRY: return NULL; } + else if (CONST_STRNEQ (bfd_section_name (sec), + "__patchable_function_entries")) + { + /* After all text sections have been garbage collected, if a + __patchable_function_entries section references a section + which wasn't marked, mark it with SEC_EXCLUDE and return + NULL. */ + asection *tsec = bfd_section_from_elf_index (sec->owner, + sym->st_shndx); + if (!tsec->gc_mark) + { + sec->flags |= SEC_EXCLUDE; + return NULL; + } + } return _bfd_elf_gc_mark_hook (sec, info, rel, h, sym); } +/* Prevent __patchable_function_entries sections from being discarded + with --gc-sections. */ + +bfd_boolean +_bfd_x86_elf_gc_mark_extra_sections (struct bfd_link_info *info, + elf_gc_mark_hook_fn gc_mark_hook) +{ + bfd *sub; + + _bfd_elf_gc_mark_extra_sections (info, gc_mark_hook); + + for (sub = info->input_bfds; sub != NULL; sub = sub->link.next) + { + asection *o; + + for (o = sub->sections; o != NULL; o = o->next) + if (!o->gc_mark + && (o->flags & SEC_EXCLUDE) == 0 + && CONST_STRNEQ (bfd_section_name (o), + "__patchable_function_entries")) + { + if (!_bfd_elf_gc_mark (info, o, gc_mark_hook)) + return FALSE; + } + } + + return TRUE; +} + static bfd_vma elf_i386_get_plt_got_vma (struct elf_x86_plt *plt_p ATTRIBUTE_UNUSED, bfd_vma off, diff --git a/bfd/elfxx-x86.h b/bfd/elfxx-x86.h index bef17dc2bac..1e43f1d786c 100644 --- a/bfd/elfxx-x86.h +++ b/bfd/elfxx-x86.h @@ -688,6 +688,9 @@ extern asection * _bfd_x86_elf_gc_mark_hook (asection *, struct bfd_link_info *, Elf_Internal_Rela *, struct elf_link_hash_entry *, Elf_Internal_Sym *); +extern bfd_boolean _bfd_x86_elf_gc_mark_extra_sections + (struct bfd_link_info *, elf_gc_mark_hook_fn); + extern long _bfd_x86_elf_get_synthetic_symtab (bfd *, long, long, bfd_vma, struct elf_x86_plt [], asymbol **, asymbol **); @@ -737,6 +740,8 @@ extern void _bfd_x86_elf_link_fixup_ifunc_symbol _bfd_x86_elf_adjust_dynamic_symbol #define elf_backend_gc_mark_hook \ _bfd_x86_elf_gc_mark_hook +#define elf_backend_gc_mark_extra_sections \ + _bfd_x86_elf_gc_mark_extra_sections #define elf_backend_omit_section_dynsym \ _bfd_elf_omit_section_dynsym_all #define elf_backend_parse_gnu_properties \ diff --git a/ld/testsuite/ld-i386/i386.exp b/ld/testsuite/ld-i386/i386.exp index 7bb3636d3b0..68a43e3c35d 100644 --- a/ld/testsuite/ld-i386/i386.exp +++ b/ld/testsuite/ld-i386/i386.exp @@ -496,6 +496,9 @@ run_dump_test "pr23930" run_dump_test "pr24322a" run_dump_test "pr24322b" run_dump_test "align-branch-1" +run_dump_test "pr25490-1" +run_dump_test "pr25490-2a" +run_dump_test "pr25490-2b" if { !([istarget "i?86-*-linux*"] || [istarget "i?86-*-gnu*"] diff --git a/ld/testsuite/ld-i386/pr25490-1.d b/ld/testsuite/ld-i386/pr25490-1.d new file mode 100644 index 00000000000..67cc5e86ec2 --- /dev/null +++ b/ld/testsuite/ld-i386/pr25490-1.d @@ -0,0 +1,8 @@ +#source: ../ld-x86-64/pr25490-1.s +#as: --32 +#ld: --gc-sections -melf_i386 +#readelf: -SW + +#... + +\[ *[0-9]+\] __patchable_function_entries +PROGBITS +[0-9a-f]+ +[0-9a-f]+000 +0+4 +00 +WA +0 +0 +1 +#pass diff --git a/ld/testsuite/ld-i386/pr25490-2a.d b/ld/testsuite/ld-i386/pr25490-2a.d new file mode 100644 index 00000000000..5096252829f --- /dev/null +++ b/ld/testsuite/ld-i386/pr25490-2a.d @@ -0,0 +1,8 @@ +#source: ../ld-x86-64/pr25490-2.s +#as: --32 +#ld: --gc-sections -melf_i386 +#readelf: -SW + +#... + +\[ *[0-9]+\] __patchable_function_entries._start +PROGBITS +[0-9a-f]+ +[0-9a-f]+000 +0+4 +00 +WA +0 +0 +1 +#pass diff --git a/ld/testsuite/ld-i386/pr25490-2b.d b/ld/testsuite/ld-i386/pr25490-2b.d new file mode 100644 index 00000000000..3f9cab548be --- /dev/null +++ b/ld/testsuite/ld-i386/pr25490-2b.d @@ -0,0 +1,9 @@ +#source: ../ld-x86-64/pr25490-2.s +#as: --32 +#ld: --gc-sections -melf_i386 +#readelf: -SW + +#failif +#... + +\[ *[0-9]+\] __patchable_function_entries.bar +PROGBITS +.* +#pass diff --git a/ld/testsuite/ld-x86-64/pr25490-1-x32.d b/ld/testsuite/ld-x86-64/pr25490-1-x32.d new file mode 100644 index 00000000000..65bc2bee28c --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-1-x32.d @@ -0,0 +1,8 @@ +#source: pr25490-1.s +#as: --x32 +#ld: --gc-sections -melf32_x86_64 +#readelf: -SW + +#... + +\[ *[0-9]+\] __patchable_function_entries +PROGBITS +[0-9a-f]+ +[0-9a-f]+000 +0+4 +00 +WA +0 +0 +1 +#pass diff --git a/ld/testsuite/ld-x86-64/pr25490-1.d b/ld/testsuite/ld-x86-64/pr25490-1.d new file mode 100644 index 00000000000..dc1d912076b --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-1.d @@ -0,0 +1,7 @@ +#as: --64 +#ld: --gc-sections -melf_x86_64 +#readelf: -SW + +#... + +\[ *[0-9]+\] __patchable_function_entries +PROGBITS +[0-9a-f]+ +[0-9a-f]+000 +0+8 +00 +WA +0 +0 +1 +#pass diff --git a/ld/testsuite/ld-x86-64/pr25490-1.s b/ld/testsuite/ld-x86-64/pr25490-1.s new file mode 100644 index 00000000000..21123ad8aa0 --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-1.s @@ -0,0 +1,13 @@ + .text + .p2align 4 + .globl _start + .type _start, %function +_start: + .section __patchable_function_entries,"aw",%progbits + .dc.a .LPFE1 + .text +.LPFE1: + nop + ret + .size _start, .-_start + .section .note.GNU-stack,"",%progbits diff --git a/ld/testsuite/ld-x86-64/pr25490-2.s b/ld/testsuite/ld-x86-64/pr25490-2.s new file mode 100644 index 00000000000..1f8df870316 --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-2.s @@ -0,0 +1,26 @@ + .text + .p2align 4 + .section .text.bar,"ax",%progbits + .globl bar + .type bar, %function +bar: + .section __patchable_function_entries.bar,"aw",%progbits + .dc.a .LPFE1 + .section .text.bar,"ax",%progbits +.LPFE1: + nop + ret + .size bar, .-bar + .p2align 4 + .section .text._start,"ax",%progbits + .globl _start + .type _start, %function +_start: + .section __patchable_function_entries._start,"aw",%progbits + .dc.a .LPFE2 + .section .text._start,"ax",%progbits +.LPFE2: + nop + ret + .size _start, .-_start + .section .note.GNU-stack,"",%progbits diff --git a/ld/testsuite/ld-x86-64/pr25490-2a-x32.d b/ld/testsuite/ld-x86-64/pr25490-2a-x32.d new file mode 100644 index 00000000000..2029029a0f3 --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-2a-x32.d @@ -0,0 +1,8 @@ +#source: pr25490-2.s +#as: --x32 +#ld: --gc-sections -melf32_x86_64 +#readelf: -SW + +#... + +\[ *[0-9]+\] __patchable_function_entries._start +PROGBITS +[0-9a-f]+ +[0-9a-f]+000 +0+4 +00 +WA +0 +0 +1 +#pass diff --git a/ld/testsuite/ld-x86-64/pr25490-2a.d b/ld/testsuite/ld-x86-64/pr25490-2a.d new file mode 100644 index 00000000000..a5f0b141c87 --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-2a.d @@ -0,0 +1,8 @@ +#source: pr25490-2.s +#as: --64 +#ld: --gc-sections -melf_x86_64 +#readelf: -SW + +#... + +\[ *[0-9]+\] __patchable_function_entries._start +PROGBITS +[0-9a-f]+ +[0-9a-f]+000 +0+8 +00 +WA +0 +0 +1 +#pass diff --git a/ld/testsuite/ld-x86-64/pr25490-2b-x32.d b/ld/testsuite/ld-x86-64/pr25490-2b-x32.d new file mode 100644 index 00000000000..a9fe6ba6b3c --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-2b-x32.d @@ -0,0 +1,9 @@ +#source: pr25490-2.s +#as: --x32 +#ld: --gc-sections -melf32_x86_64 +#readelf: -SW + +#failif +#... + +\[ *[0-9]+\] __patchable_function_entries.bar +PROGBITS +.* +#pass diff --git a/ld/testsuite/ld-x86-64/pr25490-2b.d b/ld/testsuite/ld-x86-64/pr25490-2b.d new file mode 100644 index 00000000000..5eb933f5b36 --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-2b.d @@ -0,0 +1,9 @@ +#source: pr25490-2.s +#as: --64 +#ld: --gc-sections -melf_x86_64 +#readelf: -SW + +#failif +#... + +\[ *[0-9]+\] __patchable_function_entries.bar +PROGBITS +.* +#pass diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp index c78b0fd7576..e30b21d53d4 100644 --- a/ld/testsuite/ld-x86-64/x86-64.exp +++ b/ld/testsuite/ld-x86-64/x86-64.exp @@ -467,6 +467,12 @@ run_dump_test "pr25416-2a" run_dump_test "pr25416-2b" run_dump_test "pr25416-3" run_dump_test "pr25416-4" +run_dump_test "pr25490-1" +run_dump_test "pr25490-1-x32" +run_dump_test "pr25490-2a" +run_dump_test "pr25490-2a-x32" +run_dump_test "pr25490-2b" +run_dump_test "pr25490-2b-x32" if { ![istarget "x86_64-*-linux*"] && ![istarget "x86_64-*-nacl*"]} { return -- 2.24.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] x86: Keep __patchable_function_entries sections with --gc-sections 2020-02-01 16:26 [PATCH] x86: Keep __patchable_function_entries sections with --gc-sections H.J. Lu @ 2020-02-01 17:19 ` H.J. Lu 2020-02-01 17:34 ` Fangrui Song 1 sibling, 0 replies; 10+ messages in thread From: H.J. Lu @ 2020-02-01 17:19 UTC (permalink / raw) To: Binutils [-- Attachment #1: Type: text/plain, Size: 507 bytes --] On Sat, Feb 1, 2020 at 8:26 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > After all text sections have been garbage collected, if a > __patchable_function_entries section references a section which > wasn't marked, mark it with SEC_EXCLUDE and return NULL. Otherwise, > keep it. > > Should it be handled in _bfd_elf_gc_mark_extra_sections? > It should be an error if a __patchable_function_entries section references both kept and garbage collected sections. Here is the updated patch to do it. -- H.J. [-- Attachment #2: 0001-x86-Keep-__patchable_function_entries-sections-with-.patch --] [-- Type: text/x-patch, Size: 14226 bytes --] From 16dbc36a7ff358f82b95abe892e23575f502f1d9 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Sat, 1 Feb 2020 08:19:00 -0800 Subject: [PATCH] x86: Keep __patchable_function_entries sections with --gc-sections After all text sections have been garbage collected, if a __patchable_function_entries section references a section which wasn't marked, mark it with SEC_EXCLUDE and return NULL. Otherwise, keep it. Issue an error if a __patchable_function_entries section references both kept and garbage collected sections. bfd/ PR ld/25490 * elfxx-x86.c (_bfd_x86_elf_gc_mark_hook): Handle __patchable_function_entries sections. (_bfd_x86_elf_gc_mark_extra_sections): New function. * elfxx-x86.h (_bfd_x86_elf_gc_mark_extra_sections): New. (elf_backend_gc_mark_extra_sections): Likewise. ld/ PR ld/25490 * testsuite/ld-i386/i386.exp: Run PR ld/25490 tests. * testsuite/ld-x86-64/x86-64.exp: Likewise. * testsuite/ld-i386/pr25490-1.d: New file. * testsuite/ld-i386/pr25490-2a.d: Likewise. * testsuite/ld-i386/pr25490-2b.d: Likewise. * testsuite/ld-i386/pr25490-3.d: Likewise. * testsuite/ld-x86-64/pr25490-1-x32.d: Likewise. * testsuite/ld-x86-64/pr25490-1.d: Likewise. * testsuite/ld-x86-64/pr25490-1.s: Likewise. * testsuite/ld-x86-64/pr25490-2.s: Likewise. * testsuite/ld-x86-64/pr25490-2a-x32.d: Likewise. * testsuite/ld-x86-64/pr25490-2a.d: Likewise. * testsuite/ld-x86-64/pr25490-2b-x32.d: Likewise. * testsuite/ld-x86-64/pr25490-2b.d: Likewise. * testsuite/ld-x86-64/pr25490-3-x32.d: Likewise. * testsuite/ld-x86-64/pr25490-3.d: Likewise. * testsuite/ld-x86-64/pr25490-3.s: Likewise. --- bfd/elfxx-x86.c | 49 +++++++++++++++++++++++++ bfd/elfxx-x86.h | 5 +++ ld/testsuite/ld-i386/i386.exp | 4 ++ ld/testsuite/ld-i386/pr25490-1.d | 8 ++++ ld/testsuite/ld-i386/pr25490-2a.d | 8 ++++ ld/testsuite/ld-i386/pr25490-2b.d | 9 +++++ ld/testsuite/ld-i386/pr25490-3.d | 4 ++ ld/testsuite/ld-x86-64/pr25490-1-x32.d | 8 ++++ ld/testsuite/ld-x86-64/pr25490-1.d | 7 ++++ ld/testsuite/ld-x86-64/pr25490-1.s | 13 +++++++ ld/testsuite/ld-x86-64/pr25490-2.s | 26 +++++++++++++ ld/testsuite/ld-x86-64/pr25490-2a-x32.d | 8 ++++ ld/testsuite/ld-x86-64/pr25490-2a.d | 8 ++++ ld/testsuite/ld-x86-64/pr25490-2b-x32.d | 9 +++++ ld/testsuite/ld-x86-64/pr25490-2b.d | 9 +++++ ld/testsuite/ld-x86-64/pr25490-3-x32.d | 4 ++ ld/testsuite/ld-x86-64/pr25490-3.d | 3 ++ ld/testsuite/ld-x86-64/pr25490-3.s | 28 ++++++++++++++ ld/testsuite/ld-x86-64/x86-64.exp | 8 ++++ 19 files changed, 218 insertions(+) create mode 100644 ld/testsuite/ld-i386/pr25490-1.d create mode 100644 ld/testsuite/ld-i386/pr25490-2a.d create mode 100644 ld/testsuite/ld-i386/pr25490-2b.d create mode 100644 ld/testsuite/ld-i386/pr25490-3.d create mode 100644 ld/testsuite/ld-x86-64/pr25490-1-x32.d create mode 100644 ld/testsuite/ld-x86-64/pr25490-1.d create mode 100644 ld/testsuite/ld-x86-64/pr25490-1.s create mode 100644 ld/testsuite/ld-x86-64/pr25490-2.s create mode 100644 ld/testsuite/ld-x86-64/pr25490-2a-x32.d create mode 100644 ld/testsuite/ld-x86-64/pr25490-2a.d create mode 100644 ld/testsuite/ld-x86-64/pr25490-2b-x32.d create mode 100644 ld/testsuite/ld-x86-64/pr25490-2b.d create mode 100644 ld/testsuite/ld-x86-64/pr25490-3-x32.d create mode 100644 ld/testsuite/ld-x86-64/pr25490-3.d create mode 100644 ld/testsuite/ld-x86-64/pr25490-3.s diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c index fc783b0e988..758717d99ae 100644 --- a/bfd/elfxx-x86.c +++ b/bfd/elfxx-x86.c @@ -2110,10 +2110,59 @@ _bfd_x86_elf_gc_mark_hook (asection *sec, case R_X86_64_GNU_VTENTRY: return NULL; } + else if (CONST_STRNEQ (bfd_section_name (sec), + "__patchable_function_entries")) + { + /* After all text sections have been garbage collected, if a + __patchable_function_entries section references a section + which wasn't marked, mark it with SEC_EXCLUDE and return + NULL. */ + asection *tsec = bfd_section_from_elf_index (sec->owner, + sym->st_shndx); + /* NB: A __patchable_function_entries section may reference both + kept and garbage collected sections. */ + if (tsec->gc_mark) + sec->flags |= SEC_KEEP; + else + sec->flags |= SEC_EXCLUDE; + } return _bfd_elf_gc_mark_hook (sec, info, rel, h, sym); } +/* Prevent __patchable_function_entries sections from being discarded + with --gc-sections. */ + +bfd_boolean +_bfd_x86_elf_gc_mark_extra_sections (struct bfd_link_info *info, + elf_gc_mark_hook_fn gc_mark_hook) +{ + bfd *sub; + + _bfd_elf_gc_mark_extra_sections (info, gc_mark_hook); + + for (sub = info->input_bfds; sub != NULL; sub = sub->link.next) + { + asection *o; + + for (o = sub->sections; o != NULL; o = o->next) + if (!o->gc_mark + && (o->flags & SEC_EXCLUDE) == 0 + && (o->flags & SEC_KEEP) == 0 + && CONST_STRNEQ (bfd_section_name (o), + "__patchable_function_entries")) + { + if (!_bfd_elf_gc_mark (info, o, gc_mark_hook)) + return FALSE; + if ((o->flags & SEC_EXCLUDE) && (o->flags & SEC_KEEP)) + info->callbacks->einfo (_("%F%pB(%pA): reference to garbage collected section\n"), + o->owner, o); + } + } + + return TRUE; +} + static bfd_vma elf_i386_get_plt_got_vma (struct elf_x86_plt *plt_p ATTRIBUTE_UNUSED, bfd_vma off, diff --git a/bfd/elfxx-x86.h b/bfd/elfxx-x86.h index bef17dc2bac..1e43f1d786c 100644 --- a/bfd/elfxx-x86.h +++ b/bfd/elfxx-x86.h @@ -688,6 +688,9 @@ extern asection * _bfd_x86_elf_gc_mark_hook (asection *, struct bfd_link_info *, Elf_Internal_Rela *, struct elf_link_hash_entry *, Elf_Internal_Sym *); +extern bfd_boolean _bfd_x86_elf_gc_mark_extra_sections + (struct bfd_link_info *, elf_gc_mark_hook_fn); + extern long _bfd_x86_elf_get_synthetic_symtab (bfd *, long, long, bfd_vma, struct elf_x86_plt [], asymbol **, asymbol **); @@ -737,6 +740,8 @@ extern void _bfd_x86_elf_link_fixup_ifunc_symbol _bfd_x86_elf_adjust_dynamic_symbol #define elf_backend_gc_mark_hook \ _bfd_x86_elf_gc_mark_hook +#define elf_backend_gc_mark_extra_sections \ + _bfd_x86_elf_gc_mark_extra_sections #define elf_backend_omit_section_dynsym \ _bfd_elf_omit_section_dynsym_all #define elf_backend_parse_gnu_properties \ diff --git a/ld/testsuite/ld-i386/i386.exp b/ld/testsuite/ld-i386/i386.exp index 7bb3636d3b0..e13f23cdc67 100644 --- a/ld/testsuite/ld-i386/i386.exp +++ b/ld/testsuite/ld-i386/i386.exp @@ -496,6 +496,10 @@ run_dump_test "pr23930" run_dump_test "pr24322a" run_dump_test "pr24322b" run_dump_test "align-branch-1" +run_dump_test "pr25490-1" +run_dump_test "pr25490-2a" +run_dump_test "pr25490-2b" +run_dump_test "pr25490-3" if { !([istarget "i?86-*-linux*"] || [istarget "i?86-*-gnu*"] diff --git a/ld/testsuite/ld-i386/pr25490-1.d b/ld/testsuite/ld-i386/pr25490-1.d new file mode 100644 index 00000000000..67cc5e86ec2 --- /dev/null +++ b/ld/testsuite/ld-i386/pr25490-1.d @@ -0,0 +1,8 @@ +#source: ../ld-x86-64/pr25490-1.s +#as: --32 +#ld: --gc-sections -melf_i386 +#readelf: -SW + +#... + +\[ *[0-9]+\] __patchable_function_entries +PROGBITS +[0-9a-f]+ +[0-9a-f]+000 +0+4 +00 +WA +0 +0 +1 +#pass diff --git a/ld/testsuite/ld-i386/pr25490-2a.d b/ld/testsuite/ld-i386/pr25490-2a.d new file mode 100644 index 00000000000..5096252829f --- /dev/null +++ b/ld/testsuite/ld-i386/pr25490-2a.d @@ -0,0 +1,8 @@ +#source: ../ld-x86-64/pr25490-2.s +#as: --32 +#ld: --gc-sections -melf_i386 +#readelf: -SW + +#... + +\[ *[0-9]+\] __patchable_function_entries._start +PROGBITS +[0-9a-f]+ +[0-9a-f]+000 +0+4 +00 +WA +0 +0 +1 +#pass diff --git a/ld/testsuite/ld-i386/pr25490-2b.d b/ld/testsuite/ld-i386/pr25490-2b.d new file mode 100644 index 00000000000..3f9cab548be --- /dev/null +++ b/ld/testsuite/ld-i386/pr25490-2b.d @@ -0,0 +1,9 @@ +#source: ../ld-x86-64/pr25490-2.s +#as: --32 +#ld: --gc-sections -melf_i386 +#readelf: -SW + +#failif +#... + +\[ *[0-9]+\] __patchable_function_entries.bar +PROGBITS +.* +#pass diff --git a/ld/testsuite/ld-i386/pr25490-3.d b/ld/testsuite/ld-i386/pr25490-3.d new file mode 100644 index 00000000000..4da1f85d630 --- /dev/null +++ b/ld/testsuite/ld-i386/pr25490-3.d @@ -0,0 +1,4 @@ +#source: ../ld-x86-64/pr25490-3.s +#as: --32 +#ld: --gc-sections -melf_i386 +#error: .*\(__patchable_function_entries\): reference to garbage collected section diff --git a/ld/testsuite/ld-x86-64/pr25490-1-x32.d b/ld/testsuite/ld-x86-64/pr25490-1-x32.d new file mode 100644 index 00000000000..65bc2bee28c --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-1-x32.d @@ -0,0 +1,8 @@ +#source: pr25490-1.s +#as: --x32 +#ld: --gc-sections -melf32_x86_64 +#readelf: -SW + +#... + +\[ *[0-9]+\] __patchable_function_entries +PROGBITS +[0-9a-f]+ +[0-9a-f]+000 +0+4 +00 +WA +0 +0 +1 +#pass diff --git a/ld/testsuite/ld-x86-64/pr25490-1.d b/ld/testsuite/ld-x86-64/pr25490-1.d new file mode 100644 index 00000000000..dc1d912076b --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-1.d @@ -0,0 +1,7 @@ +#as: --64 +#ld: --gc-sections -melf_x86_64 +#readelf: -SW + +#... + +\[ *[0-9]+\] __patchable_function_entries +PROGBITS +[0-9a-f]+ +[0-9a-f]+000 +0+8 +00 +WA +0 +0 +1 +#pass diff --git a/ld/testsuite/ld-x86-64/pr25490-1.s b/ld/testsuite/ld-x86-64/pr25490-1.s new file mode 100644 index 00000000000..21123ad8aa0 --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-1.s @@ -0,0 +1,13 @@ + .text + .p2align 4 + .globl _start + .type _start, %function +_start: + .section __patchable_function_entries,"aw",%progbits + .dc.a .LPFE1 + .text +.LPFE1: + nop + ret + .size _start, .-_start + .section .note.GNU-stack,"",%progbits diff --git a/ld/testsuite/ld-x86-64/pr25490-2.s b/ld/testsuite/ld-x86-64/pr25490-2.s new file mode 100644 index 00000000000..1f8df870316 --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-2.s @@ -0,0 +1,26 @@ + .text + .p2align 4 + .section .text.bar,"ax",%progbits + .globl bar + .type bar, %function +bar: + .section __patchable_function_entries.bar,"aw",%progbits + .dc.a .LPFE1 + .section .text.bar,"ax",%progbits +.LPFE1: + nop + ret + .size bar, .-bar + .p2align 4 + .section .text._start,"ax",%progbits + .globl _start + .type _start, %function +_start: + .section __patchable_function_entries._start,"aw",%progbits + .dc.a .LPFE2 + .section .text._start,"ax",%progbits +.LPFE2: + nop + ret + .size _start, .-_start + .section .note.GNU-stack,"",%progbits diff --git a/ld/testsuite/ld-x86-64/pr25490-2a-x32.d b/ld/testsuite/ld-x86-64/pr25490-2a-x32.d new file mode 100644 index 00000000000..2029029a0f3 --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-2a-x32.d @@ -0,0 +1,8 @@ +#source: pr25490-2.s +#as: --x32 +#ld: --gc-sections -melf32_x86_64 +#readelf: -SW + +#... + +\[ *[0-9]+\] __patchable_function_entries._start +PROGBITS +[0-9a-f]+ +[0-9a-f]+000 +0+4 +00 +WA +0 +0 +1 +#pass diff --git a/ld/testsuite/ld-x86-64/pr25490-2a.d b/ld/testsuite/ld-x86-64/pr25490-2a.d new file mode 100644 index 00000000000..a5f0b141c87 --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-2a.d @@ -0,0 +1,8 @@ +#source: pr25490-2.s +#as: --64 +#ld: --gc-sections -melf_x86_64 +#readelf: -SW + +#... + +\[ *[0-9]+\] __patchable_function_entries._start +PROGBITS +[0-9a-f]+ +[0-9a-f]+000 +0+8 +00 +WA +0 +0 +1 +#pass diff --git a/ld/testsuite/ld-x86-64/pr25490-2b-x32.d b/ld/testsuite/ld-x86-64/pr25490-2b-x32.d new file mode 100644 index 00000000000..a9fe6ba6b3c --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-2b-x32.d @@ -0,0 +1,9 @@ +#source: pr25490-2.s +#as: --x32 +#ld: --gc-sections -melf32_x86_64 +#readelf: -SW + +#failif +#... + +\[ *[0-9]+\] __patchable_function_entries.bar +PROGBITS +.* +#pass diff --git a/ld/testsuite/ld-x86-64/pr25490-2b.d b/ld/testsuite/ld-x86-64/pr25490-2b.d new file mode 100644 index 00000000000..5eb933f5b36 --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-2b.d @@ -0,0 +1,9 @@ +#source: pr25490-2.s +#as: --64 +#ld: --gc-sections -melf_x86_64 +#readelf: -SW + +#failif +#... + +\[ *[0-9]+\] __patchable_function_entries.bar +PROGBITS +.* +#pass diff --git a/ld/testsuite/ld-x86-64/pr25490-3-x32.d b/ld/testsuite/ld-x86-64/pr25490-3-x32.d new file mode 100644 index 00000000000..4979a3e7978 --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-3-x32.d @@ -0,0 +1,4 @@ +#source: pr25490-3.s +#as: --x32 +#ld: --gc-sections -melf32_x86_64 +#error: .*\(__patchable_function_entries\): reference to garbage collected section diff --git a/ld/testsuite/ld-x86-64/pr25490-3.d b/ld/testsuite/ld-x86-64/pr25490-3.d new file mode 100644 index 00000000000..27a11480b60 --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-3.d @@ -0,0 +1,3 @@ +#as: --64 +#ld: --gc-sections -melf_x86_64 +#error: .*\(__patchable_function_entries\): reference to garbage collected section diff --git a/ld/testsuite/ld-x86-64/pr25490-3.s b/ld/testsuite/ld-x86-64/pr25490-3.s new file mode 100644 index 00000000000..c7fae78314c --- /dev/null +++ b/ld/testsuite/ld-x86-64/pr25490-3.s @@ -0,0 +1,28 @@ + .text + .section .text.bar,"ax",%progbits + .p2align 4 + .globl bar + .type bar, %function +bar: + .section __patchable_function_entries,"aw",%progbits + .align 8 + .dc.a .LPFE1 + .section .text.bar +.LPFE1: + nop + ret + .size bar, .-bar + .section .text._start,"ax",%progbits + .p2align 4 + .globl _start + .type _start, %function +_start: + .section __patchable_function_entries + .align 8 + .dc.a .LPFE2 + .section .text._start +.LPFE2: + nop + ret + .size _start, .-_start + .section .note.GNU-stack,"",%progbits diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp index c78b0fd7576..48912ace1c1 100644 --- a/ld/testsuite/ld-x86-64/x86-64.exp +++ b/ld/testsuite/ld-x86-64/x86-64.exp @@ -467,6 +467,14 @@ run_dump_test "pr25416-2a" run_dump_test "pr25416-2b" run_dump_test "pr25416-3" run_dump_test "pr25416-4" +run_dump_test "pr25490-1" +run_dump_test "pr25490-1-x32" +run_dump_test "pr25490-2a" +run_dump_test "pr25490-2a-x32" +run_dump_test "pr25490-2b" +run_dump_test "pr25490-2b-x32" +run_dump_test "pr25490-3" +run_dump_test "pr25490-3-x32" if { ![istarget "x86_64-*-linux*"] && ![istarget "x86_64-*-nacl*"]} { return -- 2.24.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] x86: Keep __patchable_function_entries sections with --gc-sections 2020-02-01 16:26 [PATCH] x86: Keep __patchable_function_entries sections with --gc-sections H.J. Lu 2020-02-01 17:19 ` H.J. Lu @ 2020-02-01 17:34 ` Fangrui Song 2020-02-01 17:43 ` H.J. Lu 1 sibling, 1 reply; 10+ messages in thread From: Fangrui Song @ 2020-02-01 17:34 UTC (permalink / raw) To: H.J. Lu; +Cc: binutils On 2020-02-01, H.J. Lu wrote: >After all text sections have been garbage collected, if a >__patchable_function_entries section references a section which >wasn't marked, mark it with SEC_EXCLUDE and return NULL. Otherwise, >keep it. > >Should it be handled in _bfd_elf_gc_mark_extra_sections? Thanks for paying attention to these feature requests. I referenced GNU as and ld requests at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93492#c2 If we * implement SHF_LINK_ORDER * allow multiple sections with the same name ("unique") * teach GCC to use SHF_LINK_ORDER and "unique" (see https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html) An ad-hoc gc marking will be unnecessary. SHF_LINK_ORDER has been used in a few sanitizers. Now we know __patchable_function_entries can benefit from it. In the future, they may be instances. We really need a general solution. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] x86: Keep __patchable_function_entries sections with --gc-sections 2020-02-01 17:34 ` Fangrui Song @ 2020-02-01 17:43 ` H.J. Lu 2020-02-01 18:21 ` Fangrui Song 0 siblings, 1 reply; 10+ messages in thread From: H.J. Lu @ 2020-02-01 17:43 UTC (permalink / raw) To: Fangrui Song; +Cc: Binutils On Sat, Feb 1, 2020 at 9:34 AM Fangrui Song <i@maskray.me> wrote: > > On 2020-02-01, H.J. Lu wrote: > >After all text sections have been garbage collected, if a > >__patchable_function_entries section references a section which > >wasn't marked, mark it with SEC_EXCLUDE and return NULL. Otherwise, > >keep it. > > > >Should it be handled in _bfd_elf_gc_mark_extra_sections? > > Thanks for paying attention to these feature requests. > > I referenced GNU as and ld requests at > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93492#c2 > If we > > * implement SHF_LINK_ORDER I am not sure if overloading (abusing?) SHF_LINK_ORDER is a good idea. > * allow multiple sections with the same name ("unique") This is orthogonal to this. I have a question on assembly syntax: https://sourceware.org/bugzilla/show_bug.cgi?id=25380#c1 > * teach GCC to use SHF_LINK_ORDER and "unique" (see https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html) > > An ad-hoc gc marking will be unnecessary. We need to scan relocations in _patchable_function_entries section for references to garbage collected sections. We can either check section name or a SHF_XXX. But I don't know if SHF_LINK_ORDER is a good approach. > SHF_LINK_ORDER has been used in a few sanitizers. Now we know > __patchable_function_entries can benefit from it. In the future, they > may be instances. We really need a general solution. -- H.J. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] x86: Keep __patchable_function_entries sections with --gc-sections 2020-02-01 17:43 ` H.J. Lu @ 2020-02-01 18:21 ` Fangrui Song 2020-02-02 23:44 ` [PATCH] Issue an error for GC on __patchable_function_entries section H.J. Lu 0 siblings, 1 reply; 10+ messages in thread From: Fangrui Song @ 2020-02-01 18:21 UTC (permalink / raw) To: H.J. Lu; +Cc: Binutils On 2020-02-01, H.J. Lu wrote: >On Sat, Feb 1, 2020 at 9:34 AM Fangrui Song <i@maskray.me> wrote: >> >> On 2020-02-01, H.J. Lu wrote: >> >After all text sections have been garbage collected, if a >> >__patchable_function_entries section references a section which >> >wasn't marked, mark it with SEC_EXCLUDE and return NULL. Otherwise, >> >keep it. >> > >> >Should it be handled in _bfd_elf_gc_mark_extra_sections? >> >> Thanks for paying attention to these feature requests. >> >> I referenced GNU as and ld requests at >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93492#c2 >> If we >> >> * implement SHF_LINK_ORDER > >I am not sure if overloading (abusing?) SHF_LINK_ORDER is a good idea. It is an extension, but it is agreed by multiple parties on https://groups.google.com/d/msg/generic-abi/_CbBM6T6WeM/eGF9A0AnAAAJ , including HP-UX and Solaris developers. >> * allow multiple sections with the same name ("unique") > >This is orthogonal to this. I have a question on assembly syntax: > >https://sourceware.org/bugzilla/show_bug.cgi?id=25380#c1 > >> * teach GCC to use SHF_LINK_ORDER and "unique" (see https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html) >> >> An ad-hoc gc marking will be unnecessary. > >We need to scan relocations in _patchable_function_entries section for >references to garbage collected sections. We can either check section >name or a SHF_XXX. But I don't know if SHF_LINK_ORDER is a good >approach. We don't need to if we use multiple __patchable_function_entries sections. Multiple sections are a must due to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93195#c1 (COMDAT) > 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. The __patchable_function_entries creation logic I implemented in clang: foreach function foo find the first function label defined in foo's section, name it $associated ($associated can have 2 reasonable values, w/ or w/o -ffunction-sections) get or create an id for $associated, say, $unique if foo is in a COMDAT named $comdat .section __patchable_function_entries,"awo",@progbits,$associated,comdat,$comdat,unique,$unique else .section __patchable_function_entries,"awo",@progbits,$associated,unique,$unique This approach uses feature requests I referenced in *direct* links of https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html plus https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93492#c2 , and addresses all bugs I filed. >> SHF_LINK_ORDER has been used in a few sanitizers. Now we know >> __patchable_function_entries can benefit from it. In the future, they >> may be instances. We really need a general solution. > > >-- >H.J. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] Issue an error for GC on __patchable_function_entries section 2020-02-01 18:21 ` Fangrui Song @ 2020-02-02 23:44 ` H.J. Lu 2020-02-03 0:57 ` Fangrui Song 0 siblings, 1 reply; 10+ messages in thread From: H.J. Lu @ 2020-02-02 23:44 UTC (permalink / raw) To: Fangrui Song; +Cc: Binutils [-- Attachment #1: Type: text/plain, Size: 2964 bytes --] On Sat, Feb 1, 2020 at 10:21 AM Fangrui Song <i@maskray.me> wrote: > > On 2020-02-01, H.J. Lu wrote: > >On Sat, Feb 1, 2020 at 9:34 AM Fangrui Song <i@maskray.me> wrote: > >> > >> On 2020-02-01, H.J. Lu wrote: > >> >After all text sections have been garbage collected, if a > >> >__patchable_function_entries section references a section which > >> >wasn't marked, mark it with SEC_EXCLUDE and return NULL. Otherwise, > >> >keep it. > >> > > >> >Should it be handled in _bfd_elf_gc_mark_extra_sections? > >> > >> Thanks for paying attention to these feature requests. > >> > >> I referenced GNU as and ld requests at > >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93492#c2 > >> If we > >> > >> * implement SHF_LINK_ORDER > > > >I am not sure if overloading (abusing?) SHF_LINK_ORDER is a good idea. > > It is an extension, but it is agreed by multiple parties on > https://groups.google.com/d/msg/generic-abi/_CbBM6T6WeM/eGF9A0AnAAAJ , > including HP-UX and Solaris developers. > > >> * allow multiple sections with the same name ("unique") > > > >This is orthogonal to this. I have a question on assembly syntax: > > > >https://sourceware.org/bugzilla/show_bug.cgi?id=25380#c1 > > > >> * teach GCC to use SHF_LINK_ORDER and "unique" (see https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html) > >> > >> An ad-hoc gc marking will be unnecessary. > > > >We need to scan relocations in _patchable_function_entries section for > >references to garbage collected sections. We can either check section > >name or a SHF_XXX. But I don't know if SHF_LINK_ORDER is a good > >approach. > > We don't need to if we use multiple __patchable_function_entries > sections. Multiple sections are a must due to > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93195#c1 (COMDAT) > > > 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. > > The __patchable_function_entries creation logic I implemented in clang: > > foreach function foo > find the first function label defined in foo's section, name it $associated > ($associated can have 2 reasonable values, w/ or w/o -ffunction-sections) > get or create an id for $associated, say, $unique > > if foo is in a COMDAT named $comdat > .section __patchable_function_entries,"awo",@progbits,$associated,comdat,$comdat,unique,$unique > else > .section __patchable_function_entries,"awo",@progbits,$associated,unique,$unique > > This approach uses feature requests I referenced in *direct* links of > https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html plus > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93492#c2 , > and addresses all bugs I filed. > Here is a linker patch to issue an error to avoid corrupt linker output. -- H.J. [-- Attachment #2: 0001-Issue-an-error-for-GC-on-__patchable_function_entrie.patch --] [-- Type: application/x-patch, Size: 2849 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Issue an error for GC on __patchable_function_entries section 2020-02-02 23:44 ` [PATCH] Issue an error for GC on __patchable_function_entries section H.J. Lu @ 2020-02-03 0:57 ` Fangrui Song 2020-02-03 1:18 ` H.J. Lu 0 siblings, 1 reply; 10+ messages in thread From: Fangrui Song @ 2020-02-03 0:57 UTC (permalink / raw) To: H.J. Lu; +Cc: Binutils, Szabolcs Nagy On 2020-02-02, H.J. Lu wrote: >On Sat, Feb 1, 2020 at 10:21 AM Fangrui Song <i@maskray.me> wrote: >> >> On 2020-02-01, H.J. Lu wrote: >> >On Sat, Feb 1, 2020 at 9:34 AM Fangrui Song <i@maskray.me> wrote: >> >> >> >> On 2020-02-01, H.J. Lu wrote: >> >> >After all text sections have been garbage collected, if a >> >> >__patchable_function_entries section references a section which >> >> >wasn't marked, mark it with SEC_EXCLUDE and return NULL. Otherwise, >> >> >keep it. >> >> > >> >> >Should it be handled in _bfd_elf_gc_mark_extra_sections? >> >> >> >> Thanks for paying attention to these feature requests. >> >> >> >> I referenced GNU as and ld requests at >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93492#c2 >> >> If we >> >> >> >> * implement SHF_LINK_ORDER >> > >> >I am not sure if overloading (abusing?) SHF_LINK_ORDER is a good idea. >> >> It is an extension, but it is agreed by multiple parties on >> https://groups.google.com/d/msg/generic-abi/_CbBM6T6WeM/eGF9A0AnAAAJ , >> including HP-UX and Solaris developers. >> >> >> * allow multiple sections with the same name ("unique") >> > >> >This is orthogonal to this. I have a question on assembly syntax: >> > >> >https://sourceware.org/bugzilla/show_bug.cgi?id=25380#c1 >> > >> >> * teach GCC to use SHF_LINK_ORDER and "unique" (see https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html) >> >> >> >> An ad-hoc gc marking will be unnecessary. >> > >> >We need to scan relocations in _patchable_function_entries section for >> >references to garbage collected sections. We can either check section >> >name or a SHF_XXX. But I don't know if SHF_LINK_ORDER is a good >> >approach. >> >> We don't need to if we use multiple __patchable_function_entries >> sections. Multiple sections are a must due to >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93195#c1 (COMDAT) >> >> > 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. >> >> The __patchable_function_entries creation logic I implemented in clang: >> >> foreach function foo >> find the first function label defined in foo's section, name it $associated >> ($associated can have 2 reasonable values, w/ or w/o -ffunction-sections) >> get or create an id for $associated, say, $unique >> >> if foo is in a COMDAT named $comdat >> .section __patchable_function_entries,"awo",@progbits,$associated,comdat,$comdat,unique,$unique >> else >> .section __patchable_function_entries,"awo",@progbits,$associated,unique,$unique >> >> This approach uses feature requests I referenced in *direct* links of >> https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html plus >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93492#c2 , >> and addresses all bugs I filed. >> > >Here is a linker patch to issue an error to avoid corrupt >linker output. > >-- >H.J. A typo in the description. - .section __patchable_function_entries,"awo",%progbits,_start + .section __patchable_function_entries,"aw",%progbits GCC's output is the below (no SHF_LINK_ORDER). I don't have an opinion whether !SHF_LINK_ORDER should be worked around in GNU ld. CC Szabolcs who fixed AArch64 BTI https://gcc.gnu.org/ml/gcc-patches/2020-01/msg01942.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Issue an error for GC on __patchable_function_entries section 2020-02-03 0:57 ` Fangrui Song @ 2020-02-03 1:18 ` H.J. Lu 2020-02-07 2:23 ` [PATCH] ld: " H.J. Lu 0 siblings, 1 reply; 10+ messages in thread From: H.J. Lu @ 2020-02-03 1:18 UTC (permalink / raw) To: Fangrui Song; +Cc: Binutils, Szabolcs Nagy [-- Attachment #1: Type: text/plain, Size: 3473 bytes --] On Sun, Feb 2, 2020 at 4:57 PM Fangrui Song <i@maskray.me> wrote: > > On 2020-02-02, H.J. Lu wrote: > >On Sat, Feb 1, 2020 at 10:21 AM Fangrui Song <i@maskray.me> wrote: > >> > >> On 2020-02-01, H.J. Lu wrote: > >> >On Sat, Feb 1, 2020 at 9:34 AM Fangrui Song <i@maskray.me> wrote: > >> >> > >> >> On 2020-02-01, H.J. Lu wrote: > >> >> >After all text sections have been garbage collected, if a > >> >> >__patchable_function_entries section references a section which > >> >> >wasn't marked, mark it with SEC_EXCLUDE and return NULL. Otherwise, > >> >> >keep it. > >> >> > > >> >> >Should it be handled in _bfd_elf_gc_mark_extra_sections? > >> >> > >> >> Thanks for paying attention to these feature requests. > >> >> > >> >> I referenced GNU as and ld requests at > >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93492#c2 > >> >> If we > >> >> > >> >> * implement SHF_LINK_ORDER > >> > > >> >I am not sure if overloading (abusing?) SHF_LINK_ORDER is a good idea. > >> > >> It is an extension, but it is agreed by multiple parties on > >> https://groups.google.com/d/msg/generic-abi/_CbBM6T6WeM/eGF9A0AnAAAJ , > >> including HP-UX and Solaris developers. > >> > >> >> * allow multiple sections with the same name ("unique") > >> > > >> >This is orthogonal to this. I have a question on assembly syntax: > >> > > >> >https://sourceware.org/bugzilla/show_bug.cgi?id=25380#c1 > >> > > >> >> * teach GCC to use SHF_LINK_ORDER and "unique" (see https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html) > >> >> > >> >> An ad-hoc gc marking will be unnecessary. > >> > > >> >We need to scan relocations in _patchable_function_entries section for > >> >references to garbage collected sections. We can either check section > >> >name or a SHF_XXX. But I don't know if SHF_LINK_ORDER is a good > >> >approach. > >> > >> We don't need to if we use multiple __patchable_function_entries > >> sections. Multiple sections are a must due to > >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93195#c1 (COMDAT) > >> > >> > 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. > >> > >> The __patchable_function_entries creation logic I implemented in clang: > >> > >> foreach function foo > >> find the first function label defined in foo's section, name it $associated > >> ($associated can have 2 reasonable values, w/ or w/o -ffunction-sections) > >> get or create an id for $associated, say, $unique > >> > >> if foo is in a COMDAT named $comdat > >> .section __patchable_function_entries,"awo",@progbits,$associated,comdat,$comdat,unique,$unique > >> else > >> .section __patchable_function_entries,"awo",@progbits,$associated,unique,$unique > >> > >> This approach uses feature requests I referenced in *direct* links of > >> https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html plus > >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93492#c2 , > >> and addresses all bugs I filed. > >> > > > >Here is a linker patch to issue an error to avoid corrupt > >linker output. > > > >-- > >H.J. > > A typo in the description. > > - .section __patchable_function_entries,"awo",%progbits,_start > + .section __patchable_function_entries,"aw",%progbits > Fixed. Thanks. -- H.J. [-- Attachment #2: 0001-Issue-an-error-for-GC-on-__patchable_function_entrie.patch --] [-- Type: text/x-patch, Size: 2841 bytes --] From 8628117143d73743c6b6d5a3e47613d488858b26 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Sun, 2 Feb 2020 15:32:34 -0800 Subject: [PATCH] Issue an error for GC on __patchable_function_entries section __patchable_function_entries section is generated by a compiler with -fpatchable-function-entry=XX. The assembly code looks like this: --- .text .globl _start .type _start, %function _start: .section __patchable_function_entries,"aw",%progbits .dc.a .LPFE1 .text .LPFE1: .byte 0 --- But --gc-sections will silently remove __patchable_function_entries section and generate corrupt result. The linker bug will be fixed by implementing the 'o' section flag with linked-to section: https://sourceware.org/bugzilla/show_bug.cgi?id=25381 In the meantime, this patch disallows garbage collection on __patchable_function_entries section without linked-to section. bfd/ PR ld/25490 * elflink.c (_bfd_elf_gc_mark_extra_sections): Issue an error for garbage collection on __patchable_function_entries section without linked-to section. ld/ PR ld/25490 * testsuite/ld-elf/pr25490-1.d: New file. * testsuite/ld-elf/pr25490-1.d: Likewise. --- bfd/elflink.c | 7 +++++++ ld/testsuite/ld-elf/pr25490-1.d | 2 ++ ld/testsuite/ld-elf/pr25490-1.s | 9 +++++++++ 3 files changed, 18 insertions(+) create mode 100644 ld/testsuite/ld-elf/pr25490-1.d create mode 100644 ld/testsuite/ld-elf/pr25490-1.s diff --git a/bfd/elflink.c b/bfd/elflink.c index 5217528a79b..e4d92952aaf 100644 --- a/bfd/elflink.c +++ b/bfd/elflink.c @@ -13350,6 +13350,13 @@ _bfd_elf_gc_mark_extra_sections (struct bfd_link_info *info, && (isec->flags & SEC_DEBUGGING) && CONST_STRNEQ (isec->name, ".debug_line.")) debug_frag_seen = TRUE; + else if (strcmp (bfd_section_name (isec), + "__patchable_function_entries") == 0 + && elf_linked_to_section (isec) == NULL) + info->callbacks->einfo (_("%F%P: %pB(%pA): error: " + "need linked-to section " + "for --gc-sections\n"), + isec->owner, isec); } /* If no non-note alloc section in this file will be kept, then diff --git a/ld/testsuite/ld-elf/pr25490-1.d b/ld/testsuite/ld-elf/pr25490-1.d new file mode 100644 index 00000000000..7cc2e6aaa1c --- /dev/null +++ b/ld/testsuite/ld-elf/pr25490-1.d @@ -0,0 +1,2 @@ +#ld: --gc-sections -e _start +#error: .*\(__patchable_function_entries\): error: need linked-to section for --gc-sections diff --git a/ld/testsuite/ld-elf/pr25490-1.s b/ld/testsuite/ld-elf/pr25490-1.s new file mode 100644 index 00000000000..51ba1ea8014 --- /dev/null +++ b/ld/testsuite/ld-elf/pr25490-1.s @@ -0,0 +1,9 @@ + .text + .globl _start + .type _start, %function +_start: + .section __patchable_function_entries,"aw",%progbits + .dc.a .LPFE1 + .text +.LPFE1: + .byte 0 -- 2.24.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ld: Issue an error for GC on __patchable_function_entries section 2020-02-03 1:18 ` H.J. Lu @ 2020-02-07 2:23 ` H.J. Lu 2020-02-07 3:28 ` Alan Modra 0 siblings, 1 reply; 10+ messages in thread From: H.J. Lu @ 2020-02-07 2:23 UTC (permalink / raw) To: Fangrui Song, Binutils, Szabolcs Nagy On Sun, Feb 02, 2020 at 05:17:48PM -0800, H.J. Lu wrote: > On Sun, Feb 2, 2020 at 4:57 PM Fangrui Song <i@maskray.me> wrote: > > > > On 2020-02-02, H.J. Lu wrote: > > >On Sat, Feb 1, 2020 at 10:21 AM Fangrui Song <i@maskray.me> wrote: > > >> > > >> On 2020-02-01, H.J. Lu wrote: > > >> >On Sat, Feb 1, 2020 at 9:34 AM Fangrui Song <i@maskray.me> wrote: > > >> >> > > >> >> On 2020-02-01, H.J. Lu wrote: > > >> >> >After all text sections have been garbage collected, if a > > >> >> >__patchable_function_entries section references a section which > > >> >> >wasn't marked, mark it with SEC_EXCLUDE and return NULL. Otherwise, > > >> >> >keep it. > > >> >> > > > >> >> >Should it be handled in _bfd_elf_gc_mark_extra_sections? > > >> >> > > >> >> Thanks for paying attention to these feature requests. > > >> >> > > >> >> I referenced GNU as and ld requests at > > >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93492#c2 > > >> >> If we > > >> >> > > >> >> * implement SHF_LINK_ORDER > > >> > > > >> >I am not sure if overloading (abusing?) SHF_LINK_ORDER is a good idea. > > >> > > >> It is an extension, but it is agreed by multiple parties on > > >> https://groups.google.com/d/msg/generic-abi/_CbBM6T6WeM/eGF9A0AnAAAJ , > > >> including HP-UX and Solaris developers. > > >> > > >> >> * allow multiple sections with the same name ("unique") > > >> > > > >> >This is orthogonal to this. I have a question on assembly syntax: > > >> > > > >> >https://sourceware.org/bugzilla/show_bug.cgi?id=25380#c1 > > >> > > > >> >> * teach GCC to use SHF_LINK_ORDER and "unique" (see https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html) > > >> >> > > >> >> An ad-hoc gc marking will be unnecessary. > > >> > > > >> >We need to scan relocations in _patchable_function_entries section for > > >> >references to garbage collected sections. We can either check section > > >> >name or a SHF_XXX. But I don't know if SHF_LINK_ORDER is a good > > >> >approach. > > >> > > >> We don't need to if we use multiple __patchable_function_entries > > >> sections. Multiple sections are a must due to > > >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93195#c1 (COMDAT) > > >> > > >> > 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. > > >> > > >> The __patchable_function_entries creation logic I implemented in clang: > > >> > > >> foreach function foo > > >> find the first function label defined in foo's section, name it $associated > > >> ($associated can have 2 reasonable values, w/ or w/o -ffunction-sections) > > >> get or create an id for $associated, say, $unique > > >> > > >> if foo is in a COMDAT named $comdat > > >> .section __patchable_function_entries,"awo",@progbits,$associated,comdat,$comdat,unique,$unique > > >> else > > >> .section __patchable_function_entries,"awo",@progbits,$associated,unique,$unique > > >> > > >> This approach uses feature requests I referenced in *direct* links of > > >> https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html plus > > >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93492#c2 , > > >> and addresses all bugs I filed. > > >> > > > > > >Here is a linker patch to issue an error to avoid corrupt > > >linker output. > > > > > >-- > > >H.J. > > > > A typo in the description. > > > > - .section __patchable_function_entries,"awo",%progbits,_start > > + .section __patchable_function_entries,"aw",%progbits > > > > Fixed. > Now, gas supports the section flag 'o' in .section directive. I will submit a GCC patch to use it. Here is the updated patch to issue an error for garbage collection on __patchable_function_entries section without linked-to section. OK for master? Thanks. H.J. --- __patchable_function_entries section is generated by a compiler with -fpatchable-function-entry=XX. The assembly code looks like this: --- .text .globl _start .type _start, %function _start: .section __patchable_function_entries,"aw",%progbits .dc.a .LPFE1 .text .LPFE1: .byte 0 --- But --gc-sections will silently remove __patchable_function_entries section and generate corrupt result. This patch disallows garbage collection on __patchable_function_entries section without linked-to section. bfd/ PR ld/25490 * elflink.c (_bfd_elf_gc_mark_extra_sections): Issue an error for garbage collection on __patchable_function_entries section without linked-to section. ld/ PR ld/25490 * testsuite/ld-elf/pr25490-1.d: New file. * testsuite/ld-elf/pr25490-1.s: Likewise. --- bfd/elflink.c | 7 +++++++ ld/testsuite/ld-elf/pr25490-1.d | 3 +++ ld/testsuite/ld-elf/pr25490-1.s | 9 +++++++++ 3 files changed, 19 insertions(+) create mode 100644 ld/testsuite/ld-elf/pr25490-1.d create mode 100644 ld/testsuite/ld-elf/pr25490-1.s diff --git a/bfd/elflink.c b/bfd/elflink.c index 30a572d32de..3add9f18bd7 100644 --- a/bfd/elflink.c +++ b/bfd/elflink.c @@ -13365,6 +13365,13 @@ _bfd_elf_gc_mark_extra_sections (struct bfd_link_info *info, && (isec->flags & SEC_DEBUGGING) && CONST_STRNEQ (isec->name, ".debug_line.")) debug_frag_seen = TRUE; + else if (strcmp (bfd_section_name (isec), + "__patchable_function_entries") == 0 + && elf_linked_to_section (isec) == NULL) + info->callbacks->einfo (_("%F%P: %pB(%pA): error: " + "need linked-to section " + "for --gc-sections\n"), + isec->owner, isec); } /* If no non-note alloc section in this file will be kept, then diff --git a/ld/testsuite/ld-elf/pr25490-1.d b/ld/testsuite/ld-elf/pr25490-1.d new file mode 100644 index 00000000000..3f582645899 --- /dev/null +++ b/ld/testsuite/ld-elf/pr25490-1.d @@ -0,0 +1,3 @@ +#ld: --gc-sections -e _start +#target: [check_gc_sections_available] +#error: .*\(__patchable_function_entries\): error: need linked-to section for --gc-sections diff --git a/ld/testsuite/ld-elf/pr25490-1.s b/ld/testsuite/ld-elf/pr25490-1.s new file mode 100644 index 00000000000..51ba1ea8014 --- /dev/null +++ b/ld/testsuite/ld-elf/pr25490-1.s @@ -0,0 +1,9 @@ + .text + .globl _start + .type _start, %function +_start: + .section __patchable_function_entries,"aw",%progbits + .dc.a .LPFE1 + .text +.LPFE1: + .byte 0 -- 2.24.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ld: Issue an error for GC on __patchable_function_entries section 2020-02-07 2:23 ` [PATCH] ld: " H.J. Lu @ 2020-02-07 3:28 ` Alan Modra 0 siblings, 0 replies; 10+ messages in thread From: Alan Modra @ 2020-02-07 3:28 UTC (permalink / raw) To: H.J. Lu; +Cc: binutils On Thu, Feb 06, 2020 at 06:23:28PM -0800, H.J. Lu wrote: > bfd/ > > PR ld/25490 > * elflink.c (_bfd_elf_gc_mark_extra_sections): Issue an error > for garbage collection on __patchable_function_entries section > without linked-to section. > > ld/ > > PR ld/25490 > * testsuite/ld-elf/pr25490-1.d: New file. > * testsuite/ld-elf/pr25490-1.s: Likewise. OK. -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-02-07 3:28 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-01 16:26 [PATCH] x86: Keep __patchable_function_entries sections with --gc-sections H.J. Lu 2020-02-01 17:19 ` H.J. Lu 2020-02-01 17:34 ` Fangrui Song 2020-02-01 17:43 ` H.J. Lu 2020-02-01 18:21 ` Fangrui Song 2020-02-02 23:44 ` [PATCH] Issue an error for GC on __patchable_function_entries section H.J. Lu 2020-02-03 0:57 ` Fangrui Song 2020-02-03 1:18 ` H.J. Lu 2020-02-07 2:23 ` [PATCH] ld: " H.J. Lu 2020-02-07 3:28 ` Alan Modra
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).