unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] powerpc64[le]: Fix CFI for assembly templated syscalls [BZ #28532]
@ 2021-11-03 14:22 Matheus Castanho via Libc-alpha
  2021-11-03 15:46 ` Paul E Murphy via Libc-alpha
  0 siblings, 1 reply; 7+ messages in thread
From: Matheus Castanho via Libc-alpha @ 2021-11-03 14:22 UTC (permalink / raw)
  To: libc-alpha

Syscalls based on the assembly templates are missing CFI for r31, which gets
clobbered when scv is used, and info for LR is inaccurate, placed in the wrong
LOC and not using the proper offset. These are fixed by this commit.

After this change:

$ readelf -wF libc.so.6 | grep 0004b9d4.. -A 7 && objdump --disassemble=kill libc.so.6
00004a48 0000000000000020 00004a4c FDE cie=00000000 pc=000000000004b9d4..000000000004ba3c
   LOC           CFA      r31   ra
000000000004b9d4 r1+0     u     u
000000000004b9e4 r1+48    u     u
000000000004b9e8 r1+48    c-16  u
000000000004b9fc r1+48    c-16  c-32
000000000004ba08 r1+48    c-16
000000000004ba18 r1+48    u
000000000004ba1c r1+0     u

libc.so.6:     file format elf64-powerpcle

Disassembly of section .text:

000000000004b9d4 <kill>:
   4b9d4:       1f 00 4c 3c     addis   r2,r12,31
   4b9d8:       2c c3 42 38     addi    r2,r2,-15572
   4b9dc:       25 00 00 38     li      r0,37
   4b9e0:       d1 ff 21 f8     stdu    r1,-48(r1)
   4b9e4:       20 00 e1 fb     std     r31,32(r1)
   4b9e8:       98 8f ed eb     ld      r31,-28776(r13)
   4b9ec:       10 00 ff 77     andis.  r31,r31,16
   4b9f0:       1c 00 82 41     beq     4ba0c <kill+0x38>
   4b9f4:       a6 02 28 7d     mflr    r9
   4b9f8:       10 00 21 f9     std     r9,16(r1)
   4b9fc:       01 00 00 44     scv     0
   4ba00:       10 00 21 e9     ld      r9,16(r1)
   4ba04:       a6 03 28 7d     mtlr    r9
   4ba08:       08 00 00 48     b       4ba10 <kill+0x3c>
   4ba0c:       02 00 00 44     sc
   4ba10:       00 00 bf 2e     cmpdi   cr5,r31,0
   4ba14:       20 00 e1 eb     ld      r31,32(r1)
   4ba18:       30 00 21 38     addi    r1,r1,48
   4ba1c:       18 00 96 41     beq     cr5,4ba34 <kill+0x60>
   4ba20:       01 f0 20 39     li      r9,-4095
   4ba24:       40 48 23 7c     cmpld   r3,r9
   4ba28:       20 00 e0 4d     bltlr+
   4ba2c:       d0 00 63 7c     neg     r3,r3
   4ba30:       08 00 00 48     b       4ba38 <kill+0x64>
   4ba34:       20 00 e3 4c     bnslr+
   4ba38:       c8 32 fe 4b     b       2ed00 <__syscall_error>
        ...
   4ba44:       40 20 0c 00     .long 0xc2040
   4ba48:       68 00 00 00     .long 0x68
   4ba4c:       06 00 5f 5f     rlwnm   r31,r26,r0,0,3
   4ba50:       6b 69 6c 6c     xoris   r12,r3,26987
---
 sysdeps/powerpc/powerpc64/sysdep.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h
index 589f7c8d18..beb477d57b 100644
--- a/sysdeps/powerpc/powerpc64/sysdep.h
+++ b/sysdeps/powerpc/powerpc64/sysdep.h
@@ -275,12 +275,14 @@ LT_LABELSUFFIX(name,_name_end): ; \
 /* Allocate frame and save register */
 #define NVOLREG_SAVE \
     stdu r1,-SCV_FRAME_SIZE(r1); \
+    cfi_adjust_cfa_offset(SCV_FRAME_SIZE); \
     std r31,SCV_FRAME_NVOLREG_SAVE(r1); \
-    cfi_adjust_cfa_offset(SCV_FRAME_SIZE);
+    cfi_offset(r31,-(SCV_FRAME_SIZE-SCV_FRAME_NVOLREG_SAVE));
 
 /* Restore register and destroy frame */
 #define NVOLREG_RESTORE	\
     ld r31,SCV_FRAME_NVOLREG_SAVE(r1); \
+    cfi_restore(r31); \
     addi r1,r1,SCV_FRAME_SIZE; \
     cfi_adjust_cfa_offset(-SCV_FRAME_SIZE);
 
@@ -332,7 +334,7 @@ LT_LABELSUFFIX(name,_name_end): ; \
 #define DO_CALL_SCV \
     mflr r9; \
     std r9,FRAME_LR_SAVE(r1); \
-    cfi_offset(lr,FRAME_LR_SAVE); \
+    cfi_offset(lr,-(SCV_FRAME_SIZE-FRAME_LR_SAVE)); \
     .machine "push"; \
     .machine "power9"; \
     scv 0; \
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] powerpc64[le]: Fix CFI for assembly templated syscalls [BZ #28532]
  2021-11-03 14:22 [PATCH] powerpc64[le]: Fix CFI for assembly templated syscalls [BZ #28532] Matheus Castanho via Libc-alpha
@ 2021-11-03 15:46 ` Paul E Murphy via Libc-alpha
  2021-11-03 16:41   ` Matheus Castanho via Libc-alpha
  0 siblings, 1 reply; 7+ messages in thread
From: Paul E Murphy via Libc-alpha @ 2021-11-03 15:46 UTC (permalink / raw)
  To: Matheus Castanho, libc-alpha



On 11/3/21 9:22 AM, Matheus Castanho via Libc-alpha wrote:
> Syscalls based on the assembly templates are missing CFI for r31, which gets
> clobbered when scv is used, and info for LR is inaccurate, placed in the wrong
> LOC and not using the proper offset. These are fixed by this commit.
> 
> After this change:
> 
> $ readelf -wF libc.so.6 | grep 0004b9d4.. -A 7 && objdump --disassemble=kill libc.so.6
> 00004a48 0000000000000020 00004a4c FDE cie=00000000 pc=000000000004b9d4..000000000004ba3c
>     LOC           CFA      r31   ra
> 000000000004b9d4 r1+0     u     u
> 000000000004b9e4 r1+48    u     u
> 000000000004b9e8 r1+48    c-16  u
> 000000000004b9fc r1+48    c-16  c-32
> 000000000004ba08 r1+48    c-16
> 000000000004ba18 r1+48    u
> 000000000004ba1c r1+0     u
> 
> libc.so.6:     file format elf64-powerpcle
> 
> Disassembly of section .text:
> 
> 000000000004b9d4 <kill>:
>     4b9d4:       1f 00 4c 3c     addis   r2,r12,31
>     4b9d8:       2c c3 42 38     addi    r2,r2,-15572
>     4b9dc:       25 00 00 38     li      r0,37
>     4b9e0:       d1 ff 21 f8     stdu    r1,-48(r1)
>     4b9e4:       20 00 e1 fb     std     r31,32(r1)
>     4b9e8:       98 8f ed eb     ld      r31,-28776(r13)
>     4b9ec:       10 00 ff 77     andis.  r31,r31,16
>     4b9f0:       1c 00 82 41     beq     4ba0c <kill+0x38>
>     4b9f4:       a6 02 28 7d     mflr    r9
>     4b9f8:       10 00 21 f9     std     r9,16(r1)
>     4b9fc:       01 00 00 44     scv     0
>     4ba00:       10 00 21 e9     ld      r9,16(r1)
>     4ba04:       a6 03 28 7d     mtlr    r9
>     4ba08:       08 00 00 48     b       4ba10 <kill+0x3c>
>     4ba0c:       02 00 00 44     sc
>     4ba10:       00 00 bf 2e     cmpdi   cr5,r31,0
>     4ba14:       20 00 e1 eb     ld      r31,32(r1)
>     4ba18:       30 00 21 38     addi    r1,r1,48
>     4ba1c:       18 00 96 41     beq     cr5,4ba34 <kill+0x60>
>     4ba20:       01 f0 20 39     li      r9,-4095
>     4ba24:       40 48 23 7c     cmpld   r3,r9
>     4ba28:       20 00 e0 4d     bltlr+
>     4ba2c:       d0 00 63 7c     neg     r3,r3
>     4ba30:       08 00 00 48     b       4ba38 <kill+0x64>
>     4ba34:       20 00 e3 4c     bnslr+
>     4ba38:       c8 32 fe 4b     b       2ed00 <__syscall_error>
>          ...
>     4ba44:       40 20 0c 00     .long 0xc2040
>     4ba48:       68 00 00 00     .long 0x68
>     4ba4c:       06 00 5f 5f     rlwnm   r31,r26,r0,0,3
>     4ba50:       6b 69 6c 6c     xoris   r12,r3,26987
> ---
>   sysdeps/powerpc/powerpc64/sysdep.h | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h
> index 589f7c8d18..beb477d57b 100644
> --- a/sysdeps/powerpc/powerpc64/sysdep.h
> +++ b/sysdeps/powerpc/powerpc64/sysdep.h
> @@ -275,12 +275,14 @@ LT_LABELSUFFIX(name,_name_end): ; \
>   /* Allocate frame and save register */
>   #define NVOLREG_SAVE \
>       stdu r1,-SCV_FRAME_SIZE(r1); \
> +    cfi_adjust_cfa_offset(SCV_FRAME_SIZE); \
>       std r31,SCV_FRAME_NVOLREG_SAVE(r1); \
> -    cfi_adjust_cfa_offset(SCV_FRAME_SIZE);
> +    cfi_offset(r31,-(SCV_FRAME_SIZE-SCV_FRAME_NVOLREG_SAVE));
>OK. This (and similarly the case below) seem a little obfuscated for 
computing stack save slots. I wonder if a distinct macro like 
SCV_FRAME_R31_SAVE_SLOT (and similar SCV_FRAME_CALLER_LR_SAVE_SLOT 
below) would make this code more approachable for those less familiar 
with ppc64 abis.

>   /* Restore register and destroy frame */
>   #define NVOLREG_RESTORE	\
>       ld r31,SCV_FRAME_NVOLREG_SAVE(r1); \
> +    cfi_restore(r31); \
>       addi r1,r1,SCV_FRAME_SIZE; \
>       cfi_adjust_cfa_offset(-SCV_FRAME_SIZE);
> 
> @@ -332,7 +334,7 @@ LT_LABELSUFFIX(name,_name_end): ; \
>   #define DO_CALL_SCV \
>       mflr r9; \
>       std r9,FRAME_LR_SAVE(r1); \
> -    cfi_offset(lr,FRAME_LR_SAVE); \
> +    cfi_offset(lr,-(SCV_FRAME_SIZE-FRAME_LR_SAVE)) >       .machine "push"; \
>       .machine "power9"; \
>       scv 0; \
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] powerpc64[le]: Fix CFI for assembly templated syscalls [BZ #28532]
  2021-11-03 15:46 ` Paul E Murphy via Libc-alpha
@ 2021-11-03 16:41   ` Matheus Castanho via Libc-alpha
  2021-11-04 22:36     ` Paul E Murphy via Libc-alpha
  0 siblings, 1 reply; 7+ messages in thread
From: Matheus Castanho via Libc-alpha @ 2021-11-03 16:41 UTC (permalink / raw)
  To: Paul E Murphy; +Cc: libc-alpha


Paul E Murphy <murphyp@linux.ibm.com> writes:

> On 11/3/21 9:22 AM, Matheus Castanho via Libc-alpha wrote:
>> Syscalls based on the assembly templates are missing CFI for r31, which gets
>> clobbered when scv is used, and info for LR is inaccurate, placed in the wrong
>> LOC and not using the proper offset. These are fixed by this commit.
>> After this change:
>> $ readelf -wF libc.so.6 | grep 0004b9d4.. -A 7 && objdump --disassemble=kill
>> libc.so.6
>> 00004a48 0000000000000020 00004a4c FDE cie=00000000 pc=000000000004b9d4..000000000004ba3c
>>     LOC           CFA      r31   ra
>> 000000000004b9d4 r1+0     u     u
>> 000000000004b9e4 r1+48    u     u
>> 000000000004b9e8 r1+48    c-16  u
>> 000000000004b9fc r1+48    c-16  c-32
>> 000000000004ba08 r1+48    c-16
>> 000000000004ba18 r1+48    u
>> 000000000004ba1c r1+0     u
>> libc.so.6:     file format elf64-powerpcle
>> Disassembly of section .text:
>> 000000000004b9d4 <kill>:
>>     4b9d4:       1f 00 4c 3c     addis   r2,r12,31
>>     4b9d8:       2c c3 42 38     addi    r2,r2,-15572
>>     4b9dc:       25 00 00 38     li      r0,37
>>     4b9e0:       d1 ff 21 f8     stdu    r1,-48(r1)
>>     4b9e4:       20 00 e1 fb     std     r31,32(r1)
>>     4b9e8:       98 8f ed eb     ld      r31,-28776(r13)
>>     4b9ec:       10 00 ff 77     andis.  r31,r31,16
>>     4b9f0:       1c 00 82 41     beq     4ba0c <kill+0x38>
>>     4b9f4:       a6 02 28 7d     mflr    r9
>>     4b9f8:       10 00 21 f9     std     r9,16(r1)
>>     4b9fc:       01 00 00 44     scv     0
>>     4ba00:       10 00 21 e9     ld      r9,16(r1)
>>     4ba04:       a6 03 28 7d     mtlr    r9
>>     4ba08:       08 00 00 48     b       4ba10 <kill+0x3c>
>>     4ba0c:       02 00 00 44     sc
>>     4ba10:       00 00 bf 2e     cmpdi   cr5,r31,0
>>     4ba14:       20 00 e1 eb     ld      r31,32(r1)
>>     4ba18:       30 00 21 38     addi    r1,r1,48
>>     4ba1c:       18 00 96 41     beq     cr5,4ba34 <kill+0x60>
>>     4ba20:       01 f0 20 39     li      r9,-4095
>>     4ba24:       40 48 23 7c     cmpld   r3,r9
>>     4ba28:       20 00 e0 4d     bltlr+
>>     4ba2c:       d0 00 63 7c     neg     r3,r3
>>     4ba30:       08 00 00 48     b       4ba38 <kill+0x64>
>>     4ba34:       20 00 e3 4c     bnslr+
>>     4ba38:       c8 32 fe 4b     b       2ed00 <__syscall_error>
>>          ...
>>     4ba44:       40 20 0c 00     .long 0xc2040
>>     4ba48:       68 00 00 00     .long 0x68
>>     4ba4c:       06 00 5f 5f     rlwnm   r31,r26,r0,0,3
>>     4ba50:       6b 69 6c 6c     xoris   r12,r3,26987
>> ---
>>   sysdeps/powerpc/powerpc64/sysdep.h | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>> diff --git a/sysdeps/powerpc/powerpc64/sysdep.h
>> b/sysdeps/powerpc/powerpc64/sysdep.h
>> index 589f7c8d18..beb477d57b 100644
>> --- a/sysdeps/powerpc/powerpc64/sysdep.h
>> +++ b/sysdeps/powerpc/powerpc64/sysdep.h
>> @@ -275,12 +275,14 @@ LT_LABELSUFFIX(name,_name_end): ; \
>>   /* Allocate frame and save register */
>>   #define NVOLREG_SAVE \
>>       stdu r1,-SCV_FRAME_SIZE(r1); \
>> +    cfi_adjust_cfa_offset(SCV_FRAME_SIZE); \
>>       std r31,SCV_FRAME_NVOLREG_SAVE(r1); \
>> -    cfi_adjust_cfa_offset(SCV_FRAME_SIZE);
>> +    cfi_offset(r31,-(SCV_FRAME_SIZE-SCV_FRAME_NVOLREG_SAVE));
>> OK. This (and similarly the case below) seem a little obfuscated for
> computing stack save slots. I wonder if a distinct macro like
> SCV_FRAME_R31_SAVE_SLOT (and similar SCV_FRAME_CALLER_LR_SAVE_SLOT
> below) would make this code more approachable for those less familiar with ppc64
> abis.
>

But those kind of already exist: FRAME_LR_SAVE and
SCV_FRAME_NVOLREG_SAVE, but these two values are relative to the
top-most frame, so we need some math to get the values relative to the
CFA.

Wouldn't adding 2 more macros with similar meanings (but with different
references) make things even more complicated?

