Patchwork S390: Fix backtrace in vdso functions.

login
register
mail settings
Submitter Stefan Liebler
Date Nov. 28, 2017, 12:44 p.m.
Message ID <ead922fd-1eb9-3781-345d-6a2ea63ecd01@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/24570/
State New
Headers show

Comments

Stefan Liebler - Nov. 28, 2017, 12:44 p.m.
Hi,

On s390, GDB fails to show the complete backtrace from within vdso 
functions. The macro INTERNAL_VSYSCALL_CALL saves the return address in 
r14 to r10 before branching to the vdso function. The branch-instruction 
updates r14 in order to let the vdso function return. Then the original 
address in r14 is restored from r10. Unfortunately, there are no 
cfi-rules and GDB fails.

Furthermore the call of the vdso function does not comply with the s390 
ABI as no stack-frame for the vdso-function is generated.

This patch removes the s390 specific macro INTERNAL_VSYSCALL_CALL and 
the common implementation in sysdeps/unix/sysv/linux/sysdep-vdso.h is 
used. Then the vdso function is called via function-pointer and GCC 
generates a new stack-frame and emits all needed cfi-rules.

The defines CLOBBER_[0-6] are removed as they  were only used in macro 
INTERNAL_VSYSCALL_CALL.

The macro INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK is not used on s390. The 
only user is power. Thus it is removed from s390 sysdep.h.

ChangeLog:

	* sysdeps/unix/sysv/linux/s390/s390-64/sysdep.h
	(INTERNAL_VSYSCALL_CALL, CLOBBER_0, CLOBBER_1, CLOBBER_2,
	 CLOBBER_3, CLOBBER_4, CLOBBER_5, CLOBBER_6,
	INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK): Remove.
	* sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h: Likewise.
Adhemerval Zanella Netto - Nov. 28, 2017, 1:09 p.m.
On 28/11/2017 10:44, Stefan Liebler wrote:
> Hi,
> 
> On s390, GDB fails to show the complete backtrace from within vdso functions. The macro INTERNAL_VSYSCALL_CALL saves the return address in r14 to r10 before branching to the vdso function. The branch-instruction updates r14 in order to let the vdso function return. Then the original address in r14 is restored from r10. Unfortunately, there are no cfi-rules and GDB fails.
> 
> Furthermore the call of the vdso function does not comply with the s390 ABI as no stack-frame for the vdso-function is generated.
> 
> This patch removes the s390 specific macro INTERNAL_VSYSCALL_CALL and the common implementation in sysdeps/unix/sysv/linux/sysdep-vdso.h is used. Then the vdso function is called via function-pointer and GCC generates a new stack-frame and emits all needed cfi-rules.
> 
> The defines CLOBBER_[0-6] are removed as they  were only used in macro INTERNAL_VSYSCALL_CALL.
> 
> The macro INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK is not used on s390. The only user is power. Thus it is removed from s390 sysdep.h.

I am almost sure we can remove it for powerpc as well (I can't see
no immediate gain on doing a function call using inline assembly
as for INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK on powerpc). 


> 
> ChangeLog:
> 
>     * sysdeps/unix/sysv/linux/s390/s390-64/sysdep.h
>     (INTERNAL_VSYSCALL_CALL, CLOBBER_0, CLOBBER_1, CLOBBER_2,
>      CLOBBER_3, CLOBBER_4, CLOBBER_5, CLOBBER_6,
>     INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK): Remove.
>     * sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h: Likewise.

Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Florian Weimer - Nov. 28, 2017, 2:18 p.m.
On 11/28/2017 02:09 PM, Adhemerval Zanella wrote:
> I am almost sure we can remove it for powerpc as well (I can't see
> no immediate gain on doing a function call using inline assembly
> as for INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK on powerpc).

Is there another way to obtain the condition code for the error status?

   /* The vDSO does not return an error (it clear cr0.so on returning).  */
   INTERNAL_SYSCALL_DECL (err);
   result =
     INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK (get_tbfreq, err, uint64_t, 0);

