This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 1/2] Fix incorrectly named __kernel_get_tbfreq VDSOfunction
- From: Ryan Arnold <rsa at us dot ibm dot com>
- To: "Ryan S. Arnold" <ryan dot arnold at gmail dot com>
- Cc: libc-alpha <libc-alpha at sourceware dot org>, Anton Blanchard<anton at samba dot org>
- Date: Tue, 15 Jan 2013 15:10:34 -0600
- Subject: Re: [PATCH 1/2] Fix incorrectly named __kernel_get_tbfreq VDSOfunction
- References: <20120709112735.22c2e72b@kryten> <CAAKybw8XrcTffP6TRXq0qg+5mpmp6NMHQnKpVf_nPPfhibLG9Q@mail.gmail.com> <20121129.193503.1047178464074352872.davem@davemloft.net> <CAAKybw-kpKmgtOJrxU9QENwea=QU+ZEFzgJviC7TcFEd2dyXMQ@mail.gmail.com> <CAAKybw-xQZvaNm7dtQ6zMzK3omZM8C3P+iK6e6f8qzPWb6fvSg@mail.gmail.com> <CAAKybw84rXdD1-Xhc+NyM=MRkNg4WNTgnZu3-V7O10C5+t=JLw@mail.gmail.com>
- Reply-to: rsa at us dot ibm dot com
On Fri, 2012-12-07 at 14:42 -0600, Ryan S. Arnold wrote:
> On Fri, Nov 30, 2012 at 5:14 PM, Ryan S. Arnold <ryan.arnold@gmail.com> wrote:
> >> There's something goofy happening during testing, so I'm going to see
> >> if I can get to the bottom of it by tomorrow.
>
> Tulio discovered that this is failing in powerpc32 because the
> VSYSCALL macro casts the return type to (int) whereas the
> timebase_freq is 64-bits. This doesn't fail in powerpc64 because I
> removed the (int) cast from those macros last year. None-the-less,
> returning from long long int from the macros is not a solution to this
> problem.
>
> I have a hand written asm fix for this specific vdso call. We're
> investigating whether this can be provided via a simpler common C
> routine. We don't want to use a mixture of C, CPP/Macro, and inline
> asm as is currently in place because the existing syscall macros make
> assumptions about compiler behavior that may not be true in the future
> (like the compiler honoring 'register' requests).
We have a CPP macro fix for this issue instead but there is a compiler
bug prior to GCC 4.8 which causes incorrect register selection if a
particular register is specified in the output reg list of an inline asm
statement more than once. Alan Modra proposed a workaround for the
issue where we have a second inline asm in the INTERNAL_VSYSCALL_NCS
macro which binds the vDSO call return value to the ABI specified
register (or implied pair).
The change to INTERNAL_VSYSCALL_NCS interface was proposed by Peter
Bergner to work allow the INTERNAL_VSYSCALL_MACROS to return types of
size greater than long int (long long int in the case of the timebase
frequency).
PowerPC: Rename __kernel_vdso_get_tbfreq to __kernel_get_tbfreq.
In order for the __kernel_get_tbfreq vDSO call to work the
INTERNAL_VSYSCALL_NCS macro needed to be updated to prevent it from
assuming an integer return type (since the timebase frequency is a 64-bit
value) by specifying the type of the return type as a macro parameter. The
macro then specifically declares the return value as a 'register' (or
implied pair) of the denoted type. The compiler is then informed that this
register (or implied pair) is to be used for the return value.
diff --git a/ChangeLog b/ChangeLog
index 0f5a017..db98cf2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,25 @@
+2013-01-15 Anton Blanchard <anton@samba.org>
+ Ryan S. Arnold <rsa@linux.vnet.ibm.com>
+
+ * sysdeps/unix/sysv/linux/powerpc/init-first.c: Rename
+ __kernel_vdso_get_tbfreq to __kernel_get_tbfreq.
+ * sysdeps/unix/sysv/linux/powerpc/get_clockfreq.c: Add parameter to
+ INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK to specify return type.
+ * sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
+ (INTERNAL_VSYSCALL_NCS): Change "=&r" in inline asm output regs list to
+ "+r" and remove output regs list as redundant. Add explicit inline
+ asm to specify register of return val to work around compiler codegen
+ bug. Remove (int) cast on return value. Add return type parameter to
+ use in macro so that this macro does not truncate return value for
+ 64-bit values.
+ (INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK): Add return type parameter and
+ pass to INTERNAL_VSYSCALL_NCS.
+ (INLINE_VSYSCALL): Add 'long int' as return type to
+ INTERNAL_VSYSCALL_NCS macro invocation.
+ (INTERNAL_VSYSCALL): Add 'long int' as return type to
+ INTERNAL_VSYSCALL_NCS macro invocation.
+ * sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h: Likewise.
+
2013-01-14 David S. Miller <davem@davemloft.net>
* sysdeps/sparc/sparc-ifunc.h (SPARC_ASM_IFUNC2): New macro.
diff --git a/sysdeps/unix/sysv/linux/powerpc/get_clockfreq.c b/sysdeps/unix/sysv/linux/powerpc/get_clockfreq.c
index 616342a..5e88b83 100644
--- a/sysdeps/unix/sysv/linux/powerpc/get_clockfreq.c
+++ b/sysdeps/unix/sysv/linux/powerpc/get_clockfreq.c
@@ -41,7 +41,8 @@ __get_clockfreq (void)
/* If we can use the vDSO to obtain the timebase even better. */
#ifdef SHARED
INTERNAL_SYSCALL_DECL (err);
- timebase_freq = INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK (get_tbfreq, err, 0);
+ timebase_freq =
+ INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK (get_tbfreq, err, hp_timing_t, 0);
if (INTERNAL_SYSCALL_ERROR_P (timebase_freq, err)
&& INTERNAL_SYSCALL_ERRNO (timebase_freq, err) == ENOSYS)
#endif
diff --git a/sysdeps/unix/sysv/linux/powerpc/init-first.c b/sysdeps/unix/sysv/linux/powerpc/init-first.c
index 195d030..204c0c6 100644
--- a/sysdeps/unix/sysv/linux/powerpc/init-first.c
+++ b/sysdeps/unix/sysv/linux/powerpc/init-first.c
@@ -41,7 +41,7 @@ _libc_vdso_platform_setup (void)
__vdso_clock_getres = _dl_vdso_vsym ("__kernel_clock_getres", &linux2615);
- __vdso_get_tbfreq = _dl_vdso_vsym ("__kernel_vdso_get_tbfreq", &linux2615);
+ __vdso_get_tbfreq = _dl_vdso_vsym ("__kernel_get_tbfreq", &linux2615);
__vdso_getcpu = _dl_vdso_vsym ("__kernel_getcpu", &linux2615);
}
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h b/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
index e047bf7..250f4fc 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
@@ -60,7 +60,8 @@
\
if (__vdso_##name != NULL) \
{ \
- sc_ret = INTERNAL_VSYSCALL_NCS (__vdso_##name, sc_err, nr, ##args); \
+ sc_ret = \
+ INTERNAL_VSYSCALL_NCS (__vdso_##name, sc_err, long int, nr, ##args);\
if (!INTERNAL_SYSCALL_ERROR_P (sc_ret, sc_err)) \
goto out; \
if (INTERNAL_SYSCALL_ERRNO (sc_ret, sc_err) != ENOSYS) \
@@ -90,7 +91,8 @@
\
if (__vdso_##name != NULL) \
{ \
- v_ret = INTERNAL_VSYSCALL_NCS (__vdso_##name, err, nr, ##args); \
+ v_ret = \
+ INTERNAL_VSYSCALL_NCS (__vdso_##name, err, long int, nr, ##args); \
if (!INTERNAL_SYSCALL_ERROR_P (v_ret, err) \
|| INTERNAL_SYSCALL_ERRNO (v_ret, err) != ENOSYS) \
goto out; \
@@ -104,12 +106,12 @@
INTERNAL_SYSCALL (name, err, nr, ##args)
# endif
-# define INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK(name, err, nr, args...) \
+# define INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK(name, err, type, nr, args...) \
({ \
- long int sc_ret = ENOSYS; \
+ type sc_ret = ENOSYS; \
\
if (__vdso_##name != NULL) \
- sc_ret = INTERNAL_VSYSCALL_NCS (__vdso_##name, err, nr, ##args); \
+ sc_ret = INTERNAL_VSYSCALL_NCS (__vdso_##name, err, type, nr, ##args); \
else \
err = 1 << 28; \
sc_ret; \
@@ -126,7 +128,7 @@
function call, with the exception of LR (which is needed for the
"sc; bnslr+" sequence) and CR (where only CR0.SO is clobbered to signal
an error return status). */
-# define INTERNAL_VSYSCALL_NCS(funcptr, err, nr, args...) \
+# define INTERNAL_VSYSCALL_NCS(funcptr, err, type, nr, args...) \
({ \
register void *r0 __asm__ ("r0"); \
register long int r3 __asm__ ("r3"); \
@@ -139,18 +141,18 @@
register long int r10 __asm__ ("r10"); \
register long int r11 __asm__ ("r11"); \
register long int r12 __asm__ ("r12"); \
+ register type rval __asm__ ("r3"); \
LOADARGS_##nr (funcptr, args); \
__asm__ __volatile__ \
("mtctr %0\n\t" \
"bctrl\n\t" \
"mfcr %0" \
- : "=&r" (r0), \
- "=&r" (r3), "=&r" (r4), "=&r" (r5), "=&r" (r6), "=&r" (r7), \
- "=&r" (r8), "=&r" (r9), "=&r" (r10), "=&r" (r11), "=&r" (r12) \
- : ASM_INPUT_##nr \
- : "cr0", "ctr", "lr", "memory"); \
+ : "+r" (r0), "+r" (r3), "+r" (r4), "+r" (r5), "+r" (r6), "+r" (r7), \
+ "+r" (r8), "+r" (r9), "+r" (r10), "+r" (r11), "+r" (r12) \
+ : : "cr0", "ctr", "lr", "memory"); \
err = (long int) r0; \
- (int) r3; \
+ __asm__ __volatile__ ("" : "=r" (rval) : "r" (r3), "r" (r4)); \
+ rval; \
})
# undef INLINE_SYSCALL
@@ -191,7 +193,7 @@
register long int r10 __asm__ ("r10"); \
register long int r11 __asm__ ("r11"); \
register long int r12 __asm__ ("r12"); \
- LOADARGS_##nr(name, args); \
+ LOADARGS_##nr(name, args); \
__asm__ __volatile__ \
("sc \n\t" \
"mfcr %0" \
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h b/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
index 1f0c3a2..6ebab74 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
@@ -75,7 +75,8 @@
\
if (__vdso_##name != NULL) \
{ \
- sc_ret = INTERNAL_VSYSCALL_NCS (__vdso_##name, sc_err, nr, ##args); \
+ sc_ret = \
+ INTERNAL_VSYSCALL_NCS (__vdso_##name, sc_err, long int, nr, ##args);\
if (!INTERNAL_SYSCALL_ERROR_P (sc_ret, sc_err)) \
goto out; \
if (INTERNAL_SYSCALL_ERRNO (sc_ret, sc_err) != ENOSYS) \
@@ -105,7 +106,8 @@
\
if (__vdso_##name != NULL) \
{ \
- v_ret = INTERNAL_VSYSCALL_NCS (__vdso_##name, err, nr, ##args); \
+ v_ret = \
+ INTERNAL_VSYSCALL_NCS (__vdso_##name, err, long int, nr, ##args); \
if (!INTERNAL_SYSCALL_ERROR_P (v_ret, err) \
|| INTERNAL_SYSCALL_ERRNO (v_ret, err) != ENOSYS) \
goto out; \
@@ -121,12 +123,12 @@
/* This version is for internal uses when there is no desire
to set errno */
-#define INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK(name, err, nr, args...) \
+#define INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK(name, err, type, nr, args...) \
({ \
- long int sc_ret = ENOSYS; \
+ type sc_ret = ENOSYS; \
\
if (__vdso_##name != NULL) \
- sc_ret = INTERNAL_VSYSCALL_NCS (__vdso_##name, err, nr, ##args); \
+ sc_ret = INTERNAL_VSYSCALL_NCS (__vdso_##name, err, type, nr, ##args); \
else \
err = 1 << 28; \
sc_ret; \
@@ -142,7 +144,7 @@
gave back in the non-error (CR0.SO cleared) case, otherwise (CR0.SO set)
the negation of the return value in the kernel gets reverted. */
-#define INTERNAL_VSYSCALL_NCS(funcptr, err, nr, args...) \
+#define INTERNAL_VSYSCALL_NCS(funcptr, err, type, nr, args...) \
({ \
register void *r0 __asm__ ("r0"); \
register long int r3 __asm__ ("r3"); \
@@ -151,20 +153,19 @@
register long int r6 __asm__ ("r6"); \
register long int r7 __asm__ ("r7"); \
register long int r8 __asm__ ("r8"); \
+ register type rval __asm__ ("r3"); \
LOADARGS_##nr (funcptr, args); \
__asm__ __volatile__ \
("mtctr %0\n\t" \
"bctrl\n\t" \
"mfcr %0\n\t" \
"0:" \
- : "=&r" (r0), \
- "=&r" (r3), "=&r" (r4), "=&r" (r5), \
- "=&r" (r6), "=&r" (r7), "=&r" (r8) \
- : ASM_INPUT_##nr \
- : "r9", "r10", "r11", "r12", \
- "cr0", "ctr", "lr", "memory"); \
- err = (long int) r0; \
- r3; \
+ : "+r" (r0), "+r" (r3), "+r" (r4), "+r" (r5), "+r" (r6), \
+ "+r" (r7), "+r" (r8) \
+ : : "r9", "r10", "r11", "r12", "cr0", "ctr", "lr", "memory"); \
+ err = (long int) r0; \
+ __asm__ __volatile__ ("" : "=r" (rval) : "r" (r3)); \
+ rval; \
})
#undef INLINE_SYSCALL