On Tue, Mar 08, 2022 at 01:34:06PM +0100, Jan Beulich wrote:
> On 08.03.2022 13:11, Roger Pau Monné wrote:
> > On Tue, Mar 08, 2022 at 12:15:04PM +0100, Jan Beulich wrote:
> >> On 08.03.2022 11:12, Roger Pau Monné wrote:
> >>> On Mon, Mar 07, 2022 at 02:53:32PM +0100, Jan Beulich wrote:
> >>>> @@ -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?
> >>
> >> All the notes will be covered by the NOTE phdr. I had this much later
> >> in the script originally, but then the NOTE phdr covered large parts of
> >> .init.*. Clearly that yields invalid notes, which analysis (or simple
> >> dumping) tools wouldn't be happy about. We might be able to add 2nd
> >> NOTE phdr, but mkelf32 assumes exactly 2 phdrs if it finds more than
> >> one, so changes there would likely be needed then (which I'd like to
> >> avoid for the moment). I'm also not sure in how far tools can be
> >> expected to look for multiple NOTE phdrs ...
> >
> > But if we are adding a .note section now we might as well merge it
> > with .note.gnu.build-id:
> >
> > DECL_SECTION(.note) {
> > __note_gnu_build_id_start = .;
> > *(.note.gnu.build-id)
> > __note_gnu_build_id_end = .;
> > *(.note.*)
> > } PHDR(note) PHDR(text)
> >
> > And drop the .note.Xen section?
>
> In an ideal world we likely could, yes. But do we know for sure that
> nothing recognizes the Xen notes by section name?
Wouldn't that be wrong? In the elfnotes.h file it's clearly specified
that Xen notes live in a PT_NOTE program header and have 'Xen' in the
name field. There's no requirement of them being in any specific
section.
> .note.gnu.build-id
> cannot be folded in any event - see the rule for generating note.o,
> to be used by xen.efi linking in certain cases.
Right, so we need to keep the .note.gnu.build-id section, but we could
likely fold .note.Xen into .note I think?
Or at least add a comment to mention that we don't want to fold
.note.Xen into .note in case there are tools that search for specific
Xen notes to be contained in .note.Xen.
> >>>> +#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?
> >>
> >> I guess that's largely for consistency with all other directives. With the
> >> assertions that these need to be empty, we might get away without, as long
> >> as no linker would decide to set up another zero-size phdr for them.
> >
> > We already set the debug sections to not be part of any program header
> > and seem to get away with it. I'm not sure how different the sections
> > handled below would be, linkers might indeed want to place them
> > regardless?
>
> Simply because I don't know I'd like to be on the safe side. Debug sections
> can't really be taken as reference: At least GNU ld heavily special-cases
> them anyway.
>
> > If so it might be good to add a comment that while those should be
> > empty (and thus don't end up in any program header) we assign them to
> > the text one in order to avoid the linker from creating a new program
> > header for them.
>
> I'll add a sentence to the comment I'm already adding here.
Thanks, Roger.
