[PATCH] powerpc64[le]: Fix CFI for assembly templated syscalls [BZ #28532]

Paul E Murphy murphyp@linux.ibm.com
Thu Nov 4 22:36:42 GMT 2021



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
> 


More information about the Libc-alpha mailing list