Re: [PATCH, libitm]: GTM_longjmp: Jump indirect from memory address

2012-02-15 Thread Uros Bizjak
On Tue, Feb 14, 2012 at 8:52 PM, Richard Henderson r...@redhat.com wrote:
 On 02/14/2012 10:26 AM, Uros Bizjak wrote:
  #ifdef __x86_64__
 +     cfi_def_cfa(%rsi, 0)
       movq    (%rsi), %rcx
       movq    8(%rsi), %rbx
       movq    16(%rsi), %rbp
 @@ -119,20 +120,21 @@
       movq    32(%rsi), %r13
       movq    40(%rsi), %r14
       movq    48(%rsi), %r15
 +     cfi_offset(%rip, 56)
       movl    %edi, %eax
 -     cfi_offset(%rip, 56)
 -     cfi_def_cfa(%rcx, 0)
       movq    %rcx, %rsp
 +     cfi_register(%rsp, %rcx)
       jmp     *56(%rsi)

 No, your changes are incorrect.  All three markers needed to be together
 in order to provide a consistent and coherent unwind.  What you committed
 was much worse than simply not describing anything at all.

 Fixed as below.  Committed.

Thanks!

Uros.


Re: [PATCH, libitm]: GTM_longjmp: Jump indirect from memory address

2012-02-14 Thread Richard Henderson
On 02/13/2012 11:54 PM, Uros Bizjak wrote:
   movq48(%rsi), %r15
 - movq56(%rsi), %rdx
   movl%edi, %eax
 + cfi_offset(%rip, 56)
   cfi_def_cfa(%rcx, 0)
 - cfi_register(%rip, %rdx)
   movq%rcx, %rsp
 - jmp *%rdx
 + jmp *56(%rsi)

I'm sorry, I was mistaken earlier.  The rip value is not at CFA+56, it's at 
RSI+56.
The way to describe this is

cfi_def_cfa(%rsi, 0)
cfi_offset(%rip, 56)
cfi_register(%rsp, %rcx)


r~


Re: [PATCH, libitm]: GTM_longjmp: Jump indirect from memory address

2012-02-14 Thread Uros Bizjak
On Tue, Feb 14, 2012 at 7:07 PM, Richard Henderson r...@redhat.com wrote:
 On 02/13/2012 11:54 PM, Uros Bizjak wrote:
       movq    48(%rsi), %r15
 -     movq    56(%rsi), %rdx
       movl    %edi, %eax
 +     cfi_offset(%rip, 56)
       cfi_def_cfa(%rcx, 0)
 -     cfi_register(%rip, %rdx)
       movq    %rcx, %rsp
 -     jmp     *%rdx
 +     jmp     *56(%rsi)

 I'm sorry, I was mistaken earlier.  The rip value is not at CFA+56, it's at 
 RSI+56.
 The way to describe this is

        cfi_def_cfa(%rsi, 0)
        cfi_offset(%rip, 56)
        cfi_register(%rsp, %rcx)

Yes, IMO this now describes correct CFA handling. Following follow-on
patch corrects this issue (and also puts .cfi directions to the place
where they make most sense, mainly a cosmetic change).

Re-tested on x86_64-pc-linux-gnu {,-m32} and committed.

Thanks,
Uros.
Index: config/x86/sjlj.S
===
--- config/x86/sjlj.S   (revision 184213)
+++ config/x86/sjlj.S   (working copy)
@@ -112,6 +112,7 @@
 SYM(GTM_longjmp):
cfi_startproc
 #ifdef __x86_64__
+   cfi_def_cfa(%rsi, 0)
movq(%rsi), %rcx
movq8(%rsi), %rbx
movq16(%rsi), %rbp
@@ -119,20 +120,21 @@
movq32(%rsi), %r13
movq40(%rsi), %r14
movq48(%rsi), %r15
+   cfi_offset(%rip, 56)
movl%edi, %eax
-   cfi_offset(%rip, 56)
-   cfi_def_cfa(%rcx, 0)
movq%rcx, %rsp
+   cfi_register(%rsp, %rcx)
jmp *56(%rsi)
 #else
+   cfi_def_cfa(%edx, 0)
movl(%edx), %ecx
movl4(%edx), %ebx
movl8(%edx), %esi
movl12(%edx), %edi
movl16(%edx), %ebp
cfi_offset(%eip, 20)
-   cfi_def_cfa(%ecx, 0)
movl%ecx, %esp
+   cfi_register(%esp, %ecx)
jmp *20(%edx)
 #endif
cfi_endproc


