Re: [PATCH] x86/cet: Fix build on newer versions of GCC

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Tue, Aug 17, 2021 at 12:17:31PM +0100, Andrew Cooper wrote:
> On 17/08/2021 12:14, Marek Marczykowski-Górecki wrote:
> > On Tue, Aug 17, 2021 at 11:56:56AM +0100, Andrew Cooper wrote:
> >> Some versions of GCC complain with:
> >>
> >>   traps.c:405:22: error: 'get_shstk_bottom' defined but not used 
> >> [-Werror=unused-function]
> >>    static unsigned long get_shstk_bottom(unsigned long sp)
> >>                         ^~~~~~~~~~~~~~~~
> >>   cc1: all warnings being treated as errors
> >>
> >> Change #ifdef to if ( IS_ENABLED(...) ) to make the sole user of
> >> get_shstk_bottom() visible to the compiler.
> >>
> >> Fixes: 35727551c070 ("x86/cet: Fix shskt manipulation error with 
> >> BUGFRAME_{warn,run_fn}")
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> >> ---
> >> CC: Jan Beulich <JBeulich@xxxxxxxx>
> >> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> >> CC: Wei Liu <wl@xxxxxxx>
> >> CC: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> >>
> >> Not actually tested.  I don't seem to have a new enough GCC to hand.
> > I have just compile-tested it and it seems to fix the issue (indeed it
> > failed before with CONFIG_XEN_SHSTK disabled).
> 
> Oh, thanks!

And I can confirm it doesn't break anything runtime (but that's pretty
obvious looking at the patch).

> >> Most of the delta here is indentation.  This diff is more easily reviewed 
> >> with
> >> `git show --ignore-all-space`
> > Wouldn't this make the compiler include the code even if
> > CONFIG_XEN_SHSTK is disabled (not a huge issue...)? Or is it smart
> > enough to optimize it out in that case?
> 
> Its a trivial dead-code elimination example, and yes - the compiler is
> smart enough.

:)

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Attachment: signature.asc
Description: PGP signature