Subject: Re: [PATCH] powerpc/vdso: Fix incorrect CFI in gettimeofday.S

From: Michael Ellerman <[email-masked]>
Date: Wed, 04 May 2022 22:34:06 +1000

Alan Modra <[email-masked]> writes:
> On Mon, May 02, 2022 at 09:27:05AM -0500, Segher Boessenkool wrote:
>> >   2) If a function changes LR or any non-volatile register, the save
>> >      location for those regs must be given. The cfi can be at any
>> >      instruction after the saves up to the point that the reg is
>> >      changed. (Exception: LR save should be described before a bl.)
>> 
>> That isn't an exception?  bl changes the current LR after all :-)
>
> The point is that in other cases the cfi can be as late as the
> instruction that changes the reg.  For calls it must be at least one
> instruction before the call.

Got it.

> Also, I'll note for the wider audience that delaying cfi is slightly
> better than playing it safe as Michael has done in his patch in
> describing the saves right at the save instruction.  Register save cfi
> can usually be grouped together, resulting in fewer CFI_advance codes
> in .eh_frame.

I didn't want to go overboard on combining them, because it's harder to
read the source, especially with the #ifdefs we have for 64-bit.

I was able to save one CFA_advance_loc by moving the LR restore down one
line.

The .eh_frame didn't shrink, I guess because it's padded to some
alignment anyway.

>> Alan proposed a larger patch that changed to a single stack frame, but it needs changes to
>> take into account the red zone.
>
> Yes, now that you mention it, I see the obvious error in the patch I
> wrote.  I did say it was untested!

No worries, identifying the source of the bug as the missing CFI is the
key thing, it would have taken me a while to realise that.

cheers