bug#20567: guile 2.0.11 - test failures on mips64 big-endian with n32 ABI

bug#20567: guile 2.0.11 - test failures on mips64 big-endian with n32 ABI

[Schaefer, Frank]

[-- Attachment #1: Type: text/plain, Size: 1374 bytes --]

I've encountered two test failures with guile 2.0.11 on mips64 big-endian, whenever I build with -mabi=n32 (these test failures do not occur with -mabi=64).  At least one of them I have resolved with the attached patch.

First testcase failure: test-ffi (fixed by my patch).  The guile code apparently assumes that sizeof(ffi_arg)<=sizeof(void **).  This is a faulty assumption; it holds on *most* platforms, but with the mips64 n32 ABI, sizeof(void **)==4, and sizeof(ffi_arg)==8.  So every foreign function that returns a pointer is liable to corrupt data, and anyone retrieving the return value as a simple void * is only going to get sign-extension garbage on big-endian (usually NULL).

Second testcase failure: check-guile fails due to 'ERROR: foreign.test: procedure->pointer: qsort - arguments: ((null-pointer-error "pointer->bytevector" "null pointer dereference" () ()))'.  I strongly suspect it's related to the ffi_arg issue mentioned above, but I'm still peeling away macro ugliness to track it down.  If anyone wants to pitch in with additional guidance or patchwork, I'd be happy to listen and test.

(The patch applies against 2.0.11 release or against current HEAD, where it also fixes the new testcases for test-foreign-object-scm and test-foreign-object-c.  HEAD also fails the test-out-of-memory case, though, which precludes further unit tests.)

[-- Attachment #2: 0001-use-ffi_arg-instead-of-void-to-hold-FFI-call-return-.patch --]
[-- Type: application/octet-stream, Size: 1414 bytes --]

From 78ca71b314f4ad62d39d9acc53fabf0c41fa7521 Mon Sep 17 00:00:00 2001
From: Frank Schaefer <frank.schaefer@tekcomms.com>
Date: Wed, 13 May 2015 17:49:29 +0000
Subject: [PATCH] use ffi_arg instead of void * to hold FFI call return values

---
 libguile/foreign.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libguile/foreign.c b/libguile/foreign.c
index 0cab6b8..5179d8e 100644
--- a/libguile/foreign.c
+++ b/libguile/foreign.c
@@ -26,6 +26,7 @@
 #include <alignof.h>
 #include <string.h>
 #include <assert.h>
+#include <stdint.h>
 
 #include "libguile/_scm.h"
 #include "libguile/bytevectors.h"
@@ -969,7 +970,7 @@ pack (const ffi_type * type, const void *loc, int return_value_p)
 	return scm_from_pointer (mem, NULL);
       }
     case FFI_TYPE_POINTER:
-      return scm_from_pointer (*(void **) loc, NULL);
+      return scm_from_pointer ((void *)(intptr_t)(*(ffi_arg *) loc), NULL);
     default:
       abort ();
     }
@@ -1003,7 +1004,7 @@ scm_i_foreign_call (SCM foreign, const SCM *argv)
     arg_size += cif->arg_types[i]->size + cif->arg_types[i]->alignment - 1;
 
   /* Space for argument values, followed by return value.  */
-  data = alloca (arg_size + cif->rtype->size
+  data = alloca (arg_size + max (sizeof(ffi_arg), cif->rtype->size)
 		 + max (sizeof (void *), cif->rtype->alignment));
 
   /* Unpack ARGV to native values, setting ARGV pointers.  */
-- 
2.3.1

bug#20567: guile 2.0.11 - test failures on mips64 big-endian with n32 ABI

[Andy Wingo]

Mark does this ring a bell to you?  I know you have a mip64 n32 machine
that you use sometimes.

Andy

On Wed 13 May 2015 20:31, "Schaefer, Frank" <frank.schaefer@tekcomms.com> writes:

> I've encountered two test failures with guile 2.0.11 on mips64
> big-endian, whenever I build with -mabi=n32 (these test failures do
> not occur with -mabi=64).  At least one of them I have resolved with
> the attached patch.
>
> First testcase failure: test-ffi (fixed by my patch).  The guile code
> apparently assumes that sizeof(ffi_arg)<=sizeof(void **).  This is a
> faulty assumption; it holds on *most* platforms, but with the mips64
> n32 ABI, sizeof(void **)==4, and sizeof(ffi_arg)==8.  So every foreign
> function that returns a pointer is liable to corrupt data, and anyone
> retrieving the return value as a simple void * is only going to get
> sign-extension garbage on big-endian (usually NULL).
>
> Second testcase failure: check-guile fails due to 'ERROR:
> foreign.test: procedure->pointer: qsort - arguments:
> ((null-pointer-error "pointer->bytevector" "null pointer dereference"
> () ()))'.  I strongly suspect it's related to the ffi_arg issue
> mentioned above, but I'm still peeling away macro ugliness to track it
> down.  If anyone wants to pitch in with additional guidance or
> patchwork, I'd be happy to listen and test.
>
> (The patch applies against 2.0.11 release or against current HEAD,
> where it also fixes the new testcases for test-foreign-object-scm and
> test-foreign-object-c.  HEAD also fails the test-out-of-memory case,
> though, which precludes further unit tests.)