| Message ID | 03851bd4-9202-385f-d991-c9720185c946@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: fold sections in final binaries | expand |
On 01.03.2022 09: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@suse.com> Some of this will likely want mirroring to Arm as well, even if xen.efi there isn't produced by the linker. Sections are better properly folded even for ELF, and constructors not ending up in [__ctors_start,__ctors_end) can surely not do any good. Jan > --- > 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 > } > > >
Hi Jan, > On 1 Mar 2022, at 08:58, Jan Beulich <jbeulich@suse.com> wrote: > > On 01.03.2022 09: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@suse.com> > > Some of this will likely want mirroring to Arm as well, even if xen.efi > there isn't produced by the linker. Sections are better properly folded > even for ELF, and constructors not ending up in [__ctors_start,__ctors_end) > can surely not do any good. I fully agree with that and it would make sense to do both changes together to avoid differences between x86 and arm unless required. Right now our discard section on arm is a lot shorter and I do not see why we would need any of the sections that are discarded on x86. As this needs testing and checking I do not think it makes sense for you to do that right now. @Stefano and Julien: I am ok to create myself a task to sync with x86 in the next weeks/months, what do you think ? Cheers Bertrand > > Jan > >> --- >> 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 >> }
On Tue, Mar 01, 2022 at 09:55:32AM +0100, 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. Should we also use --orphan-handling=warn as to recognize orphaned sections and attempt place them? We have now detected this because of the 16Mb limit, but if we remove that check that we could end up carrying a non-trivial amount of 2Mb aligned unhandled regions. > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > TBD: We also use CONSTRUCTORS for an unknown reason. Documentation for > ld is quite clear that this is an a.out-only construct. I've found some (old) documentation where it does also mention ECOFF and XCOFF apart from a.out: "When linking object file formats which do not support arbitrary sections, such as ECOFF and XCOFF, the linker will automatically recognize C++ global constructors and destructors by name. For these object file formats, the CONSTRUCTORS command tells the linker where this information should be placed." I guess we can get rid of it. The patch LGTM: Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> With the possible addition of --orphan-handling=warn. Thanks, Roger.
On 01.03.2022 14:43, Roger Pau Monné wrote: > On Tue, Mar 01, 2022 at 09:55:32AM +0100, 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. > > Should we also use --orphan-handling=warn as to recognize orphaned > sections and attempt place them? We have now detected this because of > the 16Mb limit, but if we remove that check that we could end up > carrying a non-trivial amount of 2Mb aligned unhandled regions. Yes, I guess we should use this option. See below. >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> --- >> TBD: We also use CONSTRUCTORS for an unknown reason. Documentation for >> ld is quite clear that this is an a.out-only construct. > > I've found some (old) documentation where it does also mention ECOFF > and XCOFF apart from a.out: > > "When linking object file formats which do not support arbitrary > sections, such as ECOFF and XCOFF, the linker will automatically > recognize C++ global constructors and destructors by name. For these > object file formats, the CONSTRUCTORS command tells the linker where > this information should be placed." > > I guess we can get rid of it. I've taken note to make yet another patch. > The patch LGTM: > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Thanks. > With the possible addition of --orphan-handling=warn. As above: I agree we should make use of the option, but I don't think this should be squashed in here. The option appeared in 2.26, so with the current documented binutils baseline we'll need to probe for its availability. I'd therefore prefer to make this a separate change, and I've taken respective note as well. Jan
On Tue, Mar 01, 2022 at 03:06:30PM +0100, Jan Beulich wrote: > On 01.03.2022 14:43, Roger Pau Monné wrote: > > On Tue, Mar 01, 2022 at 09:55:32AM +0100, 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. > > > > Should we also use --orphan-handling=warn as to recognize orphaned > > sections and attempt place them? We have now detected this because of > > the 16Mb limit, but if we remove that check that we could end up > > carrying a non-trivial amount of 2Mb aligned unhandled regions. > > Yes, I guess we should use this option. See below. > > >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > >> --- > >> TBD: We also use CONSTRUCTORS for an unknown reason. Documentation for > >> ld is quite clear that this is an a.out-only construct. > > > > I've found some (old) documentation where it does also mention ECOFF > > and XCOFF apart from a.out: > > > > "When linking object file formats which do not support arbitrary > > sections, such as ECOFF and XCOFF, the linker will automatically > > recognize C++ global constructors and destructors by name. For these > > object file formats, the CONSTRUCTORS command tells the linker where > > this information should be placed." > > > > I guess we can get rid of it. > > I've taken note to make yet another patch. > > > The patch LGTM: > > > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > > Thanks. > > > With the possible addition of --orphan-handling=warn. > > As above: I agree we should make use of the option, but I don't think > this should be squashed in here. The option appeared in 2.26, so with > the current documented binutils baseline we'll need to probe for its > availability. I'd therefore prefer to make this a separate change, > and I've taken respective note as well. I didn't check when it first appeared. I'm fine with a separate change. Thanks, Roger.
On 01.03.2022 09:55, Jan Beulich wrote: > --- 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) While I did commit the change with Roger's R-b, on the basis that it's not going to make things worse, I don't think what we have here and what we do in init_constructors() is quite right: For one .init_array and .ctors are supposed to be processed in, respectively, opposite order - the former forwards, the latter backwards. See e.g. gcc's libgcc/gbl-ctors.h. And then both variants also shouldn't be intermixed; we ought to expect only one of the two kinds, and aiui for now it's always going to be .ctors. The processing in wrong order looks to not be a problem in the builds I can check, as there's only ever a single priority used. But we're at risk of this breaking down the road ... Finally, if we consider .init_array might appear, we ought to also discard (rather than leaving orphaned) .fini_array. Jan
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@suse.com> > --- > 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) ~Andrew
Hi Bertrand, On 01/03/2022 13:30, Bertrand Marquis wrote: >> On 1 Mar 2022, at 08:58, Jan Beulich <jbeulich@suse.com> wrote: >> >> On 01.03.2022 09: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@suse.com> >> >> Some of this will likely want mirroring to Arm as well, even if xen.efi >> there isn't produced by the linker. Sections are better properly folded >> even for ELF, and constructors not ending up in [__ctors_start,__ctors_end) >> can surely not do any good. > > I fully agree with that and it would make sense to do both changes together to > avoid differences between x86 and arm unless required. > > Right now our discard section on arm is a lot shorter and I do not see why we > would need any of the sections that are discarded on x86. Me neither. > > As this needs testing and checking I do not think it makes sense for you to do > that right now. > @Stefano and Julien: I am ok to create myself a task to sync with x86 in the > next weeks/months, what do you think ? I haven't looked in details the exact difference between two linker scripts. After the sync, I would expect to be mostly similar. We also have the RISCv and possibly soon PowerPC. So, I would consider to consolidate the linker scripts if possible. This would help to keep them in sync. Anyway, as discussed on IRC, let's start with updating the Arm linker scripts. We can then look at the differences. Cheers,
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@suse.com> >> --- >> 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. Jan
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@suse.com> >>> --- >>> 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
On 01.03.2022 09:55, Jan Beulich wrote: > @@ -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) Sadly there's another regression here, with old GNU ld: SORT_BY_INIT_PRIORITY is known by binutils 2.22 and newer only. 2.21 uses SORT() in respective places, but I have to admit I don't fancy adding a probe for what we can or cannot have in linker scripts. Yet for now I also don't see any alternative yet ... Jan
--- 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 }
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@suse.com> --- 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.