Re: Questions about the libffi development process

[Richard Henderson <rth@redhat.com>]

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

On 04/22/2015 10:25 PM, Russell Keith-Magee wrote:
> I can certainly appreciate the problem of having access to appropriate
> hardware for testing, and I'm definitely appreciative of the efforts
> of those (such as yourself) who are doing work that I don't have the
> skills to handle myself. If nothing else, the patch that caused this
> ARMv7 problem also *fixed* a big problem on iOS AARCH64; for that, you
> definitely have my gratitude.
> 
> My concern is that it isn't at all clear how the project as a whole is
> responding to this problem - hence my original question. From the
> perspective of an outside observer - someone who *uses* libffi, and
> can definitely report bugs, but isn't in a good position to fix bugs -
> a patch has been applied to trunk that fails the most basic of
> acceptance tests.

I reject that characterization.

The breakdown I see is that whoever submitted the iOS port in the first place
just dumped the code and left.  They are failing to maintain their port.  In
other projects, that is grounds for simply deleting the port entirely.  On the
basis that users can either maintain the port, or continue using the last
version that did work.  To do otherwise holds the entire project hostage to a
port that no one can (or cares to) maintain.


> I'm asking what, if any,
> processes are in place to make sure that a known critical bug doesn't
> get rolled into a stable release; and what, if any, processes are in
> place to get this bug in front of the eyes of people who *are* in a
> position to fix it.

None.

> That solution (or something close to it) was already proposed on
> ticket #181; while it does solve one compilation problem, it opens a
> whole lot more errors. Compile log follows:

Ok.  Guessing, based on other changes I've made for clang on other ports, the
following has a chance of working.  Sadly, fedora 21 clang is totally
mis-configured so I can't easily test that on arm-linux.


r~

