* [PATCH] ld: Add -z start-stop-gc to let __start_/__stop_ not retain C identifier name sections
@ 2021-02-27 20:26 Fangrui Song
2021-02-27 23:47 ` H.J. Lu
2021-02-28 4:20 ` Alan Modra
0 siblings, 2 replies; 11+ messages in thread
From: Fangrui Song @ 2021-02-27 20:26 UTC (permalink / raw)
To: binutils, Alan Modra, Nick Clifton; +Cc: Fangrui Song
This rule was initially added in PR11133 to work around glibc. However,
it makes GC on C identifier name sections ineffective. In particular,
many modern metadata sections (e.g. clang -fsanitize-coverage and
-fprofile-generate) cannot be GCed.
Add -z start-stop-gc to drop the rule.
PR 27451
include/
* bfdlink.h (struct bfd_link_info): Add start_stop_gc.
ld/
* emultempl/elf.em: Handle -z start-stop-gc and -z nostart-stop-gc.
* ldmain.c: Initialize start_stop_gc.
* lang.c (lang_process): Handle start_stop_gc.
* testsuite/ld-gc/gc.exp: New test.
* testsuite/ld-gc/start.s: Add .weak directive.
* testsuite/ld-gc/start2.d: New test.
* ld.texi: Mention -z start-stop-gc and -z nostart-stop-gc.
* NEWS: Mention -z start-stop-gc.
---
include/bfdlink.h | 4 ++++
ld/NEWS | 3 +++
ld/emultempl/elf.em | 4 ++++
ld/ld.texi | 10 ++++++++++
ld/ldlang.c | 4 ++--
ld/ldmain.c | 1 +
ld/testsuite/ld-gc/gc.exp | 1 +
ld/testsuite/ld-gc/start-2.d | 11 +++++++++++
ld/testsuite/ld-gc/start.s | 1 +
9 files changed, 37 insertions(+), 2 deletions(-)
create mode 100644 ld/testsuite/ld-gc/start-2.d
diff --git a/include/bfdlink.h b/include/bfdlink.h
index 95728b6f031..c4222929ae5 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -662,6 +662,10 @@ struct bfd_link_info
/* May be used to set DT_GNU_FLAGS_1 for ELF. */
bfd_vma gnu_flags_1;
+ /* TRUE if __start_/__stop_ references do not retain C identifier name
+ * sections. */
+ int start_stop_gc;
+
/* May be used to set ELF visibility for __start_* / __stop_. */
unsigned int start_stop_visibility;
diff --git a/ld/NEWS b/ld/NEWS
index 4bb4ba3f466..dfb9e9ca66b 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -5,6 +5,9 @@
* Add -z report-relative-reloc to x86 ELF linker to report dynamic
relative relocations.
+* Add -z start-stop-gc to let __start_/__stop_ refeferences not retain
+ the associated C identifier name sections.
+
Changes in 2.36:
* Add libdep plugin, for linking dependencies of static libraries that
diff --git a/ld/emultempl/elf.em b/ld/emultempl/elf.em
index 5e59f3853ee..cea89e57e25 100644
--- a/ld/emultempl/elf.em
+++ b/ld/emultempl/elf.em
@@ -760,6 +760,10 @@ fragment <<EOF
{
link_info.flags_1 |= DF_1_GLOBAUDIT;
}
+ else if (CONST_STRNEQ (optarg, "start-stop-gc"))
+ link_info.start_stop_gc = TRUE;
+ else if (CONST_STRNEQ (optarg, "nostart-stop-gc"))
+ link_info.start_stop_gc = FALSE;
else if (CONST_STRNEQ (optarg, "start-stop-visibility="))
{
if (strcmp (optarg, "start-stop-visibility=default") == 0)
diff --git a/ld/ld.texi b/ld/ld.texi
index 9643818e3ed..03a4966887f 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -1453,6 +1453,16 @@ Specify a stack size for an ELF @code{PT_GNU_STACK} segment.
Specifying zero will override any default non-zero sized
@code{PT_GNU_STACK} segment creation.
+@item start-stop-gc
+@itemx nostart-stop-gc
+@cindex start-stop-gc
+@code{__start_SECNAME} and @code{__stop_SECNAME} symbol references
+retain all input sections @code{SECNAME} by default as a workaround
+for some old programs. This makes GC ineffective on such sections.
+@samp{-z start-stop-gc} drops this rule and allows @code{SECNAME} to
+be discarded. It is recommended to use the @code{SHF_GNU_RETAIN} flag
+to retain an input section, instead of relying on the special rule.
+
@item start-stop-visibility=@var{value}
@cindex visibility
@cindex ELF symbol visibility
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 5ffc8444c77..c9d764fc9ff 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -8089,7 +8089,7 @@ lang_process (void)
/* Give initial values for __start and __stop symbols, so that ELF
gc_sections will keep sections referenced by these symbols. Must
be done before lang_do_assignments below. */
- if (config.build_constructors)
+ if (config.build_constructors && !link_info.start_stop_gc)
lang_init_start_stop ();
/* PR 13683: We must rerun the assignments prior to running garbage
@@ -8161,7 +8161,7 @@ lang_process (void)
/* Defining __start/__stop symbols early for --gc-sections to work
around a glibc build problem can result in these symbols being
defined when they should not be. Fix them now. */
- if (config.build_constructors)
+ if (config.build_constructors && !link_info.start_stop_gc)
lang_undef_start_stop ();
/* Define .startof./.sizeof. symbols with preliminary values before
diff --git a/ld/ldmain.c b/ld/ldmain.c
index 5c88ee744f8..7a3c02aeaa6 100644
--- a/ld/ldmain.c
+++ b/ld/ldmain.c
@@ -357,6 +357,7 @@ main (int argc, char **argv)
#ifdef DEFAULT_NEW_DTAGS
link_info.new_dtags = DEFAULT_NEW_DTAGS;
#endif
+ link_info.start_stop_gc = FALSE;
link_info.start_stop_visibility = STV_PROTECTED;
ldfile_add_arch ("");
diff --git a/ld/testsuite/ld-gc/gc.exp b/ld/testsuite/ld-gc/gc.exp
index b3245bb915e..67489ee8d04 100644
--- a/ld/testsuite/ld-gc/gc.exp
+++ b/ld/testsuite/ld-gc/gc.exp
@@ -89,6 +89,7 @@ test_gc "Check --gc-section/-r/-u" "gcrel" $ld "-r --gc-sections -u used_func"
run_dump_test "noent"
run_dump_test "abi-note"
run_dump_test "start"
+run_dump_test "start-2"
run_dump_test "stop"
run_dump_test "pr19167"
if { [is_elf_format] } then {
diff --git a/ld/testsuite/ld-gc/start-2.d b/ld/testsuite/ld-gc/start-2.d
new file mode 100644
index 00000000000..b1f0d6ba8fc
--- /dev/null
+++ b/ld/testsuite/ld-gc/start-2.d
@@ -0,0 +1,11 @@
+#name: --gc-sections with __start_
+#source: start.s
+#ld: --gc-sections -e _start -z start-stop-gc
+#nm: -n
+#target: *-*-linux* *-*-gnu* arm*-*-uclinuxfdpiceabi
+#xfail: frv-*-* metag-*-*
+
+#failif
+#...
+[0-9a-f]+ D +__start__foo
+#...
diff --git a/ld/testsuite/ld-gc/start.s b/ld/testsuite/ld-gc/start.s
index a417a97b22c..b0084a188ae 100644
--- a/ld/testsuite/ld-gc/start.s
+++ b/ld/testsuite/ld-gc/start.s
@@ -1,5 +1,6 @@
.globl _start
_start:
+ .weak __start__foo
.dc.a __start__foo
.section _foo,"aw",%progbits
foo:
--
2.30.1.766.gb4fecdf3b7-goog
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ld: Add -z start-stop-gc to let __start_/__stop_ not retain C identifier name sections
2021-02-27 20:26 [PATCH] ld: Add -z start-stop-gc to let __start_/__stop_ not retain C identifier name sections Fangrui Song
@ 2021-02-27 23:47 ` H.J. Lu
2021-02-28 1:50 ` Fangrui Song
2021-02-28 4:20 ` Alan Modra
1 sibling, 1 reply; 11+ messages in thread
From: H.J. Lu @ 2021-02-27 23:47 UTC (permalink / raw)
To: Fangrui Song; +Cc: Binutils, Alan Modra, Nick Clifton
On Sat, Feb 27, 2021 at 12:52 PM Fangrui Song via Binutils
<binutils@sourceware.org> wrote:
>
> This rule was initially added in PR11133 to work around glibc. However,
> it makes GC on C identifier name sections ineffective. In particular,
> many modern metadata sections (e.g. clang -fsanitize-coverage and
> -fprofile-generate) cannot be GCed.
>
> Add -z start-stop-gc to drop the rule.
>
> PR 27451
> include/
> * bfdlink.h (struct bfd_link_info): Add start_stop_gc.
> ld/
> * emultempl/elf.em: Handle -z start-stop-gc and -z nostart-stop-gc.
> * ldmain.c: Initialize start_stop_gc.
> * lang.c (lang_process): Handle start_stop_gc.
> * testsuite/ld-gc/gc.exp: New test.
> * testsuite/ld-gc/start.s: Add .weak directive.
> * testsuite/ld-gc/start2.d: New test.
> * ld.texi: Mention -z start-stop-gc and -z nostart-stop-gc.
> * NEWS: Mention -z start-stop-gc.
Does this option work with static glibc executable?
--
H.J.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ld: Add -z start-stop-gc to let __start_/__stop_ not retain C identifier name sections
2021-02-27 23:47 ` H.J. Lu
@ 2021-02-28 1:50 ` Fangrui Song
2021-02-28 2:49 ` H.J. Lu
0 siblings, 1 reply; 11+ messages in thread
From: Fangrui Song @ 2021-02-28 1:50 UTC (permalink / raw)
To: H.J. Lu; +Cc: Fangrui Song, Binutils
On 2021-02-27, H.J. Lu via Binutils wrote:
>On Sat, Feb 27, 2021 at 12:52 PM Fangrui Song via Binutils
><binutils@sourceware.org> wrote:
>>
>> This rule was initially added in PR11133 to work around glibc. However,
>> it makes GC on C identifier name sections ineffective. In particular,
>> many modern metadata sections (e.g. clang -fsanitize-coverage and
>> -fprofile-generate) cannot be GCed.
>>
>> Add -z start-stop-gc to drop the rule.
>>
>> PR 27451
>> include/
>> * bfdlink.h (struct bfd_link_info): Add start_stop_gc.
>> ld/
>> * emultempl/elf.em: Handle -z start-stop-gc and -z nostart-stop-gc.
>> * ldmain.c: Initialize start_stop_gc.
>> * lang.c (lang_process): Handle start_stop_gc.
>> * testsuite/ld-gc/gc.exp: New test.
>> * testsuite/ld-gc/start.s: Add .weak directive.
>> * testsuite/ld-gc/start2.d: New test.
>> * ld.texi: Mention -z start-stop-gc and -z nostart-stop-gc.
>> * NEWS: Mention -z start-stop-gc.
>
>Does this option work with static glibc executable?
>
>--
>H.J.
Yes for a simple example.
// https://sourceware.org/bugzilla/show_bug.cgi?id=11133
#include <stdio.h>
int main() {
printf("Hello World\n");
}
# --reproduce feature request: https://sourceware.org/bugzilla/show_bug.cgi?id=26119
gcc -fuse-ld=lld hello.cc -static -Wl,--gc-sections -Wl,--reproduce=/tmp/rep.tar
cd /tmp; tar xf rep.tar; cd rep
# Delete --chroot from response.txt
% ~/Dev/binutils-gdb/Debug/ld/ld-new @response.txt --gc-sections --print-gc-sections 2> 0
% ~/Dev/binutils-gdb/Debug/ld/ld-new @response.txt --gc-sections --print-gc-sections -z start-stop-gc 2> 1
% ./a.out
Hello World
There is one section which is now GCed:
% diff 0 1
16a17
> /home/ray/Dev/binutils-gdb/Debug/ld/ld-new: removing unused section '__libc_atexit' in file 'usr/lib/x86_64-linux-gnu/libc.a(genops.o)'
I don't know what the expectation is, but if the section is required to be retained, we should use SHF_GNU_RETAIN or other GC root approach
(see
https://maskray.me/blog/2021-01-31-metadata-sections-comdat-and-shf-link-order "What if all metadata sections are discarded?")
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ld: Add -z start-stop-gc to let __start_/__stop_ not retain C identifier name sections
2021-02-28 1:50 ` Fangrui Song
@ 2021-02-28 2:49 ` H.J. Lu
0 siblings, 0 replies; 11+ messages in thread
From: H.J. Lu @ 2021-02-28 2:49 UTC (permalink / raw)
To: Fangrui Song; +Cc: Fangrui Song, Binutils
On Sat, Feb 27, 2021 at 5:50 PM Fangrui Song <i@maskray.me> wrote:
>
> On 2021-02-27, H.J. Lu via Binutils wrote:
> >On Sat, Feb 27, 2021 at 12:52 PM Fangrui Song via Binutils
> ><binutils@sourceware.org> wrote:
> >>
> >> This rule was initially added in PR11133 to work around glibc. However,
> >> it makes GC on C identifier name sections ineffective. In particular,
> >> many modern metadata sections (e.g. clang -fsanitize-coverage and
> >> -fprofile-generate) cannot be GCed.
> >>
> >> Add -z start-stop-gc to drop the rule.
> >>
> >> PR 27451
> >> include/
> >> * bfdlink.h (struct bfd_link_info): Add start_stop_gc.
> >> ld/
> >> * emultempl/elf.em: Handle -z start-stop-gc and -z nostart-stop-gc.
> >> * ldmain.c: Initialize start_stop_gc.
> >> * lang.c (lang_process): Handle start_stop_gc.
> >> * testsuite/ld-gc/gc.exp: New test.
> >> * testsuite/ld-gc/start.s: Add .weak directive.
> >> * testsuite/ld-gc/start2.d: New test.
> >> * ld.texi: Mention -z start-stop-gc and -z nostart-stop-gc.
> >> * NEWS: Mention -z start-stop-gc.
> >
> >Does this option work with static glibc executable?
> >
> >--
> >H.J.
>
> Yes for a simple example.
>
> // https://sourceware.org/bugzilla/show_bug.cgi?id=11133
> #include <stdio.h>
> int main() {
> printf("Hello World\n");
> }
>
>
> # --reproduce feature request: https://sourceware.org/bugzilla/show_bug.cgi?id=26119
> gcc -fuse-ld=lld hello.cc -static -Wl,--gc-sections -Wl,--reproduce=/tmp/rep.tar
> cd /tmp; tar xf rep.tar; cd rep
> # Delete --chroot from response.txt
> % ~/Dev/binutils-gdb/Debug/ld/ld-new @response.txt --gc-sections --print-gc-sections 2> 0
> % ~/Dev/binutils-gdb/Debug/ld/ld-new @response.txt --gc-sections --print-gc-sections -z start-stop-gc 2> 1
> % ./a.out
> Hello World
>
> There is one section which is now GCed:
>
>
> % diff 0 1
> 16a17
> > /home/ray/Dev/binutils-gdb/Debug/ld/ld-new: removing unused section '__libc_atexit' in file 'usr/lib/x86_64-linux-gnu/libc.a(genops.o)'
From:
https://sourceware.org/bugzilla/show_bug.cgi?id=11133
---
Removing __libc_atexit was responsible for this behaviour.
---
This option may not be safe for static glibc executable and other cases.
> I don't know what the expectation is, but if the section is required to be retained, we should use SHF_GNU_RETAIN or other GC root approach
> (see
> https://maskray.me/blog/2021-01-31-metadata-sections-comdat-and-shf-link-order "What if all metadata sections are discarded?")
--
H.J.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ld: Add -z start-stop-gc to let __start_/__stop_ not retain C identifier name sections
2021-02-27 20:26 [PATCH] ld: Add -z start-stop-gc to let __start_/__stop_ not retain C identifier name sections Fangrui Song
2021-02-27 23:47 ` H.J. Lu
@ 2021-02-28 4:20 ` Alan Modra
2021-02-28 5:03 ` Fangrui Song
1 sibling, 1 reply; 11+ messages in thread
From: Alan Modra @ 2021-02-28 4:20 UTC (permalink / raw)
To: Fangrui Song; +Cc: binutils, Nick Clifton
On Sat, Feb 27, 2021 at 12:26:17PM -0800, Fangrui Song wrote:
> --- a/ld/ldlang.c
> +++ b/ld/ldlang.c
> @@ -8089,7 +8089,7 @@ lang_process (void)
> /* Give initial values for __start and __stop symbols, so that ELF
> gc_sections will keep sections referenced by these symbols. Must
> be done before lang_do_assignments below. */
> - if (config.build_constructors)
> + if (config.build_constructors && !link_info.start_stop_gc)
> lang_init_start_stop ();
>
> /* PR 13683: We must rerun the assignments prior to running garbage
This will mean start/stop symbols are not defined, which is rather
more than your stated aim of changing the behaviour of garbage
collection. Not setting h->start_stop in bfd_elf_define_start_stop
would do what you want, I think. (I haven't checked..)
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ld: Add -z start-stop-gc to let __start_/__stop_ not retain C identifier name sections
2021-02-28 4:20 ` Alan Modra
@ 2021-02-28 5:03 ` Fangrui Song
2021-03-01 3:59 ` Alan Modra
2021-03-01 4:04 ` Alan Modra
0 siblings, 2 replies; 11+ messages in thread
From: Fangrui Song @ 2021-02-28 5:03 UTC (permalink / raw)
To: Alan Modra; +Cc: Fangrui Song, binutils
On 2021-02-28, Alan Modra via Binutils wrote:
>On Sat, Feb 27, 2021 at 12:26:17PM -0800, Fangrui Song wrote:
>> --- a/ld/ldlang.c
>> +++ b/ld/ldlang.c
>> @@ -8089,7 +8089,7 @@ lang_process (void)
>> /* Give initial values for __start and __stop symbols, so that ELF
>> gc_sections will keep sections referenced by these symbols. Must
>> be done before lang_do_assignments below. */
>> - if (config.build_constructors)
>> + if (config.build_constructors && !link_info.start_stop_gc)
>> lang_init_start_stop ();
>>
>> /* PR 13683: We must rerun the assignments prior to running garbage
>
>This will mean start/stop symbols are not defined, which is rather
>more than your stated aim of changing the behaviour of garbage
>collection. Not setting h->start_stop in bfd_elf_define_start_stop
>would do what you want, I think. (I haven't checked..)
This looks much more difficult than I expected.
1. The current approach does not actually make __start_ useful.
2.
Setting h->start_stop to 0 in bfd_elf_define_start_stop does not work.
There will be null pointer dereference due to h->u2.start_stop_section
misused as h->u2.vtable .
3.
I tried another approach before sending this patch.
I added an info->start_stop_gc check here:
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -13451,7 +13451,7 @@ _bfd_elf_gc_mark_rsec (struct bfd_link_info *info, asection *sec,
symbols. */
if (h->start_stop)
{
asection *s = h->u2.start_stop_section;
*start_stop = !s->gc_mark;
return s;
}
Unfortunately if there is an undefined weak "__start_xx" and no xx is retained =>
undefined symbol error due to ldlang.c:undef_start_stop .
I don't know how to distinguish undefined strong and undefined weak in undef_start_stop.
Perhaps I should hand this over to you expert:)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ld: Add -z start-stop-gc to let __start_/__stop_ not retain C identifier name sections
2021-02-28 5:03 ` Fangrui Song
@ 2021-03-01 3:59 ` Alan Modra
2021-03-01 7:03 ` Fangrui Song
2021-03-01 4:04 ` Alan Modra
1 sibling, 1 reply; 11+ messages in thread
From: Alan Modra @ 2021-03-01 3:59 UTC (permalink / raw)
To: Fangrui Song; +Cc: Fangrui Song, binutils
On Sat, Feb 27, 2021 at 09:03:19PM -0800, Fangrui Song wrote:
> Unfortunately if there is an undefined weak "__start_xx" and no xx is retained =>
> undefined symbol error due to ldlang.c:undef_start_stop .
>
> I don't know how to distinguish undefined strong and undefined weak in undef_start_stop.
OK, so let's fix that first.
If a weak reference to a __start_foo or __stop_foo symbol ends up
having no definition due to all the foo sections being removed for
some reason, undef_start_stop currently makes the symbol strong
undefined. That risks a linker undefined symbol error. Fix that by
making the symbol undefweak and also undo some dynamic symbol state.
Note that saving the state of the symbol type at the time
lang_init_start_stop runs is not sufficient. The linker may have
merged in a shared library reference by that point and made what was
an undefweak in regular objects, a strong undefined. So it is
necessary to look at the ELF symbol flags to decide whether an
undefweak is the proper resolution.
Something probably should be done for COFF/PE too, but I'm unsure how
to do go about that.
* ldlang.c (undef_start_stop): For ELF make undefined start/stop
symbols undefweak if that was how they were referenced. Undo
dynamic state too.
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 5ffc8444c7..a77e8fabef 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6808,6 +6808,19 @@ undef_start_stop (struct bfd_link_hash_entry *h)
}
h->type = bfd_link_hash_undefined;
h->u.undef.abfd = NULL;
+ if (bfd_get_flavour (link_info.output_bfd) == bfd_target_elf_flavour)
+ {
+ const struct elf_backend_data *bed;
+ struct elf_link_hash_entry *eh = (struct elf_link_hash_entry *) h;
+ unsigned int was_forced = eh->forced_local;
+
+ bed = get_elf_backend_data (link_info.output_bfd);
+ (*bed->elf_backend_hide_symbol) (&link_info, eh, TRUE);
+ if (!eh->ref_regular_nonweak)
+ h->type = bfd_link_hash_undefweak;
+ eh->def_regular = 0;
+ eh->forced_local = was_forced;
+ }
}
}
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ld: Add -z start-stop-gc to let __start_/__stop_ not retain C identifier name sections
2021-02-28 5:03 ` Fangrui Song
2021-03-01 3:59 ` Alan Modra
@ 2021-03-01 4:04 ` Alan Modra
1 sibling, 0 replies; 11+ messages in thread
From: Alan Modra @ 2021-03-01 4:04 UTC (permalink / raw)
To: Fangrui Song; +Cc: Fangrui Song, binutils
On Sat, Feb 27, 2021 at 09:03:19PM -0800, Fangrui Song wrote:
> This looks much more difficult than I expected.
>
> 1. The current approach does not actually make __start_ useful.
>
> 2.
> Setting h->start_stop to 0 in bfd_elf_define_start_stop does not work.
> There will be null pointer dereference due to h->u2.start_stop_section
> misused as h->u2.vtable .
And in any case that idea doesn't work.
> 3.
> I tried another approach before sending this patch.
> I added an info->start_stop_gc check here:
>
> --- a/bfd/elflink.c
> +++ b/bfd/elflink.c
> @@ -13451,7 +13451,7 @@ _bfd_elf_gc_mark_rsec (struct bfd_link_info *info, asection *sec,
> symbols. */
> if (h->start_stop)
> {
> asection *s = h->u2.start_stop_section;
> *start_stop = !s->gc_mark;
> return s;
> }
>
This is the correct place to make these changes. I have the following
under test at the moment.
====
When --gc-sections is in effect, a reference from a retained section
to __start_SECNAME or __stop_SECNAME causes all input sections named
SECNAME to also be retained, if SECNAME is representable as a C
identifier and either __start_SECNAME or __stop_SECNAME is synthesized
by the linker. Add an option to disable that feature, effectively
ignoring any relocation that references a synthesized linker defined
__start_ or __stop_ symbol.
PR 27451
include/
* bfdlink.h (struct bfd_link_info): Add start_stop_gc.
bfd/
* elflink.c (_bfd_elf_gc_mark_rsec): Ignore synthesized linker
defined start/stop symbols when start_stop_gc.
(bfd_elf_gc_mark_dynamic_ref_symbol): Likewise.
(bfd_elf_define_start_stop): Don't modify ldscript_def syms.
* linker.c (bfd_generic_define_start_stop): Likewise.
ld/
* emultempl/elf.em: Handle -z start-stop-gc and -z nostart-stop-gc.
* lexsup.c (elf_static_list_options): Display help for them. Move
help for -z stack-size to here from elf_shlib_list_options. Add
help for -z start-stop-visibility and -z undefs.
* ld.texi: Document -z start-stop-gc and -z nostart-stop-gc.
* NEWS: Mention -z start-stop-gc.
* testsuite/ld-gc/start2.s,
* testsuite/ld-gc/start2.d: New test.
* testsuite/ld-gc/gc.exp: Run it.
diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 2fef817d73..818f580e35 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,12 @@
+2021-03-01 Alan Modra <amodra@gmail.com>
+ Fangrui Song <maskray@google.com>
+
+ * elflink.c (_bfd_elf_gc_mark_rsec): Ignore synthesized linker
+ defined start/stop symbols when start_stop_gc.
+ (bfd_elf_gc_mark_dynamic_ref_symbol): Likewise.
+ (bfd_elf_define_start_stop): Don't modify ldscript_def syms.
+ * linker.c (bfd_generic_define_start_stop): Likewise.
+
2021-02-25 Alan Modra <amodra@gmail.com>
PR 27441
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 7b74f2653c..74b54c2c0c 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -13444,12 +13444,15 @@ _bfd_elf_gc_mark_rsec (struct bfd_link_info *info, asection *sec,
hw->mark = 1;
}
- if (start_stop != NULL)
+ if (h->start_stop && !h->root.ldscript_def)
{
+ if (info->start_stop_gc)
+ return NULL;
+
/* To work around a glibc bug, mark XXX input sections
when there is a reference to __start_XXX or __stop_XXX
symbols. */
- if (h->start_stop)
+ else if (start_stop != NULL)
{
asection *s = h->u2.start_stop_section;
*start_stop = !s->gc_mark;
@@ -13912,6 +13915,9 @@ bfd_elf_gc_mark_dynamic_ref_symbol (struct elf_link_hash_entry *h, void *inf)
if ((h->root.type == bfd_link_hash_defined
|| h->root.type == bfd_link_hash_defweak)
+ && (!h->start_stop
+ || h->root.ldscript_def
+ || !info->start_stop_gc)
&& ((h->ref_dynamic && !h->forced_local)
|| ((h->def_regular || ELF_COMMON_DEF_P (h))
&& ELF_ST_VISIBILITY (h->other) != STV_INTERNAL
@@ -14984,6 +14990,7 @@ bfd_elf_define_start_stop (struct bfd_link_info *info,
FALSE, FALSE, TRUE);
/* NB: Common symbols will be turned into definition later. */
if (h != NULL
+ && !h->root.ldscript_def
&& (h->root.type == bfd_link_hash_undefined
|| h->root.type == bfd_link_hash_undefweak
|| ((h->ref_regular || h->def_dynamic)
diff --git a/bfd/linker.c b/bfd/linker.c
index 1fb57876c3..7e0415cbed 100644
--- a/bfd/linker.c
+++ b/bfd/linker.c
@@ -3188,6 +3188,7 @@ bfd_generic_define_start_stop (struct bfd_link_info *info,
h = bfd_link_hash_lookup (info->hash, symbol, FALSE, FALSE, TRUE);
if (h != NULL
+ && !h->ldscript_def
&& (h->type == bfd_link_hash_undefined
|| h->type == bfd_link_hash_undefweak))
{
diff --git a/include/ChangeLog b/include/ChangeLog
index 616b923b53..16c14d9b7a 100644
--- a/include/ChangeLog
+++ b/include/ChangeLog
@@ -1,3 +1,8 @@
+2021-03-01 Alan Modra <amodra@gmail.com>
+ Fangrui Song <maskray@google.com>
+
+ * bfdlink.h (struct bfd_link_info): Add start_stop_gc.
+
2021-02-21 Alan Modra <amodra@gmail.com>
* bfdlink.h (struct bfd_link_info): Add warn_multiple_definition.
diff --git a/include/bfdlink.h b/include/bfdlink.h
index 95728b6f03..0871a0c025 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -662,6 +662,10 @@ struct bfd_link_info
/* May be used to set DT_GNU_FLAGS_1 for ELF. */
bfd_vma gnu_flags_1;
+ /* TRUE if references to __start_/__stop_ synthesized symbols do not
+ specially retain C identifier named sections. */
+ int start_stop_gc;
+
/* May be used to set ELF visibility for __start_* / __stop_. */
unsigned int start_stop_visibility;
diff --git a/ld/ChangeLog b/ld/ChangeLog
index 2347f9ae19..d72d69af12 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,16 @@
+2021-03-01 Alan Modra <amodra@gmail.com>
+ Fangrui Song <maskray@google.com>
+
+ * emultempl/elf.em: Handle -z start-stop-gc and -z nostart-stop-gc.
+ * lexsup.c (elf_static_list_options): Display help for them. Move
+ help for -z stack-size to here from elf_shlib_list_options. Add
+ help for -z start-stop-visibility and -z undefs.
+ * ld.texi: Document -z start-stop-gc and -z nostart-stop-gc.
+ * NEWS: Mention -z start-stop-gc.
+ * testsuite/ld-gc/start2.s,
+ * testsuite/ld-gc/start2.d: New test.
+ * testsuite/ld-gc/gc.exp: Run it.
+
2021-03-01 Alan Modra <amodra@gmail.com>
* ldlang.c (undef_start_stop): For ELF make undefined start/stop
diff --git a/ld/NEWS b/ld/NEWS
index 4bb4ba3f46..d1aa3b0700 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -5,6 +5,9 @@
* Add -z report-relative-reloc to x86 ELF linker to report dynamic
relative relocations.
+* Add -z start-stop-gc to disable special treatment of __start_*/__stop_*
+ references when --gc-sections.
+
Changes in 2.36:
* Add libdep plugin, for linking dependencies of static libraries that
diff --git a/ld/emultempl/elf.em b/ld/emultempl/elf.em
index 5e59f3853e..cea89e57e2 100644
--- a/ld/emultempl/elf.em
+++ b/ld/emultempl/elf.em
@@ -760,6 +760,10 @@ fragment <<EOF
{
link_info.flags_1 |= DF_1_GLOBAUDIT;
}
+ else if (CONST_STRNEQ (optarg, "start-stop-gc"))
+ link_info.start_stop_gc = TRUE;
+ else if (CONST_STRNEQ (optarg, "nostart-stop-gc"))
+ link_info.start_stop_gc = FALSE;
else if (CONST_STRNEQ (optarg, "start-stop-visibility="))
{
if (strcmp (optarg, "start-stop-visibility=default") == 0)
diff --git a/ld/ld.texi b/ld/ld.texi
index 9643818e3e..6d016ecc34 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -1453,6 +1453,23 @@ Specify a stack size for an ELF @code{PT_GNU_STACK} segment.
Specifying zero will override any default non-zero sized
@code{PT_GNU_STACK} segment creation.
+@item start-stop-gc
+@itemx nostart-stop-gc
+@cindex start-stop-gc
+When @samp{--gc-sections} is in effect, a reference from a retained
+section to @code{__start_SECNAME} or @code{__stop_SECNAME} causes all
+input sections named @code{SECNAME} to also be retained, if
+@code{SECNAME} is representable as a C identifier and either
+@code{__start_SECNAME} or @code{__stop_SECNAME} is synthesized by the
+linker. @samp{-z start-stop-gc} disables this effect, allowing
+sections to be garbage collected as if the special synthesized symbols
+were not defined. @samp{-z start-stop-gc} has no effect on a
+definition of @code{__start_SECNAME} or @code{__stop_SECNAME} in an
+object file or linker script. Such a definition will prevent the
+linker providing a synthesized @code{__start_SECNAME} or
+@code{__stop_SECNAME} respectively, and therefore the special
+treatment by garbage collection for those references.
+
@item start-stop-visibility=@var{value}
@cindex visibility
@cindex ELF symbol visibility
diff --git a/ld/ldmain.c b/ld/ldmain.c
index 5c88ee744f..7a3c02aeaa 100644
--- a/ld/ldmain.c
+++ b/ld/ldmain.c
@@ -357,6 +357,7 @@ main (int argc, char **argv)
#ifdef DEFAULT_NEW_DTAGS
link_info.new_dtags = DEFAULT_NEW_DTAGS;
#endif
+ link_info.start_stop_gc = FALSE;
link_info.start_stop_visibility = STV_PROTECTED;
ldfile_add_arch ("");
diff --git a/ld/lexsup.c b/ld/lexsup.c
index f005a58a04..306ba01ae6 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -2070,8 +2070,6 @@ elf_shlib_list_options (FILE *file)
-z common Generate common symbols with STT_COMMON type\n"));
fprintf (file, _("\
-z nocommon Generate common symbols with STT_OBJECT type\n"));
- fprintf (file, _("\
- -z stack-size=SIZE Set size of stack segment\n"));
if (link_info.textrel_check == textrel_check_error)
fprintf (file, _("\
-z text Treat DT_TEXTREL in output as error (default)\n"));
@@ -2116,8 +2114,12 @@ elf_static_list_options (FILE *file)
fprintf (file, _("\
-z defs Report unresolved symbols in object files\n"));
fprintf (file, _("\
+ -z undefs Ignore unresolved symbols in object files\n"));
+ fprintf (file, _("\
-z muldefs Allow multiple definitions\n"));
fprintf (file, _("\
+ -z stack-size=SIZE Set size of stack segment\n"));
+ fprintf (file, _("\
-z execstack Mark executable as requiring executable stack\n"));
fprintf (file, _("\
-z noexecstack Mark executable as not requiring executable stack\n"));
@@ -2127,6 +2129,13 @@ elf_static_list_options (FILE *file)
-z nounique-symbol Keep duplicated local symbol names (default)\n"));
fprintf (file, _("\
-z globalaudit Mark executable requiring global auditing\n"));
+ fprintf (file, _("\
+ -z start-stop-gc Enable garbage collection on __start/__stop\n"));
+ fprintf (file, _("\
+ -z nostart-stop-gc Don't garbage collect __start/__stop (default)\n"));
+ fprintf (file, _("\
+ -z start-stop-visibility=V Set visibility of built-in __start/__stop symbols\n\"
+ to DEFAULT, PROTECTED, HIDDEN or INTERNAL\n"));
}
static void
diff --git a/ld/testsuite/ld-gc/gc.exp b/ld/testsuite/ld-gc/gc.exp
index b3245bb915..ad3bc2e641 100644
--- a/ld/testsuite/ld-gc/gc.exp
+++ b/ld/testsuite/ld-gc/gc.exp
@@ -89,6 +89,7 @@ test_gc "Check --gc-section/-r/-u" "gcrel" $ld "-r --gc-sections -u used_func"
run_dump_test "noent"
run_dump_test "abi-note"
run_dump_test "start"
+run_dump_test "start2"
run_dump_test "stop"
run_dump_test "pr19167"
if { [is_elf_format] } then {
diff --git a/ld/testsuite/ld-gc/start2.d b/ld/testsuite/ld-gc/start2.d
new file mode 100644
index 0000000000..325082e577
--- /dev/null
+++ b/ld/testsuite/ld-gc/start2.d
@@ -0,0 +1,10 @@
+#name: --gc-sections with -z start-stop-gc
+#ld: --gc-sections -e _start -z start-stop-gc
+#nm: -n
+#target: *-*-linux* *-*-gnu* arm*-*-uclinuxfdpiceabi
+#xfail: frv-*-* metag-*-*
+
+#failif
+#...
+[0-9a-f]+ D +__start__foo
+#...
diff --git a/ld/testsuite/ld-gc/start2.s b/ld/testsuite/ld-gc/start2.s
new file mode 100644
index 0000000000..b0084a188a
--- /dev/null
+++ b/ld/testsuite/ld-gc/start2.s
@@ -0,0 +1,7 @@
+.globl _start
+_start:
+ .weak __start__foo
+ .dc.a __start__foo
+ .section _foo,"aw",%progbits
+foo:
+ .long 1
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ld: Add -z start-stop-gc to let __start_/__stop_ not retain C identifier name sections
2021-03-01 3:59 ` Alan Modra
@ 2021-03-01 7:03 ` Fangrui Song
2021-03-01 7:28 ` Alan Modra
2021-03-01 8:08 ` Fangrui Song
0 siblings, 2 replies; 11+ messages in thread
From: Fangrui Song @ 2021-03-01 7:03 UTC (permalink / raw)
To: Alan Modra; +Cc: Fangrui Song, binutils
On 2021-03-01, Alan Modra wrote:
>On Sat, Feb 27, 2021 at 09:03:19PM -0800, Fangrui Song wrote:
>> Unfortunately if there is an undefined weak "__start_xx" and no xx is retained =>
>> undefined symbol error due to ldlang.c:undef_start_stop .
>>
>> I don't know how to distinguish undefined strong and undefined weak in undef_start_stop.
>
>OK, so let's fix that first.
>
>If a weak reference to a __start_foo or __stop_foo symbol ends up
>having no definition due to all the foo sections being removed for
>some reason, undef_start_stop currently makes the symbol strong
>undefined. That risks a linker undefined symbol error. Fix that by
>making the symbol undefweak and also undo some dynamic symbol state.
>
>Note that saving the state of the symbol type at the time
>lang_init_start_stop runs is not sufficient. The linker may have
>merged in a shared library reference by that point and made what was
>an undefweak in regular objects, a strong undefined. So it is
>necessary to look at the ELF symbol flags to decide whether an
>undefweak is the proper resolution.
>
>Something probably should be done for COFF/PE too, but I'm unsure how
>to do go about that.
>
> * ldlang.c (undef_start_stop): For ELF make undefined start/stop
> symbols undefweak if that was how they were referenced. Undo
> dynamic state too.
>
>diff --git a/ld/ldlang.c b/ld/ldlang.c
>index 5ffc8444c7..a77e8fabef 100644
>--- a/ld/ldlang.c
>+++ b/ld/ldlang.c
>@@ -6808,6 +6808,19 @@ undef_start_stop (struct bfd_link_hash_entry *h)
> }
> h->type = bfd_link_hash_undefined;
> h->u.undef.abfd = NULL;
>+ if (bfd_get_flavour (link_info.output_bfd) == bfd_target_elf_flavour)
>+ {
>+ const struct elf_backend_data *bed;
>+ struct elf_link_hash_entry *eh = (struct elf_link_hash_entry *) h;
>+ unsigned int was_forced = eh->forced_local;
>+
>+ bed = get_elf_backend_data (link_info.output_bfd);
>+ (*bed->elf_backend_hide_symbol) (&link_info, eh, TRUE);
>+ if (!eh->ref_regular_nonweak)
>+ h->type = bfd_link_hash_undefweak;
>+ eh->def_regular = 0;
>+ eh->forced_local = was_forced;
>+ }
> }
> }
Thanks for taking care of the feature:)
Here is another interesting test.
.weak __start_xx
.weak __stop_xx
.global _start
_start:
movq __start_xx@gotpcrel(%rip), %rdi
movq __stop_xx@gotpcrel(%rip), %rsi
.section xx,"a",unique,0
.byte 0
.section xx,"a",unique,1
.byte 1
.section xx,"a",unique,2
.byte 2
ld-new a.o --gc-sections --print-gc-sections -pie -z start-stop-gc can discard 3 sections.
ld-new a.o --gc-sections --print-gc-sections -shared -z start-stop-gc can only discard 2 - there may be a lurking bug somewhere.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ld: Add -z start-stop-gc to let __start_/__stop_ not retain C identifier name sections
2021-03-01 7:03 ` Fangrui Song
@ 2021-03-01 7:28 ` Alan Modra
2021-03-01 8:08 ` Fangrui Song
1 sibling, 0 replies; 11+ messages in thread
From: Alan Modra @ 2021-03-01 7:28 UTC (permalink / raw)
To: Fangrui Song; +Cc: Fangrui Song, binutils
On Sun, Feb 28, 2021 at 11:03:19PM -0800, Fangrui Song wrote:
> Here is another interesting test.
>
> .weak __start_xx
> .weak __stop_xx
>
> .global _start
> _start:
> movq __start_xx@gotpcrel(%rip), %rdi
> movq __stop_xx@gotpcrel(%rip), %rsi
>
> .section xx,"a",unique,0
> .byte 0
>
> .section xx,"a",unique,1
> .byte 1
>
> .section xx,"a",unique,2
> .byte 2
>
>
> ld-new a.o --gc-sections --print-gc-sections -pie -z start-stop-gc can discard 3 sections.
> ld-new a.o --gc-sections --print-gc-sections -shared -z start-stop-gc can only discard 2 - there may be a lurking bug somewhere.
I see
./ld-new: removing unused section 'xx' in file 'a.o'
./ld-new: removing unused section 'xx' in file 'a.o'
./ld-new: removing unused section 'xx' in file 'a.o'
./ld-new: a.o: relocation R_X86_64_PC32 against undefined protected symbol `__start_xx' can not be used when making a shared object
./ld-new: final link failed: bad value
I'm guessing that an x86 optimisation is being confused.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ld: Add -z start-stop-gc to let __start_/__stop_ not retain C identifier name sections
2021-03-01 7:03 ` Fangrui Song
2021-03-01 7:28 ` Alan Modra
@ 2021-03-01 8:08 ` Fangrui Song
1 sibling, 0 replies; 11+ messages in thread
From: Fangrui Song @ 2021-03-01 8:08 UTC (permalink / raw)
To: H.J. Lu; +Cc: Fangrui Song, binutils, Alan Modra
On 2021-02-28, Fangrui Song wrote:
>On 2021-03-01, Alan Modra wrote:
>>On Sat, Feb 27, 2021 at 09:03:19PM -0800, Fangrui Song wrote:
>>>Unfortunately if there is an undefined weak "__start_xx" and no xx is retained =>
>>>undefined symbol error due to ldlang.c:undef_start_stop .
>>>
>>>I don't know how to distinguish undefined strong and undefined weak in undef_start_stop.
>>
>>OK, so let's fix that first.
>>
>>If a weak reference to a __start_foo or __stop_foo symbol ends up
>>having no definition due to all the foo sections being removed for
>>some reason, undef_start_stop currently makes the symbol strong
>>undefined. That risks a linker undefined symbol error. Fix that by
>>making the symbol undefweak and also undo some dynamic symbol state.
>>
>>Note that saving the state of the symbol type at the time
>>lang_init_start_stop runs is not sufficient. The linker may have
>>merged in a shared library reference by that point and made what was
>>an undefweak in regular objects, a strong undefined. So it is
>>necessary to look at the ELF symbol flags to decide whether an
>>undefweak is the proper resolution.
>>
>>Something probably should be done for COFF/PE too, but I'm unsure how
>>to do go about that.
>>
>> * ldlang.c (undef_start_stop): For ELF make undefined start/stop
>> symbols undefweak if that was how they were referenced. Undo
>> dynamic state too.
>>
>>diff --git a/ld/ldlang.c b/ld/ldlang.c
>>index 5ffc8444c7..a77e8fabef 100644
>>--- a/ld/ldlang.c
>>+++ b/ld/ldlang.c
>>@@ -6808,6 +6808,19 @@ undef_start_stop (struct bfd_link_hash_entry *h)
>> }
>> h->type = bfd_link_hash_undefined;
>> h->u.undef.abfd = NULL;
>>+ if (bfd_get_flavour (link_info.output_bfd) == bfd_target_elf_flavour)
>>+ {
>>+ const struct elf_backend_data *bed;
>>+ struct elf_link_hash_entry *eh = (struct elf_link_hash_entry *) h;
>>+ unsigned int was_forced = eh->forced_local;
>>+
>>+ bed = get_elf_backend_data (link_info.output_bfd);
>>+ (*bed->elf_backend_hide_symbol) (&link_info, eh, TRUE);
>>+ if (!eh->ref_regular_nonweak)
>>+ h->type = bfd_link_hash_undefweak;
>>+ eh->def_regular = 0;
>>+ eh->forced_local = was_forced;
>>+ }
>> }
>>}
>
>Thanks for taking care of the feature:)
>
>
>Here is another interesting test.
>
>.weak __start_xx
>.weak __stop_xx
>
>.global _start
>_start:
> movq __start_xx@gotpcrel(%rip), %rdi
> movq __stop_xx@gotpcrel(%rip), %rsi
>
>.section xx,"a",unique,0
>.byte 0
>
>.section xx,"a",unique,1
>.byte 1
>
>.section xx,"a",unique,2
>.byte 2
>
>
>ld-new a.o --gc-sections --print-gc-sections -pie -z start-stop-gc can discard 3 sections.
>ld-new a.o --gc-sections --print-gc-sections -shared -z start-stop-gc can only discard 2 - there may be a lurking bug somewhere.
Alan's patch takes care of this issue.
There is still an x86 specific due to GOTPCRELX optimization. aarch64 looks good.
./ld-new: x.o: relocation R_X86_64_PC32 against undefined protected symbol `__start_xx' can not be used when making a shared object
I filed https://sourceware.org/bugzilla/show_bug.cgi?id=27491
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-03-01 8:08 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-27 20:26 [PATCH] ld: Add -z start-stop-gc to let __start_/__stop_ not retain C identifier name sections Fangrui Song
2021-02-27 23:47 ` H.J. Lu
2021-02-28 1:50 ` Fangrui Song
2021-02-28 2:49 ` H.J. Lu
2021-02-28 4:20 ` Alan Modra
2021-02-28 5:03 ` Fangrui Song
2021-03-01 3:59 ` Alan Modra
2021-03-01 7:03 ` Fangrui Song
2021-03-01 7:28 ` Alan Modra
2021-03-01 8:08 ` Fangrui Song
2021-03-01 4:04 ` 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).