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

From: Torsten Duwe
Date: Thu Jun 23 2016 - 06:05:57 EST


On Thu, Jun 23, 2016 at 09:45:48AM +0200, Miroslav Benes wrote:
>
> Hi,
>
> On Wed, 22 Jun 2016, Josh Poimboeuf wrote:
>
> > 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.
>
> But this exact situation should not be possible. Ftrace stub should (and
> it does on x86_64) save the caller-saved registers for us. Otherwise we

I haven't looked at the fentry solution, but the code I'm involved in saves
the registers so that ftrace, live patch and friends can work freely. But
then it restores all regs and _then_ calls the replacement, so ftrace
saving all regs is no gain at all.

> would have seen problems with kGraft already.

IMHO these problems are rare to trigger. I'm afraid some problems are already
lurking.

> > > Starting with this example, disable all compiler optimisations that
> > > do not strictly comply with the established calling conventions.
>
> It think it is too rough and I'd like to avoid it if possible.

I'm not too happy either, but function calling conventions are there
for a reason. And if you exchange one function with another version,
the convention is all you can rely on.

[ ... slightly out of context ...:]
>
> This could surely be avoided if one can prove that the new patching
> function is optimized in the same way by gcc.

I guess that is what it boils down to in general, and I'd rather avoid that,
as it sometimes requires the whole compile unit to get the same result; and
then we haven't even made the changes we wanted to live patch in the first
place.

My main intention was to create the awareness and offer one possible
solution. I'd be happy if there was a better way.

Torsten