[PATCH, libitm]: GTM_longjmp: Jump indirect from memory address

2012-02-13 Thread Uros Bizjak
Hello!

We can jump indirect from memory address, sparing a couple of cycles.

2012-02-14  Uros Bizjak  ubiz...@gmail.com

* config/x86/target.h (GTM_longjmp): Jump indirect from memory address.

Tested on x86_64-pc-linux-gnu {,-m32}.

OK for mainline?

Uros.
Index: config/x86/sjlj.S
===
--- config/x86/sjlj.S   (revision 184177)
+++ config/x86/sjlj.S   (working copy)
@@ -119,23 +119,19 @@ SYM(GTM_longjmp):
movq32(%rsi), %r13
movq40(%rsi), %r14
movq48(%rsi), %r15
-   movq56(%rsi), %rdx
movl%edi, %eax
cfi_def_cfa(%rcx, 0)
-   cfi_register(%rip, %rdx)
movq%rcx, %rsp
-   jmp *%rdx
+   jmp *56(%rsi)
 #else
movl(%edx), %ecx
movl4(%edx), %ebx
movl8(%edx), %esi
movl12(%edx), %edi
movl16(%edx), %ebp
-   movl20(%edx), %edx
cfi_def_cfa(%ecx, 0)
-   cfi_register(%eip, %edx)
movl%ecx, %esp
-   jmp *%edx
+   jmp *20(%edx)
 #endif
cfi_endproc
 


Re: [PATCH, libitm]: GTM_longjmp: Jump indirect from memory address

2012-02-13 Thread Richard Henderson
On 02/13/2012 02:54 PM, Uros Bizjak wrote:
 - movq56(%rsi), %rdx
   movl%edi, %eax
   cfi_def_cfa(%rcx, 0)
 - cfi_register(%rip, %rdx)
   movq%rcx, %rsp
 - jmp *%rdx
 + jmp *56(%rsi)

If you're going to do that, the correct fix for the unwind info is 

- cfi_register(%rip, %rdx)
+ cfi_offset(%rip, 56)

Otherwise ok.


r~


Re: [PATCH, libitm]: GTM_longjmp: Jump indirect from memory address

2012-02-13 Thread Uros Bizjak
On Mon, Feb 13, 2012 at 11:57 PM, Richard Henderson r...@redhat.com wrote:
 On 02/13/2012 02:54 PM, Uros Bizjak wrote:
 -     movq    56(%rsi), %rdx
       movl    %edi, %eax
       cfi_def_cfa(%rcx, 0)
 -     cfi_register(%rip, %rdx)
       movq    %rcx, %rsp
 -     jmp     *%rdx
 +     jmp     *56(%rsi)

 If you're going to do that, the correct fix for the unwind info is

 - cfi_register(%rip, %rdx)
 + cfi_offset(%rip, 56)

Hm, we just defined new CFA as rcx+0, so we should define location of
rip relative to new CFA. Since CFA points to stack slot just before
return address was pushed, new rip lies at CFA-8 for 64bit resp. CFA-4
for x86_32. Did I get these .cfi directives correctly?

SYM(GTM_longjmp):
cfi_startproc
#ifdef __x86_64__
movq(%rsi), %rcx
movq8(%rsi), %rbx
movq16(%rsi), %rbp
movq24(%rsi), %r12
movq32(%rsi), %r13
movq40(%rsi), %r14
movq48(%rsi), %r15
movl%edi, %eax
cfi_def_cfa(%rcx, 0)
cfi_offset(%rip, -8)
movq%rcx, %rsp
jmp *56(%rsi)
#else
movl(%edx), %ecx
movl4(%edx), %ebx
movl8(%edx), %esi
movl12(%edx), %edi
movl16(%edx), %ebp
cfi_def_cfa(%ecx, 0)
cfi_offset(%eip, -4)
movl%ecx, %esp
jmp *20(%edx)
#endif
cfi_endproc

Uros.


Re: [PATCH, libitm]: GTM_longjmp: Jump indirect from memory address

2012-02-13 Thread Richard Henderson
On 02/13/2012 04:09 PM, Uros Bizjak wrote:
 On Mon, Feb 13, 2012 at 11:57 PM, Richard Henderson r...@redhat.com wrote:
 On 02/13/2012 02:54 PM, Uros Bizjak wrote:
 - movq56(%rsi), %rdx
   movl%edi, %eax
   cfi_def_cfa(%rcx, 0)
 - cfi_register(%rip, %rdx)
   movq%rcx, %rsp
 - jmp *%rdx
 + jmp *56(%rsi)

 If you're going to do that, the correct fix for the unwind info is

 - cfi_register(%rip, %rdx)
 + cfi_offset(%rip, 56)
 
 Hm, we just defined new CFA as rcx+0, so we should define location of
 rip relative to new CFA. Since CFA points to stack slot just before
 return address was pushed, new rip lies at CFA-8 for 64bit resp. CFA-4
 for x86_32. Did I get these .cfi directives correctly?