Thanks,
Florian
Adhemerval Zanella Netto - Nov. 28, 2017, 3:53 p.m.
On 28/11/2017 12:18, Florian Weimer wrote:
> On 11/28/2017 02:09 PM, Adhemerval Zanella wrote:
>> I am almost sure we can remove it for powerpc as well (I can't see
>> no immediate gain on doing a function call using inline assembly
>> as for INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK on powerpc).
> 
> Is there another way to obtain the condition code for the error status?
> 
>   /* The vDSO does not return an error (it clear cr0.so on returning).  */
>   INTERNAL_SYSCALL_DECL (err);
>   result =
>     INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK (get_tbfreq, err, uint64_t, 0);
> 
> Thanks,
> Florian

I think we can simplify this call in specific since __kernel_get_tbfreq
always succeed if the vDSO is present. The only issue is the symbol
returns an uint64_t even for powerpc32.
Adhemerval Zanella Netto - Nov. 28, 2017, 4:34 p.m.
On 28/11/2017 13:53, Adhemerval Zanella wrote:
> 
> 
> On 28/11/2017 12:18, Florian Weimer wrote:
>> On 11/28/2017 02:09 PM, Adhemerval Zanella wrote:
>>> I am almost sure we can remove it for powerpc as well (I can't see
>>> no immediate gain on doing a function call using inline assembly
>>> as for INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK on powerpc).
>>
>> Is there another way to obtain the condition code for the error status?
>>
>>   /* The vDSO does not return an error (it clear cr0.so on returning).  */
>>   INTERNAL_SYSCALL_DECL (err);
>>   result =
>>     INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK (get_tbfreq, err, uint64_t, 0);
>>
>> Thanks,
>> Florian
> 
> I think we can simplify this call in specific since __kernel_get_tbfreq
> always succeed if the vDSO is present. The only issue is the symbol
> returns an uint64_t even for powerpc32.
> 

The problem is for __kernel_clock_getres and __kernel_clock_gettime the
fallback for clocks different than CLOCK_REALTIME or CLOCK_MONOTONIC
is to issue a syscall (and then it requires a mfcr to get the result
value).
Stefan Liebler - Dec. 4, 2017, 7:59 a.m.
On 11/28/2017 02:09 PM, Adhemerval Zanella wrote:
> 
> 
> On 28/11/2017 10:44, Stefan Liebler wrote:
>> Hi,
>>
>> On s390, GDB fails to show the complete backtrace from within vdso functions. The macro INTERNAL_VSYSCALL_CALL saves the return address in r14 to r10 before branching to the vdso function. The branch-instruction updates r14 in order to let the vdso function return. Then the original address in r14 is restored from r10. Unfortunately, there are no cfi-rules and GDB fails.
>>
>> Furthermore the call of the vdso function does not comply with the s390 ABI as no stack-frame for the vdso-function is generated.
>>
>> This patch removes the s390 specific macro INTERNAL_VSYSCALL_CALL and the common implementation in sysdeps/unix/sysv/linux/sysdep-vdso.h is used. Then the vdso function is called via function-pointer and GCC generates a new stack-frame and emits all needed cfi-rules.
>>
>> The defines CLOBBER_[0-6] are removed as they  were only used in macro INTERNAL_VSYSCALL_CALL.
>>
>> The macro INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK is not used on s390. The only user is power. Thus it is removed from s390 sysdep.h.
> 
> I am almost sure we can remove it for powerpc as well (I can't see
> no immediate gain on doing a function call using inline assembly
> as for INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK on powerpc).
> 
> 
>>
>> ChangeLog:
>>
>>      * sysdeps/unix/sysv/linux/s390/s390-64/sysdep.h
>>      (INTERNAL_VSYSCALL_CALL, CLOBBER_0, CLOBBER_1, CLOBBER_2,
>>       CLOBBER_3, CLOBBER_4, CLOBBER_5, CLOBBER_6,
>>      INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK): Remove.
>>      * sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h: Likewise.
> 
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> 

If there are no objections regarding the s390 patch, I'll commit.

Adhemerval: Do you keep working on removing 
INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK on powerpc?

