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

From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>, Wei Liu <wl@xen.org>
Subject: Re: [PATCH 7/8] x86/EFI: keep debug info in xen.efi
Date: Fri, 23 Apr 2021 12:58:05 +0200	[thread overview]
Message-ID: <YIKoPXpMKIHcqaJI@Air-de-Roger> (raw)
In-Reply-To: <c0edf6b1-46ea-e246-223e-b60356c928d6@suse.com>

On Fri, Apr 23, 2021 at 12:45:14PM +0200, Jan Beulich wrote:
> On 23.04.2021 12:07, Roger Pau Monné wrote:
> > On Fri, Apr 23, 2021 at 10:51:40AM +0200, Jan Beulich wrote:
> >> On 23.04.2021 09:30, Roger Pau Monné wrote:
> >>> /*
> >>>  * Explicitly list debug section for the PE output so that they don't
> >>>  * end up at VA 0 which is below image base and thus invalid. Also use
> >>>  * the NOLOAD directive, even when currently ignored by PE output, in
> >>>  * order to note those sections shouldn't be loaded into memory.
> >>>  *
> >>>  * Note such sections are past _end, so if loaded will be discarded by
> >>>  * Xen anyway.
> >>>  */
> >>>
> >>> Feel free to reword or expand the comment.
> >>
> >> Yes, I've edited it some while inserting. Will see to get to
> >> submitting v2 then.
> >>
> >>> Not sure there's some
> >>> reference we could add here about how debug sections are placed in PE
> >>> files usually.
> >>
> >> As said before - I don't think there's any "usually" here, which is
> >> why different environments have invented different ways. The debug
> >> info native to COFF is more like ELF's symbol table (with a little
> >> bit of extra information) plus Dwarf's .debug_line, but not really
> >> fully covering what you'd expect from debug info.
> > 
> > One last thing, do you know if the newly added debug_* sections get
> > the IMAGE_SCN_MEM_DISCARDABLE section flag set?
> 
> At least with an up-to-date ld (i.e. one meeting the requirements so
> we wouldn't force debug info to be stripped) they do.
> 
> > Not sure there's a way we can force it from the linker script TBH, but
> > would be nice and seems to be a recommended flag for debug sections
> > according to Microsoft [0].
> 
> The linker does this for debugging sections irrespective of what the
> linker script says:
> 
>    if ((sec_flags & SEC_DEBUGGING) != 0)
>      styp_flags |= IMAGE_SCN_MEM_DISCARDABLE;

Great, that's good to know. Note sure it's worth adding to the commit
message, maybe I've just missed part of the documentation where LD
notes that IMAGE_SCN_MEM_DISCARDABLE will be unconditionally added to
debug sections.

Thanks, Roger.