Re: AArch64 calling convention in assembly code

["Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com>]

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