Restoring pc to a different value than lr on aarch64

Restoring pc to a different value than lr on aarch64

[Yichao Yu]

I have a case in my code where I want to restore the value of lr (x30)
during unwinding, to a different value than the return address of the
code. However, it seems that for aarch64,
(aarch64_dwarf2_frame_init_reg among other functions) hardcode x30 and
pc to be exactly the same value after unwinding.

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.

[1] https://github.com/ARM-software/abi-aa/blob/8a7b266879c60ca1c76e94ebb279b2dac60ed6a5/aadwarf64/aadwarf64.rst#note-9

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

[Yichao Yu]

On Fri, May 6, 2022 at 8:05 AM Yichao Yu <yyc1992@gmail.com> wrote:
>
> I have a case in my code where I want to restore the value of lr (x30)
> during unwinding, to a different value than the return address of the
> code. However, it seems that for aarch64,
> (aarch64_dwarf2_frame_init_reg among other functions) hardcode x30 and
> pc to be exactly the same value after unwinding.

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.

>
> 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.
>
> [1] https://github.com/ARM-software/abi-aa/blob/8a7b266879c60ca1c76e94ebb279b2dac60ed6a5/aadwarf64/aadwarf64.rst#note-9

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

[Luis Machado]

On 5/6/22 13:46, Yichao Yu via Gdb wrote:
> On Fri, May 6, 2022 at 8:05 AM Yichao Yu <yyc1992@gmail.com> wrote:
>>
>> I have a case in my code where I want to restore the value of lr (x30)
>> during unwinding, to a different value than the return address of the
>> code. However, it seems that for aarch64,
>> (aarch64_dwarf2_frame_init_reg among other functions) hardcode x30 and
>> pc to be exactly the same value after unwinding.
> 
> 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.

>>
>> 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

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

[Yichao Yu]

> > 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...

[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
>

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

[Yichao Yu]

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...


>
> [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
> >

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

[Luis Machado]

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...

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.

It wouldn't be as simple as that change you mentioned though, as other 
parts of the code assume PC comes from the LR. PC is probably specified 
as DWARF2_FRAME_REG_SAME_VALUE (as far as I remember), so it will cause 
some issues during unwinding.

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
>>>

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

[Yichao Yu]

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...
>
> 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.

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.

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.

> 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
> >>>
>

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

[Luis Machado]

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.

> 
> 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.

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.

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).

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

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

> 
>> 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
>>>>>
>>

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

[Yichao Yu]

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
> >>>>>
> >>
>

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

[Luis Machado]

On 5/11/22 14:26, Yichao Yu wrote:
> 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?
> 

Sorry, I meant that we only need to worry about the dwarf unwinder 
functions. The other code not relying on CFI information doesn't need to 
be touched (prologue unwinder, aarch64_gen_return_address etc).

At this point, we'd like to augment aarch64_dwarf_reg_to_regnum to 
return a valid number for the dwarf PC register.

>>>
>>> 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?
> 

The aarch64 code is not shared by aarch32 (that would be arm-tdep.c). I 
just mentioned it because both arm-tdep.c and aarch64-tdep.c use the 
same mechanism for unwinding PC. That is, they return LR instead.

>> 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....)

It does need to take that into account. But using a custom function for 
unwinding PC means we will not have access to

> 
>> 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. >

That's correct.

>> 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?

I think gdb/testsuite/gdb.arch/arm-disp-step.{S,exp} is a good example. 
Basically, some .S file that contains some random instructions and CFI 
directives that we single-step over and check if GDB is following the 
values of the registers correctly. The .S file gives us more control 
over those CFI directives.

> 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.

Correct. That is a separate bug and I'll need to investigate that.

Thanks!
Luis

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

[Luis Machado]

On 5/11/22 15:51, Luis Machado wrote:
> On 5/11/22 14:26, Yichao Yu wrote:
>> 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?
>>
> 
> Sorry, I meant that we only need to worry about the dwarf unwinder 
> functions. The other code not relying on CFI information doesn't need to 
> be touched (prologue unwinder, aarch64_gen_return_address etc).
> 
> At this point, we'd like to augment aarch64_dwarf_reg_to_regnum to 
> return a valid number for the dwarf PC register.
> 
>>>>
>>>> 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?
>>
> 
> The aarch64 code is not shared by aarch32 (that would be arm-tdep.c). I 
> just mentioned it because both arm-tdep.c and aarch64-tdep.c use the 
> same mechanism for unwinding PC. That is, they return LR instead.
> 
>>> 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....)
> 
> It does need to take that into account. But using a custom function for 
> unwinding PC means we will not have access to
> 

Sorry, this got truncated. With a custom function for unwinding PC, it 
means that we only invoke such function if CFI is not available for a PC 
column. If it is, then we follow the CFI rule but don't do any 
additional processing that would've taken place in the custom function.

Given the case of having a PC column is not so common, this should be OK 
for now.

>>
>>> 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. >
> 
> That's correct.
> 
>>> 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?
> 
> I think gdb/testsuite/gdb.arch/arm-disp-step.{S,exp} is a good example. 
> Basically, some .S file that contains some random instructions and CFI 
> directives that we single-step over and check if GDB is following the 
> values of the registers correctly. The .S file gives us more control 
> over those CFI directives.
> 
>> 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.
> 
> Correct. That is a separate bug and I'll need to investigate that.
> 
> Thanks!
> Luis

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

[Yichao Yu]

On Wed, May 11, 2022 at 10:52 AM Luis Machado <luis.machado@arm.com> wrote:
>
> On 5/11/22 14:26, Yichao Yu wrote:
> > 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?
> >
>
> Sorry, I meant that we only need to worry about the dwarf unwinder
> functions. The other code not relying on CFI information doesn't need to
> be touched (prologue unwinder, aarch64_gen_return_address etc).
>
> At this point, we'd like to augment aarch64_dwarf_reg_to_regnum to
> return a valid number for the dwarf PC register.

... and I just sent out the patch
https://sourceware.org/pipermail/gdb-patches/2022-May/189035.htmlHopefully
I did it more or less right (both in the email and in the patch
itself)
BTW, is there a way to see what gdb actually outputs during the test?
Setting `verbose` doesn't seem to help. I couldn't figure out why `p
$pc` prints `<main+8>` when I used it directly but doesn't seem to in
the test... `$pc - &main` works which is probably better and is what
I'm currently using.

> >>>
> >>> 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?
> >
>
> The aarch64 code is not shared by aarch32 (that would be arm-tdep.c). I
> just mentioned it because both arm-tdep.c and aarch64-tdep.c use the
> same mechanism for unwinding PC. That is, they return LR instead.

OK, that makes sense.

>
> >> 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....)
>
> It does need to take that into account. But using a custom function for
> unwinding PC means we will not have access to
>
> >
> >> 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. >
>
> That's correct.
>
> >> 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?
>
> I think gdb/testsuite/gdb.arch/arm-disp-step.{S,exp} is a good example.
> Basically, some .S file that contains some random instructions and CFI
> directives that we single-step over and check if GDB is following the
> values of the registers correctly. The .S file gives us more control
> over those CFI directives.

By `.c` I actually mean using some inline assembly. But if .S is
supported it is indeed much easier to use.

> > 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.
>
> Correct. That is a separate bug and I'll need to investigate that.
>
> Thanks!
> Luis