Re: [PATCH] ld: Add -z start-stop-gc to let __start_/__stop_ not retain C identifier name sections

[Fangrui Song <i@maskray.me>]

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