[-- Attachment #2: d-arm-clang --]
[-- Type: text/plain, Size: 5849 bytes --]

diff --git a/src/arm/sysv.S b/src/arm/sysv.S
index fd16589..db70a7d 100644
--- a/src/arm/sysv.S
+++ b/src/arm/sysv.S
@@ -52,11 +52,12 @@
 #endif
 
 /* Conditionally compile unwinder directives.  */
-.macro UNWIND text:vararg
 #ifdef __ARM_EABI__
-	\text
+# define UNWIND(...)	__VA_ARGS__
+#else
+# define UNWIND(...)
 #endif	
-.endm
+
 #if defined(HAVE_AS_CFI_PSEUDO_OP) && defined(__ARM_EABI__)
 	.cfi_sections	.debug_frame
 #endif
@@ -77,21 +78,29 @@
 # define TYPE(X, Y)
 #endif
 
-#define ARM_FUNC_START(name, gl) \
-	.align	3; \
-	.ifne gl; .globl CNAME(name); FFI_HIDDEN(CNAME(name)); .endif; \
-	TYPE(name, %function); \
+#define ARM_FUNC_START_LOCAL(name)	\
+	.align	3;			\
+	TYPE(CNAME(name), %function);	\
 	CNAME(name):
 
+#define ARM_FUNC_START(name)		\
+	.globl CNAME(name);		\
+	FFI_HIDDEN(CNAME(name));	\
+	ARM_FUNC_START_LOCAL(name)
+
 #define ARM_FUNC_END(name) \
 	SIZE(name)
 
 /* Aid in defining a jump table with 8 bytes between entries.  */
-.macro E index
-	.if . - 0b - 8*\index
-	.error "type table out of sync"
+/* ??? The clang assembler doesn't handle .if with symbolic expressions.  */
+#ifdef __clang__
+# define E(index)
+#else
+# define E(index)				\
+	.if . - 0b - 8*index;			\
+	.error "type table out of sync";	\
 	.endif
-.endm
+#endif
 
 	.text
 	.syntax unified
@@ -111,8 +120,8 @@
 	@ r2:   fn
 	@ r3:	vfp_used
 
-ARM_FUNC_START(ffi_call_VFP, 1)
-	UNWIND	.fnstart
+ARM_FUNC_START(ffi_call_VFP)
+	UNWIND(.fnstart)
 	cfi_startproc
 
 	cmp	r3, #3			@ load only d0 if possible
@@ -122,14 +131,14 @@ ARM_FUNC_START(ffi_call_VFP, 1)
 	/* FALLTHRU */
 ARM_FUNC_END(ffi_call_VFP)
 
-ARM_FUNC_START(ffi_call_SYSV, 1)
+ARM_FUNC_START(ffi_call_SYSV)
 	stm	r1, {fp, lr}
 	mov	fp, r1
 
 	@ This is a bit of a lie wrt the origin of the unwind info, but
 	@ now we've got the usual frame pointer and two saved registers.
-	UNWIND	.save {fp,lr}
-	UNWIND	.setfp fp, sp
+	UNWIND(.save {fp,lr})
+	UNWIND(.setfp fp, sp)
 	cfi_def_cfa(fp, 8)
 	cfi_rel_offset(fp, 0)
 	cfi_rel_offset(lr, 4)
@@ -154,29 +163,29 @@ ARM_FUNC_START(ffi_call_SYSV, 1)
 	add	pc, pc, r3, lsl #3
 	nop
 0:
-E ARM_TYPE_VFP_S
+E(ARM_TYPE_VFP_S)
 	stc	p10, cr0, [r2]		@ vstr s0, [r2]
 	pop	{fp,pc}
-E ARM_TYPE_VFP_D
+E(ARM_TYPE_VFP_D)
 	stc	p11, cr0, [r2]		@ vstr d0, [r2]
 	pop	{fp,pc}
-E ARM_TYPE_VFP_N
+E(ARM_TYPE_VFP_N)
 	stc	p11, cr0, [r2], {8}	@ vstm r2, {d0-d3}
 	pop	{fp,pc}
-E ARM_TYPE_INT64
+E(ARM_TYPE_INT64)
 	str	r1, [r2, #4]
 	nop
-E ARM_TYPE_INT
+E(ARM_TYPE_INT)
 	str	r0, [r2]
 	pop	{fp,pc}
-E ARM_TYPE_VOID
+E(ARM_TYPE_VOID)
 	pop	{fp,pc}
 	nop
-E ARM_TYPE_STRUCT
+E(ARM_TYPE_STRUCT)
 	pop	{fp,pc}
 
 	cfi_endproc
-	UNWIND	.fnend
+	UNWIND(.fnend)
 ARM_FUNC_END(ffi_call_SYSV)
 
 
@@ -184,7 +193,7 @@ ARM_FUNC_END(ffi_call_SYSV)
 	int ffi_closure_inner_* (cif, fun, user_data, frame)
 */
 
-ARM_FUNC_START(ffi_go_closure_SYSV, 1)
+ARM_FUNC_START(ffi_go_closure_SYSV)
 	cfi_startproc
 	stmdb	sp!, {r0-r3}			@ save argument regs
 	cfi_adjust_cfa_offset(16)
@@ -195,8 +204,8 @@ ARM_FUNC_START(ffi_go_closure_SYSV, 1)
 	cfi_endproc
 ARM_FUNC_END(ffi_go_closure_SYSV)
 
-ARM_FUNC_START(ffi_closure_SYSV, 1)
-	UNWIND	.fnstart
+ARM_FUNC_START(ffi_closure_SYSV)
+	UNWIND(.fnstart)
 	cfi_startproc
 	stmdb	sp!, {r0-r3}			@ save argument regs
 	cfi_adjust_cfa_offset(16)
@@ -212,7 +221,7 @@ ARM_FUNC_START(ffi_closure_SYSV, 1)
 	/* Remember that EABI unwind info only applies at call sites.
 	   We need do nothing except note the save of the stack pointer
 	   and the link registers.  */
-	UNWIND	.save {sp,lr}
+	UNWIND(.save {sp,lr})
 	cfi_adjust_cfa_offset(8)
 	cfi_rel_offset(lr, 4)
 
@@ -224,10 +233,10 @@ ARM_FUNC_START(ffi_closure_SYSV, 1)
 	adr	r3, CNAME(ffi_closure_ret)
 	add	pc, r3, r0, lsl #3
 	cfi_endproc
-	UNWIND	.fnend
+	UNWIND(.fnend)
 ARM_FUNC_END(ffi_closure_SYSV)
 
-ARM_FUNC_START(ffi_go_closure_VFP, 1)
+ARM_FUNC_START(ffi_go_closure_VFP)
 	cfi_startproc
 	stmdb	sp!, {r0-r3}			@ save argument regs
 	cfi_adjust_cfa_offset(16)
@@ -238,8 +247,8 @@ ARM_FUNC_START(ffi_go_closure_VFP, 1)
 	cfi_endproc
 ARM_FUNC_END(ffi_go_closure_VFP)
 
-ARM_FUNC_START(ffi_closure_VFP, 1)
-	UNWIND	.fnstart
+ARM_FUNC_START(ffi_closure_VFP)
+	UNWIND(.fnstart)
 	cfi_startproc
 	stmdb	sp!, {r0-r3}			@ save argument regs
 	cfi_adjust_cfa_offset(16)
@@ -254,7 +263,7 @@ ARM_FUNC_START(ffi_closure_VFP, 1)
 	stmdb	sp!, {ip,lr}
 
 	/* See above.  */
-	UNWIND	.save {sp,lr}
+	UNWIND(.save {sp,lr})
 	cfi_adjust_cfa_offset(8)
 	cfi_rel_offset(lr, 4)
 
@@ -266,37 +275,37 @@ ARM_FUNC_START(ffi_closure_VFP, 1)
 	adr	r3, CNAME(ffi_closure_ret)
 	add	pc, r3, r0, lsl #3
 	cfi_endproc
-	UNWIND	.fnend
+	UNWIND(.fnend)
 ARM_FUNC_END(ffi_closure_VFP)
 
 /* Load values returned in registers for both closure entry points.
    Note that we use LDM with SP in the register set.  This is deprecated
    by ARM, but not yet unpredictable.  */
 
-ARM_FUNC_START(ffi_closure_ret, 0)
+ARM_FUNC_START_LOCAL(ffi_closure_ret)
 	cfi_startproc
 	cfi_rel_offset(sp, 0)
 	cfi_rel_offset(lr, 4)
 0:
-E ARM_TYPE_VFP_S
+E(ARM_TYPE_VFP_S)
 	ldc	p10, cr0, [r2]			@ vldr s0, [r2]
 	ldm	sp, {sp,pc}
-E ARM_TYPE_VFP_D
+E(ARM_TYPE_VFP_D)
 	ldc	p11, cr0, [r2]			@ vldr d0, [r2]
 	ldm	sp, {sp,pc}
-E ARM_TYPE_VFP_N
+E(ARM_TYPE_VFP_N)
 	ldc	p11, cr0, [r2], {8}		@ vldm r2, {d0-d3}
 	ldm	sp, {sp,pc}
-E ARM_TYPE_INT64
+E(ARM_TYPE_INT64)
 	ldr	r1, [r2, #4]
 	nop
-E ARM_TYPE_INT
+E(ARM_TYPE_INT)
 	ldr	r0, [r2]
 	ldm	sp, {sp,pc}
-E ARM_TYPE_VOID
+E(ARM_TYPE_VOID)
 	ldm	sp, {sp,pc}
 	nop
-E ARM_TYPE_STRUCT
+E(ARM_TYPE_STRUCT)
 	ldm	sp, {sp,pc}
 	cfi_endproc
 ARM_FUNC_END(ffi_closure_ret)
@@ -319,10 +328,11 @@ ARM_FUNC_START(ffi_closure_trampoline_table_page)
 	ldr	ip, [pc, #-4092]
 	ldr	pc, [pc, #-4092]
 .endr
+ARM_FUNC_END(ffi_closure_trampoline_table_page)
 
 #else
 
-ARM_FUNC_START(ffi_arm_trampoline, 1)
+ARM_FUNC_START(ffi_arm_trampoline)
 0:	adr	ip, 0b
 	ldr	pc, 1f
 1:	.long	0