Re: [PATCH v3 2/2] xen: Populate xen.lds.h and make use of its macros

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.03.2022 09:14, Michal Orzel wrote:
> --- a/xen/include/xen/xen.lds.h
> +++ b/xen/include/xen/xen.lds.h
> @@ -5,4 +5,133 @@
>   * Common macros to be used in architecture specific linker scripts.
>   */
>  
> +/*
> + * To avoid any confusion, please note that the EFI macro does not correspond
> + * to EFI support and is used when linking a native EFI (i.e. PE/COFF) 
> binary,
> + * hence its usage in this header.
> + */
> +
> +/* Macros to declare debug sections. */
> +#ifdef EFI
> +/*
> + * 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) }
> +#else
> +#define DECL_DEBUG(x, a) #x 0 : { *(x) }
> +#define DECL_DEBUG2(x, y, a) #x 0 : { *(x) *(y) }
> +#endif
> +
> +/*
> + * DWARF2+ debug sections.
> + * Explicitly list debug sections, first of all to avoid these sections being
> + * viewed as "orphan" by the linker.
> + *
> + * For the PE output this is further necessary so that they don't end up at
> + * VA 0, which is below image base and thus invalid. Note that this macro is
> + * to be used after _end, so if these sections get loaded they'll be 
> discarded
> + * at runtime anyway.
> + */
> +#define DWARF2_DEBUG_SECTIONS                     \
> +  DECL_DEBUG(.debug_abbrev, 1)                    \
> +  DECL_DEBUG2(.debug_info, .gnu.linkonce.wi.*, 1) \
> +  DECL_DEBUG(.debug_types, 1)                     \
> +  DECL_DEBUG(.debug_str, 1)                       \
> +  DECL_DEBUG2(.debug_line, .debug_line.*, 1)      \
> +  DECL_DEBUG(.debug_line_str, 1)                  \
> +  DECL_DEBUG(.debug_names, 4)                     \
> +  DECL_DEBUG(.debug_frame, 4)                     \
> +  DECL_DEBUG(.debug_loc, 1)                       \
> +  DECL_DEBUG(.debug_loclists, 4)                  \
> +  DECL_DEBUG(.debug_macinfo, 1)                   \
> +  DECL_DEBUG(.debug_macro, 1)                     \
> +  DECL_DEBUG(.debug_ranges, 8)                    \

Here and ...

> +  DECL_DEBUG(.debug_rnglists, 4)                  \
> +  DECL_DEBUG(.debug_addr, 8)                      \

... here I think you also want to switch to POINTER_ALIGN.

> +  DECL_DEBUG(.debug_aranges, 1)                   \
> +  DECL_DEBUG(.debug_pubnames, 1)                  \
> +  DECL_DEBUG(.debug_pubtypes, 1)
> +
> +/* Stabs debug sections. */
> +#define STABS_DEBUG_SECTIONS                 \
> +  .stab 0 : { *(.stab) }                     \
> +  .stabstr 0 : { *(.stabstr) }               \
> +  .stab.excl 0 : { *(.stab.excl) }           \
> +  .stab.exclstr 0 : { *(.stab.exclstr) }     \
> +  .stab.index 0 : { *(.stab.index) }         \
> +  .stab.indexstr 0 : { *(.stab.indexstr) }
> +
> +/*
> + * Required sections not related to debugging.

Nit: Perhaps better "Required ELF sections ..."? Personally I'd also
drop the mentioning of debugging - that's not really relevant here.
I'm also unsure about "Required" - .comment isn't really required.
IOW ideally simply "ELF sections" or "Sections to be retained in ELF
binaries" or some such.

Jan