Re: [PATCH] Fix PR32843, testsuite bug wrt sign/zero-extension of return values

[Andrew Haley <aph-gcc@littlepinkcloud.COM>]

This fixes the real bug, which is that x86 libffi wasn't correctly
sign/zero extending retvals.  I also changed the long chain of
if/then/elses into a switch and tidied things a bit.

While I was doing this, I found a bug in ffi_call_SYSV: in some cases
we were popping too many values from the stack, but that was being
concealed by a later movl %ebp,%esp in the epilogue.  As it turns out,
there isn't any need to pop the args from the stack after calling the
user's target function, so I just removed the code that did that.

Passes full testsuite on 32/64, no regressions.

Andrew.


2007-07-30  Andrew Haley  <aph@redhat.com>

	PR testsuite/32843
	* src/x86/ffi.c (ffi_prep_cif_machdep): in x86 case, add code for
	signed/unsigned int8/16.
	* src/x86/sysv.S (ffi_call_SYSV): Rewrite to:
	Use a jump table.
	Remove code to pop args from the stack after call.
	Special-case signed/unsigned int8/16.
	* testsuite/libffi.call/return_sc.c (main): Revert.

Index: src/x86/ffi.c
===================================================================
--- src/x86/ffi.c	(revision 127059)
+++ src/x86/ffi.c	(working copy)
@@ -121,7 +121,12 @@
     case FFI_TYPE_VOID:
 #ifdef X86
     case FFI_TYPE_STRUCT:
+    case FFI_TYPE_UINT8:
+    case FFI_TYPE_UINT16:
+    case FFI_TYPE_SINT8:
+    case FFI_TYPE_SINT16:
 #endif
+
     case FFI_TYPE_SINT64:
     case FFI_TYPE_FLOAT:
     case FFI_TYPE_DOUBLE:
Index: src/x86/sysv.S
===================================================================
--- src/x86/sysv.S	(revision 127059)
+++ src/x86/sysv.S	(working copy)
@@ -59,16 +59,15 @@
 
 	call  *28(%ebp)
 
-	/* Remove the space we pushed for the args  */
-	movl  16(%ebp),%ecx
-	addl  %ecx,%esp
-
 	/* Load %ecx with the return type code  */
 	movl  20(%ebp),%ecx	
 
+	/* Protect %esi.  We're going to pop it in the epilogue.  */
+	pushl %esi
+
 	/* If the return value pointer is NULL, assume no return value.  */
 	cmpl  $0,24(%ebp)
-	jne   retint
+	jne  0f
 
 	/* Even if there is no space for the return value, we are 
 	   obliged to handle floating-point values.  */
@@ -78,51 +77,84 @@
 
         jmp   epilogue
 
-retint:
-	cmpl  $FFI_TYPE_INT,%ecx
-	jne   retfloat
-	/* Load %ecx with the pointer to storage for the return value  */
-	movl  24(%ebp),%ecx	
-	movl  %eax,0(%ecx)
-	jmp   epilogue
+0:
+	call  1f
+
+.Lstore_table:
+	.long	noretval-.Lstore_table	/* FFI_TYPE_VOID */
+	.long	retint-.Lstore_table	/* FFI_TYPE_INT */
+	.long	retfloat-.Lstore_table	/* FFI_TYPE_FLOAT */
+	.long	retdouble-.Lstore_table	/* FFI_TYPE_DOUBLE */
+	.long	retlongdouble-.Lstore_table	/* FFI_TYPE_LONGDOUBLE */
+	.long	retuint8-.Lstore_table	/* FFI_TYPE_UINT8 */
+	.long	retsint8-.Lstore_table	/* FFI_TYPE_SINT8 */
+	.long	retuint16-.Lstore_table	/* FFI_TYPE_UINT16 */
+	.long	retsint16-.Lstore_table	/* FFI_TYPE_SINT16 */
+	.long	retint-.Lstore_table	/* FFI_TYPE_UINT32 */
+	.long	retint-.Lstore_table	/* FFI_TYPE_SINT32 */
+	.long	retint64-.Lstore_table	/* FFI_TYPE_UINT64 */
+	.long	retint64-.Lstore_table	/* FFI_TYPE_SINT64 */
+	.long	retstruct-.Lstore_table	/* FFI_TYPE_STRUCT */
+	.long	retint-.Lstore_table	/* FFI_TYPE_POINTER */
+
+1:
+	pop  %esi
+	add  (%esi, %ecx, 4), %esi
+	jmp  *%esi
+
+	/* Sign/zero extend as appropriate.  */
+retsint8:
+	movsbl  %al, %eax
+	jmp  retint
+
+retsint16:
+	movswl  %ax, %eax
+	jmp  retint
+
+retuint8:
+	movzbl  %al, %eax
+	jmp  retint
+
+retuint16:
+	movzwl  %ax, %eax
+	jmp  retint
 
 retfloat:
