Re: [PATCH 7/8] x86/EFI: keep debug info in xen.efi

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Apr 21, 2021 at 03:06:36PM +0200, Jan Beulich wrote:
> On 21.04.2021 13:15, Roger Pau Monné wrote:
> > On Thu, Apr 01, 2021 at 11:47:03AM +0200, Jan Beulich wrote:
> >> ... provided the linker supports it (which it does as of commit
> >> 2dfa8341e079 ["ELF DWARF in PE output"]).
> >>
> >> Without mentioning debugging sections, the linker would put them at
> >> VA 0, thus making them unreachable by 32-bit (relative or absolute)
> >> relocations. If relocations were resolvable (or absent) the resulting
> >> binary would have invalid section RVAs (0 - __image_base__, truncated to
> >> 32 bits). Mentioning debugging sections without specifying an address
> >> will result in the linker putting them all on the same RVA. A loader is,
> >> afaict, free to reject loading such an image, as sections shouldn't
> >> overlap. (The above describes GNU ld 2.36 behavior, which - if deemed
> >> buggy - could change.)
> > 
> > Isn't just using (NOLOAD) to signal those sections shouldn't be
> > loaded enough, and thus don't care about it's RVA?
> 
> As said in a reply earlier on another sub-thread, (NOLOAD) is meaningless
> for PE. The fact that I add them nevertheless is just for docs purposes
> (or if, in the future, the item gains significance).
> 
> The main problem though isn't "load" vs "no-load" but, as I thought I
> have expressed in the description, that there's no "don't care about it's
> RVA" in PE. All sections have to have a non-zero VA above the image base.
> This is the only way via which sane RVA values can result. If sections
> get placed at VA 0 (which is perfectly fine for ELF), the RVA would be a
> huge negative number truncated to 32 bits. (Again, prior to binutils 2.37
> this will go all silently.) Plus the respective sections would come first
> (rather than last) in the binary (which by itself may or may not be a
> problem for the EFI loader, but I wouldn't want to chance it).

Thanks for the explanation.

> >> --- a/xen/arch/x86/xen.lds.S
> >> +++ b/xen/arch/x86/xen.lds.S
> >> @@ -312,10 +312,60 @@ SECTIONS
> >>      *(.reloc)
> >>      __base_relocs_end = .;
> >>    }
> >> -  /* Trick the linker into setting the image size to exactly 16Mb. */
> >> -  . = ALIGN(__section_alignment__);
> >> -  DECL_SECTION(.pad) {
> >> -    . = ALIGN(MB(16));
> >> +  .debug_abbrev ALIGN(1) (NOLOAD) : {
> >> +     *(.debug_abbrev)
> >> +  }
> >> +  .debug_info ALIGN(1) (NOLOAD) : {
> >> +    *(.debug_info)
> >> +    *(.gnu.linkonce.wi.*)
> >> +  }
> >> +  .debug_types ALIGN(1) (NOLOAD) : {
> >> +    *(.debug_types)
> >> +  }
> >> +  .debug_str ALIGN(1) (NOLOAD) : {
> >> +    *(.debug_str)
> >> +  }
> >> +  .debug_line ALIGN(1) (NOLOAD) : {
> >> +    *(.debug_line)
> >> +    *(.debug_line.*)
> >> +  }
> >> +  .debug_line_str ALIGN(1) (NOLOAD) : {
> >> +    *(.debug_line_str)
> >> +  }
> >> +  .debug_names ALIGN(4) (NOLOAD) : {
> >> +    *(.debug_names)
> >> +  }
> >> +  .debug_frame ALIGN(4) (NOLOAD) : {
> >> +    *(.debug_frame)
> >> +  }
> >> +  .debug_loc ALIGN(1) (NOLOAD) : {
> >> +    *(.debug_loc)
> >> +  }
> >> +  .debug_loclists ALIGN(4) (NOLOAD) : {
> >> +    *(.debug_loclists)
> >> +  }
> >> +  .debug_ranges ALIGN(8) (NOLOAD) : {
> >> +    *(.debug_ranges)
> >> +  }
> >> +  .debug_rnglists ALIGN(4) (NOLOAD) : {
> >> +    *(.debug_rnglists)
> >> +  }
> >> +  .debug_addr ALIGN(8) (NOLOAD) : {
> >> +    *(.debug_addr)
> >> +  }
> >> +  .debug_aranges ALIGN(1) (NOLOAD) : {
> >> +    *(.debug_aranges)
> >> +  }
> >> +  .debug_pubnames ALIGN(1) (NOLOAD) : {
> >> +    *(.debug_pubnames)
> >> +  }
> >> +  .debug_pubtypes ALIGN(1) (NOLOAD) : {
> >> +    *(.debug_pubtypes)
> >> +  }
> >> +  /* Trick the linker into setting the image size to no less than 16Mb. */
> >> +  __image_end__ = .;
> >> +  .pad ALIGN(__section_alignment__) : {
> >> +    . = __image_end__ < __image_base__ + MB(16) ? ALIGN(MB(16)) : .;
> > 
> > I think this is inside an ifdef EFI region, since this is DWARF info
> > couldn't it also be present when building with EFI disabled?
> 
> Of course (and it's not just "could" but "will"), yet the linker will
> do fine (and perhaps even better) without when building ELF. Also
> note that we'll be responsible for keeping the list of sections up-to-
> date. The linker will recognize Dwarf sections by looking for a
> .debug_ prefix. We can't use such here (or at least I'm not aware of
> a suitable mechanism); .debug_* would mean munging together all the
> different kinds of Dwarf sections. Hence by limiting the explicit
> enumeration to PE, I'm trying to avoid anomalies in ELF down the road.

Right, so we will have to keep this list of debug_ sections updated
manually if/when more of those appear as part of DWARF updates?

Do we have a way to get some kind of warning or error when a new
section not explicitly handled here appears?

Instead of adding DWARF debug info to the xen.efi binary, is there a
way to translate the DWARF from the ELF build into the native debug
format for PE/COFF?

Thanks, Roger.