Re: Restoring pc to a different value than lr on aarch64

[Yichao Yu <yyc1992@gmail.com>]

On Tue, May 10, 2022 at 10:49 AM Luis Machado <luis.machado@arm.com> wrote:
>
> On 5/9/22 15:24, Yichao Yu wrote:
> > On Mon, May 9, 2022 at 6:44 AM Luis Machado <luis.machado@arm.com> wrote:
> >>
> >> On 5/6/22 17:30, Yichao Yu wrote:
> >>> On Fri, May 6, 2022 at 12:11 PM Yichao Yu <yyc1992@gmail.com> wrote:
> >>>>
> >>>>>> Actually I misspoke for that. It seems that sp is probably fine and
> >>>>>> the only thing missing causing pc to not work is that
> >>>>>> aarch64_dwarf_reg_to_regnum doesn't understand the PC dwarf reg
> >>>>>> number. It seems that the only thing needed is to add a
> >>>>>>
> >>>>>> +  if (reg == AARCH64_DWARF_PC)
> >>>>>> +    return AARCH64_PC_REGNUM;
> >>>>>>
> >>>>>> to that function.
> >>>>>>
> >>>>>
> >>>>> Yes, GDB always assumes the PC from the previous frame is the LR from
> >>>>> the current frame. That is what GCC generates.
> >>>>>
> >>>>> If a different setup is needed, GDB needs to be taught about it.
> >>>>
> >>>> I agree the current code makes sense for what gcc generates. However,
> >>>> I think given the document from arm, explicitly setting the PC value
> >>>> in the unwind info should also work.
> >>>> Would a patch similar to the one above be acceptable to fix this issue?
> >>>>
> >>>> A related issue is that gdb also seems to be ignoring the return
> >>>> address register in CIE. There is at least one use of it in glibc[2]
> >>>> where the return address register is set to x15 instead.
> >>>> I've verified that gdb is currently unable to unwind after the call to
> >>>> `strlen` from `rawmemchr` even though the return address is still in
> >>>> x15.
> >>>> I thought this can be fixed by chaiming that PC is RA just like the
> >>>> fallback case but that is apparently not working...
> >>>
> >>> Actually this did work but the address is wrong before the value was
> >>> written to x15... So it was just due to incorrect unwind info (the
> >>> glibc implementation should specify how to find x15 on the entry of
> >>> rawmemchr).
> >>> Is the current implementation due to some edge cases? (like old
> >>> compiler version doesn't put a valid value in the CIE for the return
> >>> address register). It seems that many other architecture simply use
> >>> _RA so I don't see why this would have broader problems...
>
> This looks like a genuine bug, one that is not hit that often given it
> is not very common for frame to change the return address column. This
> will need to be fixed eventually. Thanks for spotting that.
>
> >>
> >> It is probably historical that this has been handled like this. If there
> >> is a use case for having a PC column containing distinct data, then we
> >> could support that.
> >
> > I couldn't find any existed code using this, but it seems that this is
> > the intent from ARM and I really can't think of any other way to
> > restore everything including both pc and lr so I'd like to support
> > that at least....
> >
> >> It wouldn't be as simple as that change you mentioned though, as other
> >> parts of the code assume PC comes from the LR.
>
> I take that back. I investigated this further and I think this should
> work, although it is a use case that is not so common.
>
> >
> > There are indeed other unwinders (the prologue unwinder) that assumes
> > this, which I think should be fine. Specifying the PC explicitly
> > should only apply to the dwarf unwinder. The only other place where I
> > can find that relies on this is aarch64_gen_return_address. It doesn't
> > seem that this function is used in most of the other logics and if the
> > return address column is somehow accessible from here it should also
> > not be too hard to fix.
>
> I think it is only the case for the dwarf unwinder, as that is the
> unwinder that has access to the CFI information.

Sorry what did you mean by "it" here? (being able to handle PC != LR
during unwinding?) Do I need to worry about gen_return_address?

> >
> > Is there any other cases that I'm missing? I've also tested with my
> > simple change and it seems that unwinding from normal functions still
> > works as intended.
>
> Yes, I tried it on my end as well and it does work. What I was worried
> about is that we need to adjust the LR value in some cases.
>
> For aarch32 we need to remove the LSB, and for aarch64 we need to unmask
> the value of LR if it is signed through PAC. This is the reason why we
> have a custom dwarf2_prev_register implementation.

Is the aarch64-tdep code used for aarch32 tracee as well?

> gcc and clang emit the return address column as r30. If we don't specify
> any initialization rule for PC, then the dwarf2 unwinder will go through
> the route of fetching LR and adjusting the values.
>
> On the other hand, if PC is available in a CFI column, then the dwarf
> unwinder won't call the custom hook and will instead proceed to
> calculate PC from the CFI expression at that column, which should give
> the result that you want.