Bye.
Stefan
Adhemerval Zanella Netto - Dec. 4, 2017, 10:47 a.m.
On 04/12/2017 05:59, Stefan Liebler wrote:
> On 11/28/2017 02:09 PM, Adhemerval Zanella wrote:
>>
>>
>> On 28/11/2017 10:44, Stefan Liebler wrote:
>>> Hi,
>>>
>>> On s390, GDB fails to show the complete backtrace from within vdso functions. The macro INTERNAL_VSYSCALL_CALL saves the return address in r14 to r10 before branching to the vdso function. The branch-instruction updates r14 in order to let the vdso function return. Then the original address in r14 is restored from r10. Unfortunately, there are no cfi-rules and GDB fails.
>>>
>>> Furthermore the call of the vdso function does not comply with the s390 ABI as no stack-frame for the vdso-function is generated.
>>>
>>> This patch removes the s390 specific macro INTERNAL_VSYSCALL_CALL and the common implementation in sysdeps/unix/sysv/linux/sysdep-vdso.h is used. Then the vdso function is called via function-pointer and GCC generates a new stack-frame and emits all needed cfi-rules.
>>>
>>> The defines CLOBBER_[0-6] are removed as they  were only used in macro INTERNAL_VSYSCALL_CALL.
>>>
>>> The macro INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK is not used on s390. The only user is power. Thus it is removed from s390 sysdep.h.
>>
>> I am almost sure we can remove it for powerpc as well (I can't see
>> no immediate gain on doing a function call using inline assembly
>> as for INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK on powerpc).
>>
>>
>>>
>>> ChangeLog:
>>>
>>>      * sysdeps/unix/sysv/linux/s390/s390-64/sysdep.h
>>>      (INTERNAL_VSYSCALL_CALL, CLOBBER_0, CLOBBER_1, CLOBBER_2,
>>>       CLOBBER_3, CLOBBER_4, CLOBBER_5, CLOBBER_6,
>>>      INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK): Remove.
>>>      * sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h: Likewise.
>>
>> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>>
> 
> If there are no objections regarding the s390 patch, I'll commit.
> 
> Adhemerval: Do you keep working on removing INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK on powerpc?

Different than s390 where the an error is indicate through a negative value,
powerpc signals an error through CR0.SO.  And __kernel_clock_getres and 
__kernel_clock_gettime fallback to a syscall call in case the timers are
not supported through vDSO, which requires we check CR0.SO value.  I do
not a reliable way to check it after a function call on GCC (we can issue
a volatile assembly after it, but I think it is still fragile and it
would require a arch-specific wrapper anyway).

There is also the issue with __kernel_get_tbfreq which returns a 64 bit
value for powercp32, so we need another wrapper that does expect the return
code as long int.
Stefan Liebler - Dec. 4, 2017, 3:42 p.m.
On 12/04/2017 11:47 AM, Adhemerval Zanella wrote:
> 
> 
> On 04/12/2017 05:59, Stefan Liebler wrote:
>> On 11/28/2017 02:09 PM, Adhemerval Zanella wrote:
>>>
>>>
>>> On 28/11/2017 10:44, Stefan Liebler wrote:
>>>> Hi,
>>>>
>>>> On s390, GDB fails to show the complete backtrace from within vdso functions. The macro INTERNAL_VSYSCALL_CALL saves the return address in r14 to r10 before branching to the vdso function. The branch-instruction updates r14 in order to let the vdso function return. Then the original address in r14 is restored from r10. Unfortunately, there are no cfi-rules and GDB fails.
>>>>
>>>> Furthermore the call of the vdso function does not comply with the s390 ABI as no stack-frame for the vdso-function is generated.
>>>>
>>>> This patch removes the s390 specific macro INTERNAL_VSYSCALL_CALL and the common implementation in sysdeps/unix/sysv/linux/sysdep-vdso.h is used. Then the vdso function is called via function-pointer and GCC generates a new stack-frame and emits all needed cfi-rules.
>>>>
>>>> The defines CLOBBER_[0-6] are removed as they  were only used in macro INTERNAL_VSYSCALL_CALL.
>>>>
>>>> The macro INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK is not used on s390. The only user is power. Thus it is removed from s390 sysdep.h.
>>>
>>> I am almost sure we can remove it for powerpc as well (I can't see
>>> no immediate gain on doing a function call using inline assembly
>>> as for INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK on powerpc).
>>>
>>>
>>>>
>>>> ChangeLog:
>>>>
>>>>       * sysdeps/unix/sysv/linux/s390/s390-64/sysdep.h
>>>>       (INTERNAL_VSYSCALL_CALL, CLOBBER_0, CLOBBER_1, CLOBBER_2,
>>>>        CLOBBER_3, CLOBBER_4, CLOBBER_5, CLOBBER_6,
>>>>       INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK): Remove.
>>>>       * sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h: Likewise.
>>>
>>> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>>>
>>
>> If there are no objections regarding the s390 patch, I'll commit.
>>
>> Adhemerval: Do you keep working on removing INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK on powerpc?
> 
> Different than s390 where the an error is indicate through a negative value,
> powerpc signals an error through CR0.SO.  And __kernel_clock_getres and
> __kernel_clock_gettime fallback to a syscall call in case the timers are
> not supported through vDSO, which requires we check CR0.SO value.  I do
> not a reliable way to check it after a function call on GCC (we can issue
> a volatile assembly after it, but I think it is still fragile and it
> would require a arch-specific wrapper anyway).
> 
> There is also the issue with __kernel_get_tbfreq which returns a 64 bit
> value for powercp32, so we need another wrapper that does expect the return
> code as long int.
> 