No.  The value at %rcx-8 is total garbage.  There no guarantee that
the call stack leading to this abort has anything in common with the
call stack that created the jmpbuf, except *above* %rcx, the new CFA.

The new rip is at rsi+56.  You can see that in that you jump to it.


r~


Re: [PATCH, libitm]: GTM_longjmp: Jump indirect from memory address

2012-02-13 Thread Uros Bizjak
On Tue, Feb 14, 2012 at 1:15 AM, Richard Henderson r...@redhat.com wrote:

 -     movq    56(%rsi), %rdx
       movl    %edi, %eax
       cfi_def_cfa(%rcx, 0)
 -     cfi_register(%rip, %rdx)
       movq    %rcx, %rsp
 -     jmp     *%rdx
 +     jmp     *56(%rsi)

 If you're going to do that, the correct fix for the unwind info is

 - cfi_register(%rip, %rdx)
 + cfi_offset(%rip, 56)

 Hm, we just defined new CFA as rcx+0, so we should define location of
 rip relative to new CFA. Since CFA points to stack slot just before
 return address was pushed, new rip lies at CFA-8 for 64bit resp. CFA-4
 for x86_32. Did I get these .cfi directives correctly?

 No.  The value at %rcx-8 is total garbage.  There no guarantee that
 the call stack leading to this abort has anything in common with the
 call stack that created the jmpbuf, except *above* %rcx, the new CFA.

 The new rip is at rsi+56.  You can see that in that you jump to it.

Thanks for the explanation, I will commit the patch with your suggested change.

Uros.


Re: [PATCH, libitm]: GTM_longjmp: Jump indirect from memory address

2012-02-13 Thread Uros Bizjak
On Tue, Feb 14, 2012 at 8:39 AM, Uros Bizjak ubiz...@gmail.com wrote:

 - cfi_register(%rip, %rdx)
 + cfi_offset(%rip, 56)

 Hm, we just defined new CFA as rcx+0, so we should define location of
 rip relative to new CFA. Since CFA points to stack slot just before
 return address was pushed, new rip lies at CFA-8 for 64bit resp. CFA-4
 for x86_32. Did I get these .cfi directives correctly?

 No.  The value at %rcx-8 is total garbage.  There no guarantee that
 the call stack leading to this abort has anything in common with the
 call stack that created the jmpbuf, except *above* %rcx, the new CFA.

 The new rip is at rsi+56.  You can see that in that you jump to it.

 Thanks for the explanation, I will commit the patch with your suggested 
 change.

Now with the patch attached... (please also note that rip is now
defined with offset to old CFA, before CFA is updated to new
register).

Uros.
Index: ChangeLog
===
--- ChangeLog   (revision 184197)
+++ ChangeLog   (working copy)
@@ -1,3 +1,7 @@
+2012-02-15  Uros Bizjak  ubiz...@gmail.com
+
+   * config/x86/target.h (GTM_longjmp): Jump indirect from memory address.
+
 2012-02-13  Eric Botcazou  ebotca...@adacore.com
 
* configure.tgt (target_cpu): Handle sparc and sparc64  sparcv9.
Index: config/x86/sjlj.S
===
--- config/x86/sjlj.S   (revision 184150)
+++ config/x86/sjlj.S   (working copy)
@@ -119,23 +119,21 @@
movq32(%rsi), %r13
movq40(%rsi), %r14
movq48(%rsi), %r15
-   movq56(%rsi), %rdx
movl%edi, %eax
+   cfi_offset(%rip, 56)
cfi_def_cfa(%rcx, 0)
-   cfi_register(%rip, %rdx)
movq%rcx, %rsp
-   jmp *%rdx
+   jmp *56(%rsi)
 #else
movl(%edx), %ecx
movl4(%edx), %ebx
movl8(%edx), %esi
movl12(%edx), %edi
movl16(%edx), %ebp
-   movl20(%edx), %edx
+   cfi_offset(%eip, 20)
cfi_def_cfa(%ecx, 0)
-   cfi_register(%eip, %edx)
movl%ecx, %esp
-   jmp *%edx
+   jmp *20(%edx)
 #endif
cfi_endproc