>>   /* Restore register and destroy frame */
>>   #define NVOLREG_RESTORE	\
>>       ld r31,SCV_FRAME_NVOLREG_SAVE(r1); \
>> +    cfi_restore(r31); \
>>       addi r1,r1,SCV_FRAME_SIZE; \
>>       cfi_adjust_cfa_offset(-SCV_FRAME_SIZE);
>> @@ -332,7 +334,7 @@ LT_LABELSUFFIX(name,_name_end): ; \
>>   #define DO_CALL_SCV \
>>       mflr r9; \
>>       std r9,FRAME_LR_SAVE(r1); \
>> -    cfi_offset(lr,FRAME_LR_SAVE); \
>> +    cfi_offset(lr,-(SCV_FRAME_SIZE-FRAME_LR_SAVE)) >       .machine "push"; \
>>       .machine "power9"; \
>>       scv 0; \
>>


--
Matheus Castanho

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] powerpc64[le]: Fix CFI for assembly templated syscalls [BZ #28532]
  2021-11-03 16:41   ` Matheus Castanho via Libc-alpha
@ 2021-11-04 22:36     ` Paul E Murphy via Libc-alpha
  2021-11-24 13:29       ` [PATCH v2] powerpc64[le]: Fix CFI and LR save address for asm " Matheus Castanho via Libc-alpha
  0 siblings, 1 reply; 7+ messages in thread
From: Paul E Murphy via Libc-alpha @ 2021-11-04 22:36 UTC (permalink / raw)
  To: Matheus Castanho; +Cc: libc-alpha



On 11/3/21 11:41 AM, Matheus Castanho wrote:
> 
> Paul E Murphy <murphyp@linux.ibm.com> writes:
> 
>> On 11/3/21 9:22 AM, Matheus Castanho via Libc-alpha wrote:
>>> Syscalls based on the assembly templates are missing CFI for r31, which gets
>>> clobbered when scv is used, and info for LR is inaccurate, placed in the wrong
>>> LOC and not using the proper offset. These are fixed by this commit.
>>> After this change:
>>> $ readelf -wF libc.so.6 | grep 0004b9d4.. -A 7 && objdump --disassemble=kill
>>> libc.so.6
>>> 00004a48 0000000000000020 00004a4c FDE cie=00000000 pc=000000000004b9d4..000000000004ba3c
>>>      LOC           CFA      r31   ra
>>> 000000000004b9d4 r1+0     u     u
>>> 000000000004b9e4 r1+48    u     u
>>> 000000000004b9e8 r1+48    c-16  u
>>> 000000000004b9fc r1+48    c-16  c-32
>>> 000000000004ba08 r1+48    c-16
>>> 000000000004ba18 r1+48    u
>>> 000000000004ba1c r1+0     u
>>> libc.so.6:     file format elf64-powerpcle
>>> Disassembly of section .text:
>>> 000000000004b9d4 <kill>:
>>>      4b9d4:       1f 00 4c 3c     addis   r2,r12,31
>>>      4b9d8:       2c c3 42 38     addi    r2,r2,-15572
>>>      4b9dc:       25 00 00 38     li      r0,37
>>>      4b9e0:       d1 ff 21 f8     stdu    r1,-48(r1)
>>>      4b9e4:       20 00 e1 fb     std     r31,32(r1)
>>>      4b9e8:       98 8f ed eb     ld      r31,-28776(r13)
>>>      4b9ec:       10 00 ff 77     andis.  r31,r31,16
>>>      4b9f0:       1c 00 82 41     beq     4ba0c <kill+0x38>
>>>      4b9f4:       a6 02 28 7d     mflr    r9
>>>      4b9f8:       10 00 21 f9     std     r9,16(r1)
>>>      4b9fc:       01 00 00 44     scv     0
>>>      4ba00:       10 00 21 e9     ld      r9,16(r1)
>>>      4ba04:       a6 03 28 7d     mtlr    r9
>>>      4ba08:       08 00 00 48     b       4ba10 <kill+0x3c>
>>>      4ba0c:       02 00 00 44     sc
>>>      4ba10:       00 00 bf 2e     cmpdi   cr5,r31,0
>>>      4ba14:       20 00 e1 eb     ld      r31,32(r1)
>>>      4ba18:       30 00 21 38     addi    r1,r1,48
>>>      4ba1c:       18 00 96 41     beq     cr5,4ba34 <kill+0x60>
>>>      4ba20:       01 f0 20 39     li      r9,-4095
>>>      4ba24:       40 48 23 7c     cmpld   r3,r9
>>>      4ba28:       20 00 e0 4d     bltlr+
>>>      4ba2c:       d0 00 63 7c     neg     r3,r3
>>>      4ba30:       08 00 00 48     b       4ba38 <kill+0x64>
>>>      4ba34:       20 00 e3 4c     bnslr+
>>>      4ba38:       c8 32 fe 4b     b       2ed00 <__syscall_error>
>>>           ...
>>>      4ba44:       40 20 0c 00     .long 0xc2040
>>>      4ba48:       68 00 00 00     .long 0x68
>>>      4ba4c:       06 00 5f 5f     rlwnm   r31,r26,r0,0,3
>>>      4ba50:       6b 69 6c 6c     xoris   r12,r3,26987
>>> ---
>>>    sysdeps/powerpc/powerpc64/sysdep.h | 6 ++++--
>>>    1 file changed, 4 insertions(+), 2 deletions(-)
>>> diff --git a/sysdeps/powerpc/powerpc64/sysdep.h
>>> b/sysdeps/powerpc/powerpc64/sysdep.h
>>> index 589f7c8d18..beb477d57b 100644
>>> --- a/sysdeps/powerpc/powerpc64/sysdep.h
>>> +++ b/sysdeps/powerpc/powerpc64/sysdep.h
>>> @@ -275,12 +275,14 @@ LT_LABELSUFFIX(name,_name_end): ; \
>>>    /* Allocate frame and save register */
>>>    #define NVOLREG_SAVE \
>>>        stdu r1,-SCV_FRAME_SIZE(r1); \
>>> +    cfi_adjust_cfa_offset(SCV_FRAME_SIZE); \
>>>        std r31,SCV_FRAME_NVOLREG_SAVE(r1); \
>>> -    cfi_adjust_cfa_offset(SCV_FRAME_SIZE);
>>> +    cfi_offset(r31,-(SCV_FRAME_SIZE-SCV_FRAME_NVOLREG_SAVE));
>>> OK. This (and similarly the case below) seem a little obfuscated for
>> computing stack save slots. I wonder if a distinct macro like
>> SCV_FRAME_R31_SAVE_SLOT (and similar SCV_FRAME_CALLER_LR_SAVE_SLOT
>> below) would make this code more approachable for those less familiar with ppc64
>> abis.
>>
> 
> But those kind of already exist: FRAME_LR_SAVE and
> SCV_FRAME_NVOLREG_SAVE, but these two values are relative to the
> top-most frame, so we need some math to get the values relative to the
> CFA.
> 
> Wouldn't adding 2 more macros with similar meanings (but with different
> references) make things even more complicated?
After refreshing my understanding of CFI. I agree with you.

> 
>>>    /* Restore register and destroy frame */
>>>    #define NVOLREG_RESTORE	\
>>>        ld r31,SCV_FRAME_NVOLREG_SAVE(r1); \
>>> +    cfi_restore(r31); \
>>>        addi r1,r1,SCV_FRAME_SIZE; \
>>>        cfi_adjust_cfa_offset(-SCV_FRAME_SIZE);
>>> @@ -332,7 +334,7 @@ LT_LABELSUFFIX(name,_name_end): ; \
>>>    #define DO_CALL_SCV \
>>>        mflr r9; \
>>>        std r9,FRAME_LR_SAVE(r1); \
>>> -    cfi_offset(lr,FRAME_LR_SAVE); \
>>> +    cfi_offset(lr,-(SCV_FRAME_SIZE-FRAME_LR_SAVE)) >       .machine "push"; \

I suspect this change is highlighting a bug in the asm.  We should be 
saving lr to the caller frame's lr slot, not the callee's.

>>>        .machine "power9"; \
>>>        scv 0; \
>>>
> 
> 
> --
> Matheus Castanho
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] powerpc64[le]: Fix CFI and LR save address for asm syscalls [BZ #28532]
  2021-11-04 22:36     ` Paul E Murphy via Libc-alpha
@ 2021-11-24 13:29       ` Matheus Castanho via Libc-alpha
  2021-11-24 14:08         ` Andreas Schwab
  0 siblings, 1 reply; 7+ messages in thread
From: Matheus Castanho via Libc-alpha @ 2021-11-24 13:29 UTC (permalink / raw)
  To: libc-alpha; +Cc: murphyp

Syscalls based on the assembly templates are missing CFI for r31, which gets
clobbered when scv is used, and info for LR is inaccurate, placed in the wrong
LOC and not using the proper offset. LR was also being saved to the callee's
frame, while the ABI mandates it to be saved to the caller's frame. These are
fixed by this commit.

After this change:

$ readelf -wF libc.so.6 | grep 0004b9d4.. -A 7 && objdump --disassemble=kill libc.so.6
00004a48 0000000000000020 00004a4c FDE cie=00000000 pc=000000000004b9d4..000000000004ba3c
   LOC           CFA      r31   ra
000000000004b9d4 r1+0     u     u
000000000004b9e4 r1+48    u     u
000000000004b9e8 r1+48    c-16  u
000000000004b9fc r1+48    c-16  c+16
000000000004ba08 r1+48    c-16
000000000004ba18 r1+48    u
000000000004ba1c r1+0     u

libc.so.6:     file format elf64-powerpcle

Disassembly of section .text:

000000000004b9d4 <kill>:
   4b9d4:       1f 00 4c 3c     addis   r2,r12,31
   4b9d8:       2c c3 42 38     addi    r2,r2,-15572
   4b9dc:       25 00 00 38     li      r0,37
   4b9e0:       d1 ff 21 f8     stdu    r1,-48(r1)
   4b9e4:       20 00 e1 fb     std     r31,32(r1)
   4b9e8:       98 8f ed eb     ld      r31,-28776(r13)
   4b9ec:       10 00 ff 77     andis.  r31,r31,16
   4b9f0:       1c 00 82 41     beq     4ba0c <kill+0x38>
   4b9f4:       a6 02 28 7d     mflr    r9
   4b9f8:       40 00 21 f9     std     r9,64(r1)
   4b9fc:       01 00 00 44     scv     0
   4ba00:       40 00 21 e9     ld      r9,64(r1)
   4ba04:       a6 03 28 7d     mtlr    r9
   4ba08:       08 00 00 48     b       4ba10 <kill+0x3c>
   4ba0c:       02 00 00 44     sc
   4ba10:       00 00 bf 2e     cmpdi   cr5,r31,0
   4ba14:       20 00 e1 eb     ld      r31,32(r1)
   4ba18:       30 00 21 38     addi    r1,r1,48
   4ba1c:       18 00 96 41     beq     cr5,4ba34 <kill+0x60>
   4ba20:       01 f0 20 39     li      r9,-4095
   4ba24:       40 48 23 7c     cmpld   r3,r9
   4ba28:       20 00 e0 4d     bltlr+
   4ba2c:       d0 00 63 7c     neg     r3,r3
   4ba30:       08 00 00 48     b       4ba38 <kill+0x64>
   4ba34:       20 00 e3 4c     bnslr+
   4ba38:       c8 32 fe 4b     b       2ed00 <__syscall_error>
        ...
   4ba44:       40 20 0c 00     .long 0xc2040
   4ba48:       68 00 00 00     .long 0x68
   4ba4c:       06 00 5f 5f     rlwnm   r31,r26,r0,0,3
   4ba50:       6b 69 6c 6c     xoris   r12,r3,26987
---
 sysdeps/powerpc/powerpc64/sysdep.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h
index 589f7c8d18..c751bc76f9 100644
--- a/sysdeps/powerpc/powerpc64/sysdep.h
+++ b/sysdeps/powerpc/powerpc64/sysdep.h
@@ -275,12 +275,14 @@ LT_LABELSUFFIX(name,_name_end): ; \
 /* Allocate frame and save register */
 #define NVOLREG_SAVE \
     stdu r1,-SCV_FRAME_SIZE(r1); \
+    cfi_adjust_cfa_offset(SCV_FRAME_SIZE); \
     std r31,SCV_FRAME_NVOLREG_SAVE(r1); \
-    cfi_adjust_cfa_offset(SCV_FRAME_SIZE);
+    cfi_offset(r31,-(SCV_FRAME_SIZE-SCV_FRAME_NVOLREG_SAVE));
 
 /* Restore register and destroy frame */
 #define NVOLREG_RESTORE	\
     ld r31,SCV_FRAME_NVOLREG_SAVE(r1); \
+    cfi_restore(r31); \
     addi r1,r1,SCV_FRAME_SIZE; \
     cfi_adjust_cfa_offset(-SCV_FRAME_SIZE);
 
@@ -331,13 +333,13 @@ LT_LABELSUFFIX(name,_name_end): ; \
 
 #define DO_CALL_SCV \
     mflr r9; \
-    std r9,FRAME_LR_SAVE(r1); \
+    std r9,SCV_FRAME_SIZE+FRAME_LR_SAVE(r1);   \
     cfi_offset(lr,FRAME_LR_SAVE); \
     .machine "push"; \
     .machine "power9"; \
     scv 0; \
     .machine "pop"; \
-    ld r9,FRAME_LR_SAVE(r1); \
+    ld r9,SCV_FRAME_SIZE+FRAME_LR_SAVE(r1);      \
     mtlr r9; \
     cfi_restore(lr);
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] powerpc64[le]: Fix CFI and LR save address for asm syscalls [BZ #28532]
  2021-11-24 13:29       ` [PATCH v2] powerpc64[le]: Fix CFI and LR save address for asm " Matheus Castanho via Libc-alpha
@ 2021-11-24 14:08         ` Andreas Schwab
  2021-11-26 21:29           ` Matheus Castanho via Libc-alpha
  0 siblings, 1 reply; 7+ messages in thread
From: Andreas Schwab @ 2021-11-24 14:08 UTC (permalink / raw)
  To: Matheus Castanho via Libc-alpha; +Cc: murphyp

On Nov 24 2021, Matheus Castanho via Libc-alpha wrote:

> diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h
> index 589f7c8d18..c751bc76f9 100644
> --- a/sysdeps/powerpc/powerpc64/sysdep.h
> +++ b/sysdeps/powerpc/powerpc64/sysdep.h
> @@ -275,12 +275,14 @@ LT_LABELSUFFIX(name,_name_end): ; \
>  /* Allocate frame and save register */
>  #define NVOLREG_SAVE \
>      stdu r1,-SCV_FRAME_SIZE(r1); \
> +    cfi_adjust_cfa_offset(SCV_FRAME_SIZE); \
>      std r31,SCV_FRAME_NVOLREG_SAVE(r1); \
> -    cfi_adjust_cfa_offset(SCV_FRAME_SIZE);
> +    cfi_offset(r31,-(SCV_FRAME_SIZE-SCV_FRAME_NVOLREG_SAVE));

Perhaps use cfi_rel_offset instead.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] powerpc64[le]: Fix CFI and LR save address for asm syscalls [BZ #28532]
  2021-11-24 14:08         ` Andreas Schwab
@ 2021-11-26 21:29           ` Matheus Castanho via Libc-alpha
  0 siblings, 0 replies; 7+ messages in thread
From: Matheus Castanho via Libc-alpha @ 2021-11-26 21:29 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: murphyp, Matheus Castanho via Libc-alpha


Andreas Schwab <schwab@linux-m68k.org> writes:

> On Nov 24 2021, Matheus Castanho via Libc-alpha wrote:
>
>> diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h
>> index 589f7c8d18..c751bc76f9 100644
>> --- a/sysdeps/powerpc/powerpc64/sysdep.h
>> +++ b/sysdeps/powerpc/powerpc64/sysdep.h
>> @@ -275,12 +275,14 @@ LT_LABELSUFFIX(name,_name_end): ; \
>>  /* Allocate frame and save register */
>>  #define NVOLREG_SAVE \
>>      stdu r1,-SCV_FRAME_SIZE(r1); \
>> +    cfi_adjust_cfa_offset(SCV_FRAME_SIZE); \
>>      std r31,SCV_FRAME_NVOLREG_SAVE(r1); \
>> -    cfi_adjust_cfa_offset(SCV_FRAME_SIZE);
>> +    cfi_offset(r31,-(SCV_FRAME_SIZE-SCV_FRAME_NVOLREG_SAVE));
>
> Perhaps use cfi_rel_offset instead.

Thanks, that's much cleaner. I included that on v3 [1]

[1] https://sourceware.org/pipermail/libc-alpha/2021-November/133491.html

--
Matheus Castanho

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-11-26 21:31 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-03 14:22 [PATCH] powerpc64[le]: Fix CFI for assembly templated syscalls [BZ #28532] Matheus Castanho via Libc-alpha
2021-11-03 15:46 ` Paul E Murphy via Libc-alpha
2021-11-03 16:41   ` Matheus Castanho via Libc-alpha
2021-11-04 22:36     ` Paul E Murphy via Libc-alpha
2021-11-24 13:29       ` [PATCH v2] powerpc64[le]: Fix CFI and LR save address for asm " Matheus Castanho via Libc-alpha
2021-11-24 14:08         ` Andreas Schwab
2021-11-26 21:29           ` Matheus Castanho via Libc-alpha

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).