Okay.
Then I've just committed this patch.

Patch

commit a1000218276c4cfb9a72e53f17b144b137faf334
Author: Stefan Liebler <stli@linux.vnet.ibm.com>
Date:   Mon Nov 27 11:24:45 2017 +0100

    S390: Fix backtrace in vdso functions.
    
    On s390, GDB fails to show the complete backtrace from within vdso functions.
    The macro INTERNAL_VSYSCALL_CALL saves the return address in r14 to r10
    before branching to the vdso function. The branch-instruction updates r14
    in order to let the vdso function return. Then the original address in r14 is
    restored from r10. Unfortunately, there are no cfi-rules and GDB fails.
    
    Furthermore the call of the vdso function does not comply with the s390 ABI
    as no stack-frame for the vdso-function is generated.
    
    This patch removes the s390 specific macro INTERNAL_VSYSCALL_CALL
    and the common implementation in sysdeps/unix/sysv/linux/sysdep-vdso.h is used.
    Then the vdso function is called via function-pointer and GCC generates a
    new stack-frame and emits all needed cfi-rules.
    
    The defines CLOBBER_[0-6] are removed as they  were only used in macro
    INTERNAL_VSYSCALL_CALL.
    
    The macro INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK is not used on s390.
    The only user is power. Thus it is removed from s390 sysdep.h.
    
    ChangeLog:
    
    	* sysdeps/unix/sysv/linux/s390/s390-64/sysdep.h
    	(INTERNAL_VSYSCALL_CALL, CLOBBER_0, CLOBBER_1, CLOBBER_2,
    	 CLOBBER_3, CLOBBER_4, CLOBBER_5, CLOBBER_6,
    	INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK): Remove.
    	* sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h: Likewise.

diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h b/sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h
index e56fc32..644b83c 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h
@@ -271,48 +271,12 @@ 
 #define ASMFMT_5 , "0" (gpr2), "d" (gpr3), "d" (gpr4), "d" (gpr5), "d" (gpr6)
 #define ASMFMT_6 , "0" (gpr2), "d" (gpr3), "d" (gpr4), "d" (gpr5), "d" (gpr6), "d" (gpr7)
 
-#define CLOBBER_0 , "3", "4", "5"
-#define CLOBBER_1 , "3", "4", "5"
-#define CLOBBER_2 , "4", "5"
-#define CLOBBER_3 , "5"
-#define CLOBBER_4
-#define CLOBBER_5
-#define CLOBBER_6
-
 /* List of system calls which are supported as vsyscalls.  */
 #define HAVE_CLOCK_GETRES_VSYSCALL	1
 #define HAVE_CLOCK_GETTIME_VSYSCALL	1
 #define HAVE_GETTIMEOFDAY_VSYSCALL	1
 #define HAVE_GETCPU_VSYSCALL		1
 
