Re: [Regression] [PATCH] x86: fold sections in final binaries

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.03.2022 08:29, Jan Beulich wrote:
> On 03.03.2022 21:02, Andrew Cooper wrote:
>> On 01/03/2022 08:55, Jan Beulich wrote:
>>> Especially when linking a PE binary (xen.efi), standalone output
>>> sections are expensive: Often the linker will align the subsequent one
>>> on the section alignment boundary (2Mb) when the linker script doesn't
>>> otherwise place it. (I haven't been able to derive from observed
>>> behavior under what conditions it would not do so.)
>>>
>>> With gcov enabled (and with gcc11) I'm observing enough sections that,
>>> as of quite recently, the resulting image doesn't fit in 16Mb anymore,
>>> failing the final ASSERT() in the linker script. (That assertion is
>>> slated to go away, but that's a separate change.)
>>>
>>> Any destructor related sections can be discarded, as we never "exit"
>>> the hypervisor. This includes .text.exit, which is referenced from
>>> .dtors.*. Constructor related sections need to all be taken care of, not
>>> just those with historically used names: .ctors.* and .text.startup is
>>> what gcc11 populates. While there re-arrange ordering / sorting to match
>>> that used by the linker provided scripts.
>>>
>>> Finally, for xen.efi only, also discard .note.gnu.*. These are
>>> meaningless in a PE binary. Quite likely, while not meaningless there,
>>> the section is also of no use in ELF, but keep it there for now.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> ---
>>> TBD: We also use CONSTRUCTORS for an unknown reason. Documentation for
>>>      ld is quite clear that this is an a.out-only construct.
>>>      Implementation doesn't look to fully match this for ELF, but I'd
>>>      nevertheless be inclined to remove its use.
>>>
>>> --- a/xen/arch/x86/xen.lds.S
>>> +++ b/xen/arch/x86/xen.lds.S
>>> @@ -194,6 +194,7 @@ SECTIONS
>>>  #endif
>>>         _sinittext = .;
>>>         *(.init.text)
>>> +       *(.text.startup)
>>>         _einittext = .;
>>>         /*
>>>          * Here are the replacement instructions. The linker sticks them
>>> @@ -258,9 +259,10 @@ SECTIONS
>>>  
>>>         . = ALIGN(8);
>>>         __ctors_start = .;
>>> -       *(.ctors)
>>> +       *(SORT_BY_INIT_PRIORITY(.init_array.*))
>>> +       *(SORT_BY_INIT_PRIORITY(.ctors.*))
>>>         *(.init_array)
>>> -       *(SORT(.init_array.*))
>>> +       *(.ctors)
>>>         __ctors_end = .;
>>>    } PHDR(text)
>>>  
>>> @@ -404,16 +406,20 @@ SECTIONS
>>>  
>>>    /* Sections to be discarded */
>>>    /DISCARD/ : {
>>> +       *(.text.exit)
>>>         *(.exit.text)
>>>         *(.exit.data)
>>>         *(.exitcall.exit)
>>>         *(.discard)
>>>         *(.discard.*)
>>>         *(.eh_frame)
>>> +       *(.dtors)
>>> +       *(.dtors.*)
>>>  #ifdef EFI
>>>         *(.comment)
>>>         *(.comment.*)
>>>         *(.note.Xen)
>>> +       *(.note.gnu.*)
>>>  #endif
>>>    }
>>
>> This breaks reliably in Gitlab CI.
>>
>> https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/2159059956 (gcc 11)
> 
> Hmm, I wonder why I'm not seeing this locally. The lack of mentioning of
> .fini_array in the linker script struck me as odd already before. I can
> easily make a patch to add those sections to the script, but I'd first
> like to understand why I'm seeing gcc11 use .ctors / .dtors while here
> it's .init_array / .fini_array.

And it's as simple as this, seen in gcc's config.log:

configure:24049: checking for .preinit_array/.init_array/.fini_array support
configure:24214: checking cross compile... guessing
configure:24219: result: no

The mentioning of .preinit_array there of course makes me wonder whether
we need to also cater for that section. But with the orphan section
warning in place (hopefully soon), we'd at least be made aware by the
linker if such a section ever appears for whichever reason.

Jan