This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] mips/o32: fix internal_syscall5/6/7



On 16/08/2017 10:44, Joseph Myers wrote:
> On Wed, 16 Aug 2017, Maciej W. Rozycki wrote:
> 
>> On Tue, 15 Aug 2017, Joseph Myers wrote:
>>
>>> In which case having a volatile integer variable with value 4, declaring a 
>>> VLA whose size is that variable, and storing a pointer to that VLA in a 
>>> variable, would be an alternative to alloca to force a frame pointer, but 
>>> with deallocation happening when the scope ends rather than the function 
>>> ending (and the syscall macro has its own scope, so using it inside a loop 
>>> wouldn't be a problem).
>>
>>  I suspect using volatile variables will cause unnecessary memory traffic.  
>> Passing the size specifier through an empty `asm' might give better code; 
>> also I think we can use 0 as the size requested, not to decrease the stack 
>> pointer unnecessarily, e.g.:
> 
> Sure, as long as (a) the compiler can't know the size is actually constant 
> and (b) it can't know the VLA isn't actually used, as if it can tell 
> either of those things it can optimize away the variable stack allocation.
> 
>>  Also I wonder if there's actually a dependable way to have GCC itself 
>> allocate the argument space we require.  For example if we set `s' to 1 
>> above instead for `internal_syscall6', then would `0($sp)' and `4($sp)' be 
>> valid to place arguments #5 and #6 at respectively without the subsequent 
>> $sp adjustment we currently have in the syscall `asm' or would it be UB?
> 
> You can't tell whether the compiler might have allocated other variables 
> on the stack after the dynamic adjustment - that is, whether any 
> particular offset from sp is in fact unused or not.
> 

What about the below? I can use some help to see if I am handling all the
required ABI requirements for the __libc_do_syscall, but on an qemu emulated
system I see no regression on basic tests (including some cancellation one
from glibc to see the syscall is correctly unwinded) and tst-rwlock15 also
does not fail anymore.


diff --git a/sysdeps/unix/sysv/linux/mips/mips32/Makefile b/sysdeps/unix/sysv/linux/mips/mips32/Makefile
index 33b4615..cbdf032 100644
--- a/sysdeps/unix/sysv/linux/mips/mips32/Makefile
+++ b/sysdeps/unix/sysv/linux/mips/mips32/Makefile
@@ -1,8 +1,26 @@
+ifeq ($(subdir),elf)
+sysdep-dl-routines += libc-do-syscall
+endif
+
 ifeq ($(subdir),conform)
 # For bugs 17786 and 21278.
 conformtest-xfail-conds += mips-o32-linux
 endif
 
+ifeq ($(subdir),io)
+sysdep_routines += libc-do-syscall
+endif
+
+ifeq ($(subdir),nptl)
+libpthread-sysdep_routines += libc-do-syscall
+libpthread-shared-only-routines += libc-do-syscall
+endif
+
+ifeq ($(subdir),rt)
+librt-sysdep_routines += libc-do-syscall
+librt-shared-only-routines += libc-do-syscall
+endif
+
 ifeq ($(subdir),stdlib)
 tests += bug-getcontext-mips-gp
 endif
diff --git a/sysdeps/unix/sysv/linux/mips/mips32/libc-do-syscall.S b/sysdeps/unix/sysv/linux/mips/mips32/libc-do-syscall.S
new file mode 100644
index 0000000..a7184d9
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/mips/mips32/libc-do-syscall.S
@@ -0,0 +1,54 @@
+/* Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library.  If not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <sys/asm.h>
+#include <sysdep.h>
+#include <asm/unistd.h>
+#include <sgidefs.h>
+
+
+/* long int __libc_do_syscall (long int, ...)  */
+
+#define FRAMESZ 32
+
+        .text
+        .set    nomips16
+	.hidden __libc_do_syscall
+ENTRY(__libc_do_syscall)
+        move    $2, $4
+        move    $4, $5
+        move    $5, $6
+        move    $6, $7
+        lw      $7, 16(sp)
+        lw      $8, 20(sp)
+        lw      $9, 24(sp)
+        lw      $10,28(sp)
+	.set 	noreorder
+	PTR_SUBU sp, FRAMESZ
+	cfi_adjust_cfa_offset (FRAMESZ)
+        sw      $8, 16(sp)
+        sw      $9, 20(sp)
+        sw      $10,24(sp)
+        syscall
+	PTR_ADDU sp, FRAMESZ
+	cfi_adjust_cfa_offset (-FRAMESZ)
+	.set	reorder
+        beq     $7, $0, 1f
+        subu    $2, $0, $2
+1:      jr      ra
+        nop
+END (__libc_do_syscall)
diff --git a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
index e9e3ee7..3a8920a 100644
--- a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
+++ b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
@@ -121,13 +121,13 @@
 # define INTERNAL_SYSCALL_MIPS16(number, err, nr, args...)		\
 	internal_syscall##nr ("lw\t%0, %2\n\t",				\
 			      "R" (number),				\
-			      0, err, args)
+			      SYS_ify(name), err, args)
 
 #else /* !__mips16 */
 # define INTERNAL_SYSCALL(name, err, nr, args...)			\
 	internal_syscall##nr ("li\t%0, %2\t\t\t# " #name "\n\t",	\
 			      "IK" (SYS_ify (name)),			\
-			      0, err, args)
+			      SYS_ify(name), err, args)
 
 # define INTERNAL_SYSCALL_NCS(number, err, nr, args...)			\
 	internal_syscall##nr (MOVE32 "\t%0, %2\n\t",			\
@@ -136,6 +136,7 @@
 
 #endif /* !__mips16 */
 
+
 #define internal_syscall0(v0_init, input, number, err, dummy...)	\
 ({									\
 	long _sys_result;						\
@@ -262,109 +263,41 @@
 	_sys_result;							\
 })
 
-/* We need to use a frame pointer for the functions in which we
-   adjust $sp around the syscall, or debug information and unwind
-   information will be $sp relative and thus wrong during the syscall.  As
-   of GCC 4.7, this is sufficient.  */
-#define FORCE_FRAME_POINTER						\
-  void *volatile __fp_force __attribute__ ((unused)) = alloca (4)
+long int __libc_do_syscall (long int, ...) attribute_hidden;
 
 #define internal_syscall5(v0_init, input, number, err,			\
 			  arg1, arg2, arg3, arg4, arg5)			\
 ({									\
-	long _sys_result;						\
-									\
-	FORCE_FRAME_POINTER;						\
-	{								\
-	register long __s0 asm ("$16") __attribute__ ((unused))		\
-	  = (number);							\
-	register long __v0 asm ("$2");					\
-	register long __a0 asm ("$4") = (long) (arg1);			\
-	register long __a1 asm ("$5") = (long) (arg2);			\
-	register long __a2 asm ("$6") = (long) (arg3);			\
-	register long __a3 asm ("$7") = (long) (arg4);			\
-	__asm__ volatile (						\
-	".set\tnoreorder\n\t"						\
-	"subu\t$29, 32\n\t"						\
-	"sw\t%6, 16($29)\n\t"						\
-	v0_init								\
-	"syscall\n\t"							\
-	"addiu\t$29, 32\n\t"						\
-	".set\treorder"							\
-	: "=r" (__v0), "+r" (__a3)					\
-	: input, "r" (__a0), "r" (__a1), "r" (__a2),			\
-	  "r" ((long) (arg5))						\
-	: __SYSCALL_CLOBBERS);						\
-	err = __a3;							\
-	_sys_result = __v0;						\
-	}								\
+	long int _sys_result;						\
+	_sys_result = __libc_do_syscall (number, arg1, arg2, arg3,	\
+					 arg4, arg5);			\
+	err = _sys_result > -4096UL ? 1 : 0;				\
+	if (err)							\
+	  _sys_result = -_sys_result;					\
 	_sys_result;							\
 })
 
 #define internal_syscall6(v0_init, input, number, err,			\
 			  arg1, arg2, arg3, arg4, arg5, arg6)		\
 ({									\
-	long _sys_result;						\
-									\
-	FORCE_FRAME_POINTER;						\
-	{								\
-	register long __s0 asm ("$16") __attribute__ ((unused))		\
-	  = (number);							\
-	register long __v0 asm ("$2");					\
-	register long __a0 asm ("$4") = (long) (arg1);			\
-	register long __a1 asm ("$5") = (long) (arg2);			\
-	register long __a2 asm ("$6") = (long) (arg3);			\
-	register long __a3 asm ("$7") = (long) (arg4);			\
-	__asm__ volatile (						\
-	".set\tnoreorder\n\t"						\
-	"subu\t$29, 32\n\t"						\
-	"sw\t%6, 16($29)\n\t"						\
-	"sw\t%7, 20($29)\n\t"						\
-	v0_init								\
-	"syscall\n\t"							\
-	"addiu\t$29, 32\n\t"						\
-	".set\treorder"							\
-	: "=r" (__v0), "+r" (__a3)					\
-	: input, "r" (__a0), "r" (__a1), "r" (__a2),			\
-	  "r" ((long) (arg5)), "r" ((long) (arg6))			\
-	: __SYSCALL_CLOBBERS);						\
-	err = __a3;							\
-	_sys_result = __v0;						\
-	}								\
+	long int _sys_result;						\
+	_sys_result = __libc_do_syscall (number, arg1, arg2, arg3,	\
+					 arg4, arg5, arg6);		\
+	err = _sys_result > -4096UL ? 1 : 0;				\
+	if (err)							\
+	  _sys_result = -_sys_result;					\
 	_sys_result;							\
 })
 
 #define internal_syscall7(v0_init, input, number, err,			\
 			  arg1, arg2, arg3, arg4, arg5, arg6, arg7)	\
 ({									\
-	long _sys_result;						\
-									\
-	FORCE_FRAME_POINTER;						\
-	{								\
-	register long __s0 asm ("$16") __attribute__ ((unused))		\
-	  = (number);							\
-	register long __v0 asm ("$2");					\
-	register long __a0 asm ("$4") = (long) (arg1);			\
-	register long __a1 asm ("$5") = (long) (arg2);			\
-	register long __a2 asm ("$6") = (long) (arg3);			\
-	register long __a3 asm ("$7") = (long) (arg4);			\
-	__asm__ volatile (						\
-	".set\tnoreorder\n\t"						\
-	"subu\t$29, 32\n\t"						\
-	"sw\t%6, 16($29)\n\t"						\
-	"sw\t%7, 20($29)\n\t"						\
-	"sw\t%8, 24($29)\n\t"						\
-	v0_init								\
-	"syscall\n\t"							\
-	"addiu\t$29, 32\n\t"						\
-	".set\treorder"							\
-	: "=r" (__v0), "+r" (__a3)					\
-	: input, "r" (__a0), "r" (__a1), "r" (__a2),			\
-	  "r" ((long) (arg5)), "r" ((long) (arg6)), "r" ((long) (arg7))	\
-	: __SYSCALL_CLOBBERS);						\
-	err = __a3;							\
-	_sys_result = __v0;						\
-	}								\
+	long int _sys_result;						\
+	_sys_result = __libc_do_syscall (number, arg1, arg2, arg3,	\
+					 arg4, arg5, arg6, arg7);	\
+	err = _sys_result > -4096UL ? 1 : 0;				\
+	if (err)							\
+	  _sys_result = -_sys_result;					\
 	_sys_result;							\
 })
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]