[klibc] Bug#334917: [PATCH] Re: klibc barfs on m68k syscall interface

maximilian attems max at stro.at
Sat Jan 29 08:38:10 PST 2011


On Sat, Jan 29, 2011 at 04:18:21PM +0000, Thorsten Glaser wrote:
> 
> I’ve fixed the m68k syscall of klibc and made it able to use
> six-argument syscalls like mmap2. However, I could not yet
> fully test it (only mostly; opendir() specifically fails) due
> to: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47533
 
well ok thanks for the effort, but please really do use git.
The klibc git repo is at 
http://git.kernel.org/?p=libs/klibc/klibc.git;a=summary

git clone it and work away.
 
> +--- klibc-1.5.21.orig/usr/klibc/arch/m68k/syscall.S	2011-01-29 13:28:39.000000000 +0100
> ++++ klibc-1.5.21/usr/klibc/arch/m68k/syscall.S	2011-01-29 15:47:09.000000000 +0100
> +@@ -11,17 +11,47 @@
> + 	.globl	__syscall_common
> + 	.type	__syscall_common, @function
> + __syscall_common:
> +-	movem.l %d2-%d6, -(%sp)	/* 5 registers saved */
> +-	movem.l	24(%sp), %d1-%d6
> ++#if 0
> ++	/* debugging: define "int32_t last_syscall;" in your code... */
> ++	move.l	%d0, (last_syscall)	/* to see which is actually run */
> ++#endif
please no no ugly ifdefs.
> ++	/*
> ++	 * According to eglibc, separate moves are faster than movem;
> ++	 * speed is important and this code is not duplicated anyway,
> ++	 * so we do the same here. We use %a1 as scratch register for
> ++	 * saving; syscall arguments are to be in %d1 to %d5 and %a0.
> ++	 */
> ++	move.l	24(%sp), %a0		/* orig.sp+24: arg 6 */
> ++	move.l	%d5, -(%sp)		/* push d5 (callee saved) */
> ++	move.l	24(%sp), %d5		/* orig.sp+20: arg 5 */
> ++	move.l	%d4, -(%sp)		/* push d4 (callee saved) */
> ++	move.l	24(%sp), %d4		/* orig.sp+16: arg 4 */
> ++	move.l	%d3, -(%sp)		/* push d3 (callee saved) */
> ++	move.l	24(%sp), %d3		/* orig.sp+12: arg 3 */
> ++	move.l	%d2, %a1		/* save d2 (callee saved) in a1 */
> ++	move.l	20(%sp), %d2		/* orig.sp+8:  arg 2 */
> ++	move.l	16(%sp), %d1		/* orig.sp+4:  arg 1 */
> + 	trap	#0
> +-	cmpi.l	#-4095, %d0
> +-	blt.l	1f
> ++	move.l	%a1, %d2		/* restore d2 from a1 (scratch) */
> ++	move.l	(%sp)+, %d3		/* pop d3..d5, see above */
> ++	move.l	(%sp)+, %d4
> ++	move.l	(%sp)+, %d5
> ++
> ++	/* syscall is done, result in %d0, registers are restored */
> ++	.globl	__syscall_checkandout
> ++__syscall_checkandout:
> ++	/* now check for error */
> ++	cmp.l	#-4095, %d0
> ++	bcs.l	1f			/* jmp short if _not_ error */
> ++
> ++	/* prepare for error return */
> + 	neg.l	%d0
> + 	move.l	%d0, (errno)
> +-	moveq	#-1, %d0
> +-1:
> +-	movea.l	%d0, %a0	/* Redundant return */
> +-	movem.l (%sp)+, %d2-%d6 /* Restore registers */
> ++	move.l	#-1, %d0
> ++	/* fallthrough to common return path */
> ++
> ++1:	/* copy return value to %a0 for syscalls returning pointers */
> ++	move.l	%d0, %a0
> + 	rts
> + 
> + 	.size	__syscall_common,.-__syscall_common

sorry but impossible to review as a patch in a patch,
please use format-patch.
> +Index: klibc-1.5.21/usr/klibc/README.klibc
> +===================================================================
> +--- klibc-1.5.21.orig/usr/klibc/README.klibc	2011-01-29 14:49:53.000000000 +0100
> ++++ klibc-1.5.21/usr/klibc/README.klibc	2011-01-29 14:53:57.000000000 +0100
> +@@ -44,7 +44,7 @@
> +    i386:	 Working
> +    ia64:	 Working static, shared untested
> +    m32r:	 Untested
> +-   m68k:	 Untested
> ++   m68k:	 Working
> +    m68knommu:	 Not yet ported
> +    mips:	 Working
> +    mips64:	 Not yet ported

not according your message and up to the maintainer, please.

> +Index: klibc-1.5.21/usr/klibc/arch/m68k/vfork.S
> +===================================================================
> +--- klibc-1.5.21.orig/usr/klibc/arch/m68k/vfork.S	2011-01-29 14:47:11.000000000 +0100
> ++++ klibc-1.5.21/usr/klibc/arch/m68k/vfork.S	2011-01-29 15:28:51.000000000 +0100
> +@@ -15,14 +15,9 @@
> + 	move.l	(%sp)+, %d1		/* Return address */
> + 	move.l	# __NR_vfork, %d0
> + 	trap	#0
> +-	move.l	%d1, -(%sp)
> +-	cmpi.l	#-4095, %d0
> +-	blt.l	1f
> +-	neg.l	%d0
> +-	move.l	%d0, (errno)
> +-	moveq	#-1, %d0
> +-1:
> +-	movea.l	%d0, %a0
> +-	rts
> ++	move.l	%d1, -(%sp)		/* restore stack */
> ++
> ++	/* common code from syscall.S */
> ++	bra	__syscall_checkandout
> + 
> + 	.size	vfork, .-vfork

same critique here, please work against git.

I don't take arch specific patches in Debian unless there
is no way around, so please repost the patches considering
aboves critiques.

thanks.

-- 
maks



More information about the klibc mailing list