-	cmpl  $FFI_TYPE_FLOAT,%ecx
-	jne   retdouble
 	/* Load %ecx with the pointer to storage for the return value  */
 	movl  24(%ebp),%ecx	
 	fstps (%ecx)
 	jmp   epilogue
 
 retdouble:
-	cmpl  $FFI_TYPE_DOUBLE,%ecx
-	jne   retlongdouble
 	/* Load %ecx with the pointer to storage for the return value  */
 	movl  24(%ebp),%ecx	
 	fstpl (%ecx)
 	jmp   epilogue
 
 retlongdouble:
-	cmpl  $FFI_TYPE_LONGDOUBLE,%ecx
-	jne   retint64
 	/* Load %ecx with the pointer to storage for the return value  */
 	movl  24(%ebp),%ecx	
 	fstpt (%ecx)
 	jmp   epilogue
 	
 retint64:	
-	cmpl  $FFI_TYPE_SINT64,%ecx
-        jne   retstruct
 	/* Load %ecx with the pointer to storage for the return value  */
 	movl  24(%ebp),%ecx	
 	movl  %eax,0(%ecx)
 	movl  %edx,4(%ecx)
+	jmp   epilogue
 	
+retint:
+	/* Load %ecx with the pointer to storage for the return value  */
+	movl  24(%ebp),%ecx	
+	movl  %eax,0(%ecx)
+
 retstruct:
 	/* Nothing to do!  */
 
 noretval:
 epilogue:
+        popl %esi
         movl %ebp,%esp
         popl %ebp
         ret
@@ -162,7 +194,15 @@
 	movl	-12(%ebp), %ecx
 	cmpl	$FFI_TYPE_INT, %eax
 	je	.Lcls_retint
-	cmpl	$FFI_TYPE_FLOAT, %eax
+
+	/* Handle FFI_TYPE_UINT8, FFI_TYPE_SINT8, FFI_TYPE_UINT16,
+	   FFI_TYPE_SINT16, FFI_TYPE_UINT32, FFI_TYPE_SINT32.  */
+	cmpl	$FFI_TYPE_UINT64, %eax
+	jge	0f
+	cmpl	$FFI_TYPE_UINT8, %eax
+	jge	.Lcls_retint
+	
+0:	cmpl	$FFI_TYPE_FLOAT, %eax
 	je	.Lcls_retfloat
 	cmpl	$FFI_TYPE_DOUBLE, %eax
 	je	.Lcls_retdouble
Index: testsuite/libffi.call/return_sc.c
===================================================================
--- testsuite/libffi.call/return_sc.c	(revision 127059)
+++ testsuite/libffi.call/return_sc.c	(working copy)
@@ -30,7 +30,7 @@
        sc < (signed char) 127; sc++)
     {
       ffi_call(&cif, FFI_FN(return_sc), &rint, values);
-      CHECK((signed char) rint == sc);
+      CHECK(rint == (ffi_arg) sc);
     }
   exit(0);
 }