* [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).