Re: [PATCH] livepatch: set -f{function,data}-sections compiler option

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

On Wed, Mar 02, 2022 at 05:09:10PM +0100, Jan Beulich wrote:
> On 02.03.2022 16:46, Roger Pau Monné wrote:
> > On Wed, Mar 02, 2022 at 03:41:21PM +0100, Jan Beulich wrote:
> >> On 02.03.2022 14:44, Roger Pau Monne wrote:
> >>> @@ -292,6 +295,9 @@ SECTIONS
> >>>         *(.data)
> >>>         *(.data.rel)
> >>>         *(.data.rel.*)
> >>> +#ifdef CONFIG_LIVEPATCH
> >>> +       *(.data.*)
> >>> +#endif
> >>>         CONSTRUCTORS
> >>>    } PHDR(text)
> >>>  
> >>> @@ -308,6 +314,9 @@ SECTIONS
> >>>         . = ALIGN(SMP_CACHE_BYTES);
> >>>         __per_cpu_data_end = .;
> >>>         *(.bss)
> >>> +#ifdef CONFIG_LIVEPATCH
> >>> +       *(.bss.*)
> >>> +#endif
> >>
> >> ... are these two really in need of being conditional?
> > 
> > Will drop if you agree. I didn't want to risk introducing unwanted
> > changes in the !CONFIG_LIVEPATCH case.
> 
> The only "unwanted" change I can imagine here would be that we place a
> section which the linker would otherwise need to guess how to place,
> for being "orphan".
> 
> >>> --- a/xen/common/Kconfig
> >>> +++ b/xen/common/Kconfig
> >>> @@ -353,7 +353,9 @@ config CRYPTO
> >>>  config LIVEPATCH
> >>>   bool "Live patching support"
> >>>   default X86
> >>> - depends on "$(XEN_HAS_BUILD_ID)" = "y"
> >>> + depends on "$(XEN_HAS_BUILD_ID)" = "y" && \
> >>> +            $(cc-option,-ffunction-sections) && \
> >>> +            $(cc-option,-fdata-sections)
> >>
> >> Is this for certain Clang versions? Gcc has been supporting this in
> >> 4.1.x already (didn't check when it was introduced).
> > 
> > I've checked clang and it seems to be prevent in at least Clang 5,
> > which is likely enough?
> 
> Clang5 accepts the options fine here. But that wouldn't be enough,
> ./README says "Clang/LLVM 3.5 or later".
> 
> > I've added the check just to be on the safe side.
> 
> Well, yes, if you're unsure and the old version can't be checked,
> then perhaps indeed better to probe.

OK, so I've managed to probe clang 3.5.0 and it does support
-f{function,data}-sections so we can drop the check.

Thanks, Roger.