Re: [PATCH] Disable non-ABI-compliant optimisations for live patching

[Josh Poimboeuf <jpoimboe@redhat.com>]

On Wed, Jun 22, 2016 at 04:24:41PM +0200, Torsten Duwe wrote:
> Live patching, as we use it, deliberately disrupts the fabric of
> compile units; thus all assumptions a compiler can make about the
> control flow may be invalid. As an example, it could analyse that a
> callee does not touch a caller-saved register at all, so why waste
> memory bandwidth saving it? The register allocations for the live
> patch replacement function may however be quite different.
> 
> Starting with this example, disable all compiler optimisations that
> do not strictly comply with the established calling conventions.
> 
> Signed-off-by: Torsten Duwe <duwe@suse.de>
> ---
> 
> Working on the arm64 ftrace-with-regs/livepatch, it struck me that
> this is a general problem: with live patching, certain optimisations
> must be switched off for all architectures, the new(?) IPA register
> allocator in gcc6 is only one example. We should tackle this
> well before it bites us.
> 
> 	Torsten

I think this is a good idea.  While we're at it, should we also disable
some of the other IPA options?  These sound especially problematic:

-fipa-sra
   Perform interprocedural scalar replacement of aggregates, removal of
   unused parameters and replacement of parameters passed by reference
   by parameters passed by value.

-fipa-cp
   Perform interprocedural constant propagation.  This optimization
   analyzes the program to determine when values passed to functions are
   constants and then optimizes accordingly.  This optimization can
   substantially increase performance if the application has constants
   passed to functions.

-fipa-icf
   Perform Identical Code Folding for functions and read-only variables.
   The optimization reduces code size and may disturb unwind stacks by
   replacing a function by equivalent one with a different name. The
   optimization works more effectively with link time optimization
   enabled.


> 
> ---
>  Makefile | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index b409076..424d2e6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -743,6 +743,13 @@ KBUILD_CFLAGS 	+= $(call cc-option, -femit-struct-debug-baseonly) \
>  		   $(call cc-option,-fno-var-tracking)
>  endif
>  
> +ifdef CONFIG_LIVEPATCH
> +# The compiler might generate ABI "shortcuts" to speed up the code,
> +# making assumptions which are no longer valid when live patching
> +# is enabled. Disable all of them.
> +KBUILD_CFLAGS	+= $(call cc-option,-fno-ipa-ra)
> +endif
> +
>  ifdef CONFIG_FUNCTION_TRACER
>  ifndef CC_FLAGS_FTRACE
>  CC_FLAGS_FTRACE := -pg
> -- 
> 2.6.6
> 

-- 
Josh