Right, and I guess that means that the custom unwind rule needs to
take the pointer authentication into account?
(I've never used any machines with that enabled so I'm not 100% how
it's supposed to work....)

> We can't initialize PC to DWARF2_FRAME_REG_RA though (not yet, anyway),
> as that would default to mapping PC to the return address column. For
> gcc and clang, this would be x30. That would give GDB no chance of
> adjusting the LR values if required (for PAC-signed LR values).

Just so that I understand, if LR is signed, then upon return it should
still have the signed value. (Or in the case a different register is
used for return address, `ret xn` would not change xn value)
Only the resulting PC should have the signed part masked off. That's
why the unwinding of LR is correct without any post-processing logic
as-is but the unwinding of PC needs to be treated specially.

> It would be nice to have a testcase for this, alongside your patch, to
> make sure GDB is always doing the correct unwinding.

The part for returning the name of PC is easy. I assume the test would
be mainly throwing in a aarch64-xxx.{c,exp} and to check if explicitly
specifying the PC in the unwind info result in valid unwinding. I'll
try look for some but any examples that I can follow?
This should fix the case where PC is explicitly specified but wont fix
the case where a return column is specified. Handling that requires a
different way to post-processing the PC.

> Could you please send a patch to gdb-patches@?

Yes, sure. Mainly want to first figure out what's the expected solution here.

> >
> >> PC is probably specified
> >> as DWARF2_FRAME_REG_SAME_VALUE (as far as I remember), so it will cause
> >> some issues during unwinding.
> >
> > I don't believe this is the case. AFAICT, the only way gcc emits debug
> > info to specify that pc is in lr without relying on architecture
> > detail is that it specifies the return address column is 30 in the
> > CIE. Currently, on aarch64, the return address column is basically
> > never used but if the how for PC is changed to RA then it seems that
> > gdb does pick it up correctly.
> >
> >> There is a comment in gdb/dwarf2/expr.c about some odd cases. For example:
> >>
> >>
> >>         /* GCC, in its infinite wisdom decided to not provide unwind
> >>            information for registers that are "same value".  Since
> >>            DWARF2 (3 draft 7) doesn't define such behavior, said
> >>            registers are actually undefined (which is different to CFI
> >>            "undefined").  Code above issues a complaint about this.
> >>            Here just fudge the books, assume GCC, and that the value is
> >>            more inner on the stack.  */
> >>
> >>
> >>>
> >>>
> >>>>
> >>>> [2] https://github.com/bminor/glibc/blob/b92a49359f33a461db080a33940d73f47c756126/sysdeps/aarch64/rawmemchr.S#L34
> >>>>
> >>>>>>>
> >>>>>>> According to aadwarf64[1],
> >>>>>>>
> >>>>>>>> having both LR and PC columns is useful for describing asynchronously created stack frames. A DWARF expression may use this register to restore the context in case of a signal context.
> >>>>>>>
> >>>>>>> so assume the intention is that if I explicitly unwind the pc in
> >>>>>>> addition to lr, it should work. I tried to do that, and also to set
> >>>>>>> return address column to 32, as well as trying to mark the frame as
> >>>>>>> signal frame but none of them seems to work. Is there any way for gdb
> >>>>>>> to honer the explicit unwinding of pc?
> >>>>>>>
> >>>>>>> Also it seems that the sp is also card coded to be cfa. My code also
> >>>>>>> contains explicit saving and restoring of that as well so if that's
> >>>>>>> the case (haven't tested yet) it would be a problem too...
> >>>>>>>
> >>>>>>> Would it be possible to not use this hard-coded logic if the frame
> >>>>>>> contains explicit override of the pc value?
> >>>>>>>
> >>>>>>> Yichao
> >>>>>>>
> >>>>>>> A bit more about the actual code. This is done as part of runtime
> >>>>>>> patching code. The actual restoration of lr is done by returning to a
> >>>>>>> runtime allocated stub that restores lr and directly branch back to
> >>>>>>> the return location. After returning, all registers values are
> >>>>>>> restored back to their previous one. The stack pointer is also
> >>>>>>> switched out since we cannot rely on how much stack space the call
> >>>>>>> site has available.
> >>>>>
> >>>>> This seems to work in a similar way as signal handler. GDB needs to be
> >>>>> taught where to find the registers so it can properly unwind things.
> >>>>>
> >>>>>>>
> >>>>>>> [1] https://github.com/ARM-software/abi-aa/blob/8a7b266879c60ca1c76e94ebb279b2dac60ed6a5/aadwarf64/aadwarf64.rst#note-9
> >>>>>
> >>
>