Re: AArch64 calling convention in assembly code

Re: AArch64 calling convention in assembly code

[Alexander Fedotov]

>>Pushing LR on the stack resolves a problem
FP of course, not LR.
So the correct code must be like this:

_cpu_init_hook:
    stp        x29, x30, [sp, #-16]!
    mov        x29, sp
    bl        _init_vectors
    bl        _flat_map
    ldp        x29, x30, [sp], #16
    ret

But still my point is that GDB should catch such an error and do not hang.

Alex

On Tue, Jul 31, 2018 at 9:34 PM, Alexander Fedotov <alfedotov@gmail.com> wrote:
> Hello dear AArch64 maintainers
> Please look into code snippet below from newlib/libgloss/aarch64/rdimon-aem-el3.
>
> Seems to me this code violates AArch64 calling convention and actually
> breaks debugging in GDB. GDB tries to unwind call stack and got
> endless reentrancy...
>
> FUNCTION (_cpu_init_hook):
>     sub    sp, sp, #16
>     str    x30, [sp, xzr]
>     bl    _init_vectors
>     bl    _flat_map
>     ldr    x30, [sp, xzr]
>     add    sp, sp, #16
>     ret
>
>
> We have couple of calls there (_init_vectors, _flat_map). If you'll
> try to step into any subroutine you will found that GDB hangs and
> can't step anymore.
>
> Pushing LR on the stack resolves a problem.
>
> So my message is that:
> 1. Current code in _cpu_init_hook is incorrect
> 2. GDB should handle this and do not hang
>
> Alex



-- 
Best regards,
AF

Re: AArch64 calling convention in assembly code

[Richard Earnshaw (lists)]

On 01/08/18 14:28, Alexander Fedotov wrote:
>>> Pushing LR on the stack resolves a problem
> FP of course, not LR.
> So the correct code must be like this:
> 
> _cpu_init_hook:
>     stp        x29, x30, [sp, #-16]!
>     mov        x29, sp
>     bl        _init_vectors
>     bl        _flat_map
>     ldp        x29, x30, [sp], #16
>     ret
> 
> But still my point is that GDB should catch such an error and do not hang.
> 
> Alex
> 
> On Tue, Jul 31, 2018 at 9:34 PM, Alexander Fedotov <alfedotov@gmail.com> wrote:
>> Hello dear AArch64 maintainers
>> Please look into code snippet below from newlib/libgloss/aarch64/rdimon-aem-el3.
>>
>> Seems to me this code violates AArch64 calling convention and actually
>> breaks debugging in GDB. GDB tries to unwind call stack and got
>> endless reentrancy...
>>
>> FUNCTION (_cpu_init_hook):
>>     sub    sp, sp, #16
>>     str    x30, [sp, xzr]
>>     bl    _init_vectors
>>     bl    _flat_map
>>     ldr    x30, [sp, xzr]
>>     add    sp, sp, #16
>>     ret
>>
>>
>> We have couple of calls there (_init_vectors, _flat_map). If you'll
>> try to step into any subroutine you will found that GDB hangs and
>> can't step anymore.
>>
>> Pushing LR on the stack resolves a problem.

X30 is LR.

All we really need is a CFI unwind record that GDB can understand.

R.

>>
>> So my message is that:
>> 1. Current code in _cpu_init_hook is incorrect
>> 2. GDB should handle this and do not hang
>>
>> Alex
> 
> 
> 

Re: AArch64 calling convention in assembly code

[Alexander Fedotov]

Yes, but X29 is not saved in original version. I can understand this
with 'frame-less functions" approach. So this code has a right for a
life.

I suspect that such a problem could happen if user will pass
"-fomit-frame-pointer" option to GCC as well.

>>All we really need is a CFI unwind record that GDB can understand.
For plain assembly code ?

Alex

On Wed, Aug 1, 2018 at 6:48 PM, Richard Earnshaw (lists)
<Richard.Earnshaw@arm.com> wrote:
> On 01/08/18 14:28, Alexander Fedotov wrote:
>>>> Pushing LR on the stack resolves a problem
>> FP of course, not LR.
>> So the correct code must be like this:
>>
>> _cpu_init_hook:
>>     stp        x29, x30, [sp, #-16]!
>>     mov        x29, sp
>>     bl        _init_vectors
>>     bl        _flat_map
>>     ldp        x29, x30, [sp], #16
>>     ret
>>
>> But still my point is that GDB should catch such an error and do not hang.
>>
>> Alex
>>
>> On Tue, Jul 31, 2018 at 9:34 PM, Alexander Fedotov <alfedotov@gmail.com> wrote:
>>> Hello dear AArch64 maintainers
>>> Please look into code snippet below from newlib/libgloss/aarch64/rdimon-aem-el3.
>>>
>>> Seems to me this code violates AArch64 calling convention and actually
>>> breaks debugging in GDB. GDB tries to unwind call stack and got
>>> endless reentrancy...
>>>
>>> FUNCTION (_cpu_init_hook):
>>>     sub    sp, sp, #16
>>>     str    x30, [sp, xzr]
>>>     bl    _init_vectors
>>>     bl    _flat_map
>>>     ldr    x30, [sp, xzr]
>>>     add    sp, sp, #16
>>>     ret
>>>
>>>
>>> We have couple of calls there (_init_vectors, _flat_map). If you'll
>>> try to step into any subroutine you will found that GDB hangs and
>>> can't step anymore.
>>>
>>> Pushing LR on the stack resolves a problem.
>
> X30 is LR.
>
> All we really need is a CFI unwind record that GDB can understand.
>
> R.
>
>>>
>>> So my message is that:
>>> 1. Current code in _cpu_init_hook is incorrect
>>> 2. GDB should handle this and do not hang
>>>
>>> Alex
>>
>>
>>
>



-- 
Best regards,
AF

Re: AArch64 calling convention in assembly code

[Richard Earnshaw (lists)]

[-- Attachment #1: Type: text/plain, Size: 2291 bytes --]

On 01/08/18 16:56, Alexander Fedotov wrote:
> Yes, but X29 is not saved in original version. I can understand this
> with 'frame-less functions" approach. So this code has a right for a
> life.
> 
> I suspect that such a problem could happen if user will pass
> "-fomit-frame-pointer" option to GCC as well.
> 
>>> All we really need is a CFI unwind record that GDB can understand.
> For plain assembly code ?
> 

Yep, see attached.  This also simplifies the entry/exit sequences slightly.


	* aarch64/cpu-init/rdimon-aem-el3.S (cpu_init_hook): Simplify
	entry/exit sequences.  Add CFI unwind rules.

R.

> Alex
> 
> On Wed, Aug 1, 2018 at 6:48 PM, Richard Earnshaw (lists)
> <Richard.Earnshaw@arm.com> wrote:
>> On 01/08/18 14:28, Alexander Fedotov wrote:
>>>>> Pushing LR on the stack resolves a problem
>>> FP of course, not LR.
>>> So the correct code must be like this:
>>>
>>> _cpu_init_hook:
>>>     stp        x29, x30, [sp, #-16]!
>>>     mov        x29, sp
>>>     bl        _init_vectors
>>>     bl        _flat_map
>>>     ldp        x29, x30, [sp], #16
>>>     ret
>>>
>>> But still my point is that GDB should catch such an error and do not hang.
>>>
>>> Alex
>>>
>>> On Tue, Jul 31, 2018 at 9:34 PM, Alexander Fedotov <alfedotov@gmail.com> wrote:
>>>> Hello dear AArch64 maintainers
>>>> Please look into code snippet below from newlib/libgloss/aarch64/rdimon-aem-el3.
>>>>
>>>> Seems to me this code violates AArch64 calling convention and actually
>>>> breaks debugging in GDB. GDB tries to unwind call stack and got
>>>> endless reentrancy...
>>>>
>>>> FUNCTION (_cpu_init_hook):
>>>>     sub    sp, sp, #16
>>>>     str    x30, [sp, xzr]
>>>>     bl    _init_vectors
>>>>     bl    _flat_map
>>>>     ldr    x30, [sp, xzr]
>>>>     add    sp, sp, #16
>>>>     ret
>>>>
>>>>
>>>> We have couple of calls there (_init_vectors, _flat_map). If you'll
>>>> try to step into any subroutine you will found that GDB hangs and
>>>> can't step anymore.
>>>>
>>>> Pushing LR on the stack resolves a problem.
>>
>> X30 is LR.
>>
>> All we really need is a CFI unwind record that GDB can understand.
>>
>> R.
>>
>>>>
>>>> So my message is that:
>>>> 1. Current code in _cpu_init_hook is incorrect
>>>> 2. GDB should handle this and do not hang
>>>>
>>>> Alex
>>>
>>>
>>>
>>
> 
> 
> 


[-- Attachment #2: rdimon-cfi.patch --]
[-- Type: text/x-diff, Size: 725 bytes --]

diff --git a/libgloss/aarch64/cpu-init/rdimon-aem-el3.S b/libgloss/aarch64/cpu-init/rdimon-aem-el3.S
index 41db8f9..9edbccd 100644
--- a/libgloss/aarch64/cpu-init/rdimon-aem-el3.S
+++ b/libgloss/aarch64/cpu-init/rdimon-aem-el3.S
@@ -157,12 +157,16 @@ ttb:
 	.align 2
 	.global	FUNCTION (_cpu_init_hook)
 	.type	FUNCTION (_cpu_init_hook), %function
+	.cfi_sections	.debug_frame
 FUNCTION (_cpu_init_hook):
-	sub	sp, sp, #16
-	str	x30, [sp, xzr]
+	.cfi_startproc
+	str	x30, [sp, -16]!
+	.cfi_def_cfa_offset 16
+	.cfi_offset 30, -16
 	bl	_init_vectors
 	bl	_flat_map
-	ldr	x30, [sp, xzr]
-	add	sp, sp, #16
+	ldr	x30, [sp], 16
+	.cfi_restore 30
 	ret
+	.cfi_endproc
 	.size	FUNCTION (_cpu_init_hook), .-FUNCTION (_cpu_init_hook)

Re: AArch64 calling convention in assembly code

[Jeff Johnston]

I have pushed Richard's patch to master.

-- Jeff J.

On Wed, Aug 1, 2018 at 12:19 PM, Richard Earnshaw (lists) <
Richard.Earnshaw@arm.com> wrote:

> On 01/08/18 16:56, Alexander Fedotov wrote:
> > Yes, but X29 is not saved in original version. I can understand this
> > with 'frame-less functions" approach. So this code has a right for a
> > life.
> >
> > I suspect that such a problem could happen if user will pass
> > "-fomit-frame-pointer" option to GCC as well.
> >
> >>> All we really need is a CFI unwind record that GDB can understand.
> > For plain assembly code ?
> >
>
> Yep, see attached.  This also simplifies the entry/exit sequences slightly.
>
>
>         * aarch64/cpu-init/rdimon-aem-el3.S (cpu_init_hook): Simplify
>         entry/exit sequences.  Add CFI unwind rules.
>
> R.
>
> > Alex
> >
> > On Wed, Aug 1, 2018 at 6:48 PM, Richard Earnshaw (lists)
> > <Richard.Earnshaw@arm.com> wrote:
> >> On 01/08/18 14:28, Alexander Fedotov wrote:
> >>>>> Pushing LR on the stack resolves a problem
> >>> FP of course, not LR.
> >>> So the correct code must be like this:
> >>>
> >>> _cpu_init_hook:
> >>>     stp        x29, x30, [sp, #-16]!
> >>>     mov        x29, sp
> >>>     bl        _init_vectors
> >>>     bl        _flat_map
> >>>     ldp        x29, x30, [sp], #16
> >>>     ret
> >>>
> >>> But still my point is that GDB should catch such an error and do not
> hang.
> >>>
> >>> Alex
> >>>
> >>> On Tue, Jul 31, 2018 at 9:34 PM, Alexander Fedotov <
> alfedotov@gmail.com> wrote:
> >>>> Hello dear AArch64 maintainers
> >>>> Please look into code snippet below from newlib/libgloss/aarch64/
> rdimon-aem-el3.
> >>>>
> >>>> Seems to me this code violates AArch64 calling convention and actually
> >>>> breaks debugging in GDB. GDB tries to unwind call stack and got
> >>>> endless reentrancy...
> >>>>
> >>>> FUNCTION (_cpu_init_hook):
> >>>>     sub    sp, sp, #16
> >>>>     str    x30, [sp, xzr]
> >>>>     bl    _init_vectors
> >>>>     bl    _flat_map
> >>>>     ldr    x30, [sp, xzr]
> >>>>     add    sp, sp, #16
> >>>>     ret
> >>>>
> >>>>
> >>>> We have couple of calls there (_init_vectors, _flat_map). If you'll
> >>>> try to step into any subroutine you will found that GDB hangs and
> >>>> can't step anymore.
> >>>>
> >>>> Pushing LR on the stack resolves a problem.
> >>
> >> X30 is LR.
> >>
> >> All we really need is a CFI unwind record that GDB can understand.
> >>
> >> R.
> >>
> >>>>
> >>>> So my message is that:
> >>>> 1. Current code in _cpu_init_hook is incorrect
> >>>> 2. GDB should handle this and do not hang
> >>>>
> >>>> Alex
> >>>
> >>>
> >>>
> >>
> >
> >
> >
>
>