* [Qemu-devel] [PATCH] linux-user: provide frame information in x86-64 safe_syscall
@ 2016-05-27 15:06 Peter Maydell
2016-05-27 16:21 ` Richard Henderson
2016-05-27 17:41 ` Peter Maydell
0 siblings, 2 replies; 5+ messages in thread
From: Peter Maydell @ 2016-05-27 15:06 UTC (permalink / raw)
To: qemu-devel; +Cc: patches, Riku Voipio, Richard Henderson
Use cfi directives in the x86-64 safe_syscall to allow gdb to get
backtraces right from within it. (In particular this will be
quite a common situation if the user interrupts QEMU while it's
in a blocked safe-syscall: at the point of the syscall insn RBP
is in use for something else, and so gdb can't find the frame then
without assistance.)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
The requirements for frame information annotations seem to be a bit
of an undocumented black art, but I think I have these right. At
any rate, gdb now gives correct backtraces from all points in
the routine as far as I can see. Review appreciated...
linux-user/host/x86_64/safe-syscall.inc.S | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/linux-user/host/x86_64/safe-syscall.inc.S b/linux-user/host/x86_64/safe-syscall.inc.S
index dde434c..bbb1eca 100644
--- a/linux-user/host/x86_64/safe-syscall.inc.S
+++ b/linux-user/host/x86_64/safe-syscall.inc.S
@@ -24,6 +24,7 @@
* -1-and-errno-set convention is done by the calling wrapper.
*/
safe_syscall_base:
+ .cfi_startproc
/* This saves a frame pointer and aligns the stack for the syscall.
* (It's unclear if the syscall ABI has the same stack alignment
* requirements as the userspace function call ABI, but better safe than
@@ -31,6 +32,8 @@ safe_syscall_base:
* does not list any ABI differences regarding stack alignment.)
*/
push %rbp
+ .cfi_def_cfa_offset 16
+ .cfi_offset rbp,-16
/* The syscall calling convention isn't the same as the
* C one:
@@ -70,12 +73,19 @@ safe_syscall_start:
safe_syscall_end:
/* code path for having successfully executed the syscall */
pop %rbp
+ .cfi_remember_state
+ .cfi_def_cfa_offset 8
+ .cfi_restore ebp
ret
return_ERESTARTSYS:
/* code path when we didn't execute the syscall */
+ .cfi_restore_state
mov $-TARGET_ERESTARTSYS, %rax
pop %rbp
+ .cfi_def_cfa_offset 8
+ .cfi_restore ebp
ret
+ .cfi_endproc
.size safe_syscall_base, .-safe_syscall_base
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: provide frame information in x86-64 safe_syscall
2016-05-27 15:06 [Qemu-devel] [PATCH] linux-user: provide frame information in x86-64 safe_syscall Peter Maydell
@ 2016-05-27 16:21 ` Richard Henderson
2016-05-27 16:34 ` Peter Maydell
2016-05-27 17:41 ` Peter Maydell
1 sibling, 1 reply; 5+ messages in thread
From: Richard Henderson @ 2016-05-27 16:21 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches, Riku Voipio
On 05/27/2016 08:06 AM, Peter Maydell wrote:
> @@ -31,6 +32,8 @@ safe_syscall_base:
> * does not list any ABI differences regarding stack alignment.)
> */
> push %rbp
> + .cfi_def_cfa_offset 16
> + .cfi_offset rbp,-16
While this is correct, there are two other directives that make it easier to
describe changes without having to compute globally correct constants. Here
they would be:
.cfi_adjust_cfa_offset 8
Add 8 to the offset, i.e. decrement the SP by 8.
.cfi_rel_offset rbp, 0
Save rbp at the current top-of-stack.
The assembler will compute the absolute values from these relative values.
Using them makes it easy to see from a narrow window of code that the
annotations are correct.
Otherwise,
Reviewed-by: Richard Henderson <rth@twiddle.net>
r~
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: provide frame information in x86-64 safe_syscall
2016-05-27 16:21 ` Richard Henderson
@ 2016-05-27 16:34 ` Peter Maydell
2016-05-27 21:15 ` Richard Henderson
0 siblings, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2016-05-27 16:34 UTC (permalink / raw)
To: Richard Henderson; +Cc: QEMU Developers, Patch Tracking, Riku Voipio
On 27 May 2016 at 17:21, Richard Henderson <rth@twiddle.net> wrote:
> On 05/27/2016 08:06 AM, Peter Maydell wrote:
>>
>> @@ -31,6 +32,8 @@ safe_syscall_base:
>> * does not list any ABI differences regarding stack alignment.)
>> */
>> push %rbp
>> + .cfi_def_cfa_offset 16
>> + .cfi_offset rbp,-16
>
>
> While this is correct, there are two other directives that make it easier to
> describe changes without having to compute globally correct constants. Here
> they would be:
>
> .cfi_adjust_cfa_offset 8
>
> Add 8 to the offset, i.e. decrement the SP by 8.
Presumably .cfi_startproc sets the initial offset to 8?
(It's not documented that it does so, which is I think partly why
I preferred to use a directive that definitely set the offset
to the right thing.)
> .cfi_rel_offset rbp, 0
>
> Save rbp at the current top-of-stack.
>
> The assembler will compute the absolute values from these relative values.
> Using them makes it easy to see from a narrow window of code that the
> annotations are correct.
>
> Otherwise,
>
> Reviewed-by: Richard Henderson <rth@twiddle.net>
Thanks. I'll spin a v2 with your changes in it next week.
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: provide frame information in x86-64 safe_syscall
2016-05-27 15:06 [Qemu-devel] [PATCH] linux-user: provide frame information in x86-64 safe_syscall Peter Maydell
2016-05-27 16:21 ` Richard Henderson
@ 2016-05-27 17:41 ` Peter Maydell
1 sibling, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2016-05-27 17:41 UTC (permalink / raw)
To: QEMU Developers; +Cc: Richard Henderson, Riku Voipio, Patch Tracking
On 27 May 2016 at 16:06, Peter Maydell <peter.maydell@linaro.org> wrote:
> return_ERESTARTSYS:
> /* code path when we didn't execute the syscall */
> + .cfi_restore_state
> mov $-TARGET_ERESTARTSYS, %rax
> pop %rbp
> + .cfi_def_cfa_offset 8
> + .cfi_restore ebp
These should read '.cfi_restore rbp'. I have no idea how that got through
testing since it doesn't compile at all...
thanks
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: provide frame information in x86-64 safe_syscall
2016-05-27 16:34 ` Peter Maydell
@ 2016-05-27 21:15 ` Richard Henderson
0 siblings, 0 replies; 5+ messages in thread
From: Richard Henderson @ 2016-05-27 21:15 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers, Patch Tracking, Riku Voipio
On 05/27/2016 09:34 AM, Peter Maydell wrote:
> On 27 May 2016 at 17:21, Richard Henderson <rth@twiddle.net> wrote:
>> On 05/27/2016 08:06 AM, Peter Maydell wrote:
>>>
>>> @@ -31,6 +32,8 @@ safe_syscall_base:
>>> * does not list any ABI differences regarding stack alignment.)
>>> */
>>> push %rbp
>>> + .cfi_def_cfa_offset 16
>>> + .cfi_offset rbp,-16
>>
>>
>> While this is correct, there are two other directives that make it easier to
>> describe changes without having to compute globally correct constants. Here
>> they would be:
>>
>> .cfi_adjust_cfa_offset 8
>>
>> Add 8 to the offset, i.e. decrement the SP by 8.
>
> Presumably .cfi_startproc sets the initial offset to 8?
> (It's not documented that it does so, which is I think partly why
> I preferred to use a directive that definitely set the offset
> to the right thing.)
It is documented to set up the normal no-instructions-executed call frame.
Which in the case of x86, does have a non-zero offset.
There is a ".cfi_startproc simple" that begins a frame with no opcodes at all.
r~
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-05-27 21:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-27 15:06 [Qemu-devel] [PATCH] linux-user: provide frame information in x86-64 safe_syscall Peter Maydell
2016-05-27 16:21 ` Richard Henderson
2016-05-27 16:34 ` Peter Maydell
2016-05-27 21:15 ` Richard Henderson
2016-05-27 17:41 ` Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.