-/* This version is for internal uses when there is no desire
-   to set errno */
-#define INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK(name, err, nr, args...)	      \
-  ({									      \
-    long int _ret = ENOSYS;						      \
-									      \
-    __typeof (__vdso_##name) vdsop = __vdso_##name;			      \
-    PTR_DEMANGLE (vdsop);						      \
-    if (vdsop != NULL)							      \
-      _ret = INTERNAL_VSYSCALL_CALL (vdsop, err, nr, ##args);		      \
-    else								      \
-      err = 1 << 28;							      \
-    _ret;								      \
-  })
-
-#define INTERNAL_VSYSCALL_CALL(fn, err, nr, args...)			      \
-  ({									      \
-    DECLARGS_##nr(args)							      \
-    register long _ret __asm__("2");					      \
-    __asm__ __volatile__ (						      \
-			  "lr 10,14\n\t"				      \
-			  "basr 14,%1\n\t"				      \
-			  "lr 14,10\n\t"				      \
-			  : "=d" (_ret)					      \
-			  : "d" (fn) ASMFMT_##nr			      \
-			  : "cc", "memory", "0", "1", "10" CLOBBER_##nr);     \
-    _ret; })
-
 /* Pointer mangling support.  */
 #if IS_IN (rtld)
 /* We cannot use the thread descriptor because in ld.so we use setjmp
diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/sysdep.h b/sysdeps/unix/sysv/linux/s390/s390-64/sysdep.h
index 622991d..18ed339 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/s390/s390-64/sysdep.h
@@ -277,14 +277,6 @@ 
 #define ASMFMT_5 , "0" (gpr2), "d" (gpr3), "d" (gpr4), "d" (gpr5), "d" (gpr6)
 #define ASMFMT_6 , "0" (gpr2), "d" (gpr3), "d" (gpr4), "d" (gpr5), "d" (gpr6), "d" (gpr7)
 
-#define CLOBBER_0 , "3", "4", "5"
-#define CLOBBER_1 , "3", "4", "5"
-#define CLOBBER_2 , "4", "5"
-#define CLOBBER_3 , "5"
-#define CLOBBER_4
-#define CLOBBER_5
-#define CLOBBER_6
-
 /* List of system calls which are supported as vsyscalls.  */
 #define HAVE_CLOCK_GETRES_VSYSCALL	1
 #define HAVE_CLOCK_GETTIME_VSYSCALL	1
@@ -293,34 +285,6 @@ 
 
 #define SINGLE_THREAD_BY_GLOBAL		1
 
-/* This version is for internal uses when there is no desire
-   to set errno */
-#define INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK(name, err, nr, args...)	      \
-  ({									      \
-    long int _ret = ENOSYS;						      \
-									      \
-    __typeof (__vdso_##name) vdsop = __vdso_##name;			      \
-    PTR_DEMANGLE (vdsop);						      \
-    if (vdsop != NULL)							      \
-      _ret = INTERNAL_VSYSCALL_CALL (vdsop, err, nr, ##args);		      \
-    else								      \
-      err = 1 << 28;							      \
-    _ret;								      \
-  })
-
-#define INTERNAL_VSYSCALL_CALL(fn, err, nr, args...)			      \
-  ({									      \
-    DECLARGS_##nr(args)							      \
-    register long _ret __asm__("2");					      \
-    __asm__ __volatile__ (						      \
-			  "lgr 10,14\n\t"				      \
-			  "basr 14,%1\n\t"				      \
-			  "lgr 14,10\n\t"				      \
-			  : "=d" (_ret)					      \
-			  : "a" (fn) ASMFMT_##nr			      \
-			  : "cc", "memory", "0", "1", "10" CLOBBER_##nr);     \
-    _ret; })
-
 /* Pointer mangling support.  */
 #if IS_IN (rtld)
 /* We cannot use the thread descriptor because in ld.so we use setjmp