On Mon, Mar 07, 2022 at 02:53:32PM +0100, Jan Beulich wrote:
> As was e.g. making necessary 4b7fd8153ddf ("x86: fold sections in final
> binaries"), arbitrary sections appearing without our linker script
> placing them explicitly can be a problem. Have the linker make us aware
> of such sections, so we would know that the script needs adjusting.
>
> To deal with the resulting warnings:
> - Retain .note.* explicitly for ELF, and discard all of them (except the
> earlier consumed .note.gnu.build-id) for PE/COFF.
> - Have explicit statements for .got, .plt, and alike and add assertions
> that they're empty. No output sections will be created for these as
> long as they remain empty (or else the assertions would cause early
> failure anyway).
> - Collect all .rela.* into a single section, with again an assertion
> added for the resulting section to be empty.
> - Extend the enumerating of .debug_* to ELF. Note that for Clang adding
> of .debug_macinfo is necessary. Amend this by its Dwarf5 counterpart,
> .debug_macro, then as well (albeit more may need adding for full
> coverage).
>
> Suggested-by: Roger Pau Monné <roger....@citrix.com>
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
LGTM, just two questions.
> ---
> v2: Don't use (NOLOAD) for ELF; it has undocumented (and unhelpful)
> behavior with GNU ld there. Also place .{sym,str,shstr}tab for LLVM
> ld.
> ---
> I would have wanted to make this generic (by putting it in
> xen/Makefile), but the option cannot be added to LDFLAGS, or else
> there'll be a flood of warnings with $(LD) -r. (Besides, adding to
> LDFLAGS would mean use of the option on every linker pass rather than
> just the last one.)
>
> Retaining of .note in xen-syms is under question. Plus if we want to
> retain all notes, the question is whether they wouldn't better go into
> .init.rodata. But .note.gnu.build-id shouldn't move there, and when
> notes are discontiguous all intermediate space will also be assigned to
> the NOTE segment, thus making the contents useless for tools going just
> by program headers.
>
> Newer Clang may require yet more .debug_* to be added. I've only played
> with versions 5 and 7 so far.
>
> Unless we would finally drop all mentioning of Stabs sections, we may
> want to extend to there what is done for Dwarf here (allowing the EFI
> conditional around the section to also go away).
>
> See also https://sourceware.org/pipermail/binutils/2022-March/119922.html.
>
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -120,6 +120,8 @@ syms-warn-dup-y := --warn-dup
> syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) :=
> syms-warn-dup-$(CONFIG_ENFORCE_UNIQUE_SYMBOLS) := --error-dup
>
> +orphan-handling-$(call ld-option,--orphan-handling=warn) +=
> --orphan-handling=warn
> +
> $(TARGET): TMP = $(@D)/.$(@F).elf32
> $(TARGET): $(TARGET)-syms $(efi-y) $(obj)/boot/mkelf32
> $(obj)/boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TMP)
> $(XEN_IMG_OFFSET) \
> @@ -146,7 +148,7 @@ $(TARGET)-syms: $(BASEDIR)/prelink.o $(o
> >$(@D)/.$(@F).1.S
> $(MAKE) $(build)=$(@D) $(@D)/.$(@F).1.o
> $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
> - $(@D)/.$(@F).1.o -o $@
> + $(orphan-handling-y) $(@D)/.$(@F).1.o -o $@
> $(NM) -pa --format=sysv $(@D)/$(@F) \
> | $(BASEDIR)/tools/symbols --all-symbols --xensyms --sysv
> --sort \
> >$(@D)/$(@F).map
> @@ -220,7 +222,7 @@ endif
> | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort
> >$(@D)/.$(@F).1s.S
> $(MAKE) $(build)=$(@D) .$(@F).1r.o .$(@F).1s.o
> $(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T $(obj)/efi.lds -N $< \
> - $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o $(note_file_option)
> -o $@
> + $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o $(orphan-handling-y)
> $(note_file_option) -o $@
> $(NM) -pa --format=sysv $(@D)/$(@F) \
> | $(BASEDIR)/tools/symbols --all-symbols --xensyms --sysv
> --sort >$(@D)/$(@F).map
> rm -f $(@D)/.$(@F).[0-9]* $(@D)/..$(@F).[0-9]*
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -12,6 +12,13 @@
> #undef __XEN_VIRT_START
> #define __XEN_VIRT_START __image_base__
> #define DECL_SECTION(x) x :
> +/*
> + * Use the NOLOAD directive, despite currently ignored by (at least) GNU ld
> + * for PE output, in order to record that we'd prefer these sections to not
> + * be loaded into memory.
> + */
> +#define DECL_DEBUG(x, a) #x ALIGN(a) (NOLOAD) : { *(x) }
> +#define DECL_DEBUG2(x, y, a) #x ALIGN(a) (NOLOAD) : { *(x) *(y) }
>
> ENTRY(efi_start)
>
> @@ -19,6 +26,8 @@ ENTRY(efi_start)
>
> #define FORMAT "elf64-x86-64"
> #define DECL_SECTION(x) #x : AT(ADDR(#x) - __XEN_VIRT_START)
> +#define DECL_DEBUG(x, a) #x 0 : { *(x) }
> +#define DECL_DEBUG2(x, y, a) #x 0 : { *(x) *(y) }
Would it be helpful to place those in a
>
> ENTRY(start_pa)
>
> @@ -179,6 +188,13 @@ SECTIONS
> #endif
> #endif
>
> +#ifndef EFI
> + /* Retain these just for the purpose of possible analysis tools. */
> + DECL_SECTION(.note) {
> + *(.note.*)
> + } PHDR(note) PHDR(text)
Wouldn't it be enough to place it in the note program header?
The buildid note is already placed in .rodata, so any remaining notes
don't need to be in a LOAD section?
> +#endif
> +
> _erodata = .;
>
> . = ALIGN(SECTION_ALIGN);
> @@ -266,6 +282,32 @@ SECTIONS
> __ctors_end = .;
> } PHDR(text)
>
> +#ifndef EFI
> + /*
> + * With --orphan-sections=warn (or =error) we need to handle certain linker
> + * generated sections. These are all expected to be empty; respective
> + * ASSERT()s can be found towards the end of this file.
> + */
> + DECL_SECTION(.got) {
> + *(.got)
> + } PHDR(text)
> + DECL_SECTION(.got.plt) {
> + *(.got.plt)
> + } PHDR(text)
> + DECL_SECTION(.igot.plt) {
> + *(.igot.plt)
> + } PHDR(text)
> + DECL_SECTION(.iplt) {
> + *(.iplt)
> + } PHDR(text)
> + DECL_SECTION(.plt) {
> + *(.plt)
> + } PHDR(text)
> + DECL_SECTION(.rela) {
> + *(.rela.*)
> + } PHDR(text)
Why do you need to explicitly place those in the text program header?
Thanks, Roger.
