Questions about the libffi development process

Questions about the libffi development process

[Russell Keith-Magee]

I've got 2 questions about the libffi development process.

1. A recent commit:

https://github.com/atgreen/libffi/commit/a4b785ea

(authored by Richard Henderson) appears to have broken ARMv7 support
to the point where libffi doesn't build. (Or, at least, it raises a
syntax error on iOS - but I can't see any reason the same problem
wouldn't exist for other ARM platforms).

The problem has been logged as #181:

https://github.com/atgreen/libffi/issues/181

Unfortunately, I don't have the expertise to fix this problem myself,
and it's not clear from the discussion that the problem has been
acknowledged by anyone in a position to fix it, or that it will be
considered a release blocking bug for a future release.

Is there anything I can/should do to bring attention to this problem
and/or mark it as a showstopper (other than what has already been done
on the ticket and in this email)?

2. I can see the version number in the README was bumped to 4 just
after the 3.2.1 release - but what is the state of planning around a
formal v4 release (or whatever the next version will be)? Is there a
vague target date? A TODO list to be cleared off? Or just general
"whenever it's ready" guidance?

Yours,
Russ Magee %-)

Re: Questions about the libffi development process

[Richard Henderson]

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

On 04/17/2015 08:29 PM, Russell Keith-Magee wrote:
> I've got 2 questions about the libffi development process.
> 
> 1. A recent commit:
> 
> https://github.com/atgreen/libffi/commit/a4b785ea
> 
> (authored by Richard Henderson) appears to have broken ARMv7 support
> to the point where libffi doesn't build. (Or, at least, it raises a
> syntax error on iOS - but I can't see any reason the same problem
> wouldn't exist for other ARM platforms).

It certainly does build for armv7l-unknown-linux-gnueabihf.  But then, only iOS
defines FFI_EXEC_TRAMPOLINE_TABLE, wherein the trouble lies.

The development process relies on those having access to proprietary operating
systems fixing the problems that arise on those systems.  I don't have access
to iOS.

That said, the problem appears to be trivial.  Please try the following.


r~

[-- Attachment #2: z --]
[-- Type: text/plain, Size: 931 bytes --]

diff --git a/src/arm/ffi.c b/src/arm/ffi.c
index 9c8732d..f030a0c 100644
--- a/src/arm/ffi.c
+++ b/src/arm/ffi.c
@@ -537,7 +537,7 @@ void ffi_go_closure_VFP (void) FFI_HIDDEN;
 #include <stdio.h>
 #include <stdlib.h>
 
-extern void *ffi_closure_trampoline_table_page;
+extern int ffi_closure_trampoline_table_page[];
 
 typedef struct ffi_trampoline_table ffi_trampoline_table;
 typedef struct ffi_trampoline_table_entry ffi_trampoline_table_entry;
diff --git a/src/arm/sysv.S b/src/arm/sysv.S
index fd16589..6b3ad21 100644
--- a/src/arm/sysv.S
+++ b/src/arm/sysv.S
@@ -313,12 +313,13 @@ ARM_FUNC_END(ffi_closure_ret)
    keep all the magic numbers the same within ffi.c.  */
 
 	.align	12
-ARM_FUNC_START(ffi_closure_trampoline_table_page)
+ARM_FUNC_START(ffi_closure_trampoline_table_page, 1)
 .rept	4096 / 12
 	nop
 	ldr	ip, [pc, #-4092]
 	ldr	pc, [pc, #-4092]
 .endr
+ARM_FUNC_END(ffi_closure_trampoline_table_page)
 
 #else
 

Re: Questions about the libffi development process

[Russell Keith-Magee]

Hi Richard,

Thanks for taking a look at this.

On Wed, Apr 22, 2015 at 2:11 AM, Richard Henderson <rth@redhat.com> wrote:
>
> On 04/17/2015 08:29 PM, Russell Keith-Magee wrote:
> > I've got 2 questions about the libffi development process.
> >
> > 1. A recent commit:
> >
> > https://github.com/atgreen/libffi/commit/a4b785ea
> >
> > (authored by Richard Henderson) appears to have broken ARMv7 support
> > to the point where libffi doesn't build. (Or, at least, it raises a
> > syntax error on iOS - but I can't see any reason the same problem
> > wouldn't exist for other ARM platforms).
>
> It certainly does build for armv7l-unknown-linux-gnueabihf.  But then, only iOS
> defines FFI_EXEC_TRAMPOLINE_TABLE, wherein the trouble lies.
>
> The development process relies on those having access to proprietary operating
> systems fixing the problems that arise on those systems.  I don't have access
> to iOS.

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. The code doesn't compile on a platform that,
according to the documentation, is supported. Prior to the patch in
question being applied, the code *did* compile. The problem has been
reported in a ticket, but there's no response on the ticket that
indicates that this is (or would be considered) a "release blocker" -
or even that the existence of the bug has been acknowledged.

I'm not asking for the magic code faeries to fix the problem for me
(although that would certainly be nice!). 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.

> That said, the problem appears to be trivial.  Please try the following.

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:

====
libtool: compile:  xcrun -sdk iphoneos clang -arch armv7
-DHAVE_CONFIG_H -I. -I.. -I. -I../include -Iinclude -I../src -I.
-I../include -Iinclude -I../src -miphoneos-version-min=7.0 -MT
src/arm/sysv.lo -MD -MP -MF src/arm/.deps/sysv.Tpo -c
../src/arm/sysv.S  -fno-common -DPIC -o src/arm/.libs/sysv.o
../src/arm/sysv.S:55:19: error: expected identifier in '.macro' directive
.macro UNWIND text:vararg
                  ^
../src/arm/sysv.S:59:6: error: unexpected '.endm' in file, no current
macro definition
.endm
     ^
../src/arm/sysv.S:102:2: error: .arch directive not valid for Mach-O
 .arch armv5t
 ^
../src/arm/sysv.S:114:11: error: unknown directive
.align 3; .ifne 1; .globl _ffi_call_VFP;; .endif;; _ffi_call_VFP:
          ^
../src/arm/sysv.S:114:43: error: Encountered a .endif that doesn't
follow a .if or .else
.align 3; .ifne 1; .globl _ffi_call_VFP;; .endif;; _ffi_call_VFP:
                                          ^
../src/arm/sysv.S:115:2: error: invalid instruction
 UNWIND .fnstart
 ^
../src/arm/sysv.S:119:8: error: invalid operand for instruction
 ldcle p11, cr0, [r0] @ vldrle d0, [sp]
       ^
../src/arm/sysv.S:120:8: error: invalid operand for instruction
 ldcgt p11, cr0, [r0], {16} @ vldmgt sp, {d0-d7}
       ^
../src/arm/sysv.S:125:11: error: unknown directive
.align 3; .ifne 1; .globl _ffi_call_SYSV;; .endif;; _ffi_call_SYSV:
          ^
../src/arm/sysv.S:125:44: error: Encountered a .endif that doesn't
follow a .if or .else
.align 3; .ifne 1; .globl _ffi_call_SYSV;; .endif;; _ffi_call_SYSV:
                                           ^
../src/arm/sysv.S:131:15: error: unexpected token in argument list
 UNWIND .save {fp,lr}
              ^
../src/arm/sysv.S:132:16: error: unexpected token in argument list
 UNWIND .setfp fp, sp
               ^
<instantiation>:1:5: error: expected absolute expression
.if . - 0b - 8*0
    ^
../src/arm/sysv.S:157:1: note: while in macro instantiation
E 0
^
<instantiation>:2:2: error: unknown directive
 .error "type table out of sync"
 ^
../src/arm/sysv.S:157:1: note: while in macro instantiation
E 0
^
../src/arm/sysv.S:158:6: error: invalid operand for instruction
 stc p10, cr0, [r2] @ vstr s0, [r2]
     ^
<instantiation>:1:5: error: expected absolute expression
.if . - 0b - 8*1
    ^
../src/arm/sysv.S:160:1: note: while in macro instantiation
E 1
^
<instantiation>:2:2: error: unknown directive
 .error "type table out of sync"
 ^
../src/arm/sysv.S:160:1: note: while in macro instantiation
E 1
^
../src/arm/sysv.S:161:6: error: invalid operand for instruction
 stc p11, cr0, [r2] @ vstr d0, [r2]
     ^
<instantiation>:1:5: error: expected absolute expression
.if . - 0b - 8*2
    ^
../src/arm/sysv.S:163:1: note: while in macro instantiation
E 2
^
<instantiation>:2:2: error: unknown directive
 .error "type table out of sync"
 ^
../src/arm/sysv.S:163:1: note: while in macro instantiation
E 2
^
../src/arm/sysv.S:164:6: error: invalid operand for instruction
 stc p11, cr0, [r2], {8} @ vstm r2, {d0-d3}
     ^
<instantiation>:1:5: error: expected absolute expression
.if . - 0b - 8*3
    ^
../src/arm/sysv.S:166:1: note: while in macro instantiation
E 3
^
<instantiation>:2:2: error: unknown directive
 .error "type table out of sync"
 ^
../src/arm/sysv.S:166:1: note: while in macro instantiation
E 3
^
<instantiation>:1:5: error: expected absolute expression
.if . - 0b - 8*4
    ^
../src/arm/sysv.S:169:1: note: while in macro instantiation
E 4
^
<instantiation>:2:2: error: unknown directive
 .error "type table out of sync"
 ^
../src/arm/sysv.S:169:1: note: while in macro instantiation
E 4
^
<instantiation>:1:5: error: expected absolute expression
.if . - 0b - 8*5
    ^
../src/arm/sysv.S:172:1: note: while in macro instantiation
E 5
^
<instantiation>:2:2: error: unknown directive
 .error "type table out of sync"
 ^
../src/arm/sysv.S:172:1: note: while in macro instantiation
E 5
^
<instantiation>:1:5: error: expected absolute expression
.if . - 0b - 8*6
    ^
../src/arm/sysv.S:175:1: note: while in macro instantiation
E 6
^
<instantiation>:2:2: error: unknown directive
 .error "type table out of sync"
 ^
../src/arm/sysv.S:175:1: note: while in macro instantiation
E 6
^
../src/arm/sysv.S:179:2: error: invalid instruction
 UNWIND .fnend
 ^
../src/arm/sysv.S:187:11: error: unknown directive
.align 3; .ifne 1; .globl _ffi_go_closure_SYSV;; .endif;; _ffi_go_closure_SYSV:
          ^
../src/arm/sysv.S:187:50: error: Encountered a .endif that doesn't
follow a .if or .else
.align 3; .ifne 1; .globl _ffi_go_closure_SYSV;; .endif;; _ffi_go_closure_SYSV:
                                                 ^
../src/arm/sysv.S:198:11: error: unknown directive
.align 3; .ifne 1; .globl _ffi_closure_SYSV;; .endif;; _ffi_closure_SYSV:
          ^
../src/arm/sysv.S:198:47: error: Encountered a .endif that doesn't
follow a .if or .else
.align 3; .ifne 1; .globl _ffi_closure_SYSV;; .endif;; _ffi_closure_SYSV:
                                              ^
../src/arm/sysv.S:199:2: error: invalid instruction
 UNWIND .fnstart
 ^
../src/arm/sysv.S:215:15: error: unexpected token in argument list
 UNWIND .save {sp,lr}
              ^
../src/arm/sysv.S:227:2: error: invalid instruction
 UNWIND .fnend
 ^
../src/arm/sysv.S:230:11: error: unknown directive
.align 3; .ifne 1; .globl _ffi_go_closure_VFP;; .endif;; _ffi_go_closure_VFP:
          ^
../src/arm/sysv.S:230:49: error: Encountered a .endif that doesn't
follow a .if or .else
.align 3; .ifne 1; .globl _ffi_go_closure_VFP;; .endif;; _ffi_go_closure_VFP:
                                                ^
../src/arm/sysv.S:241:11: error: unknown directive
.align 3; .ifne 1; .globl _ffi_closure_VFP;; .endif;; _ffi_closure_VFP:
          ^
../src/arm/sysv.S:241:46: error: Encountered a .endif that doesn't
follow a .if or .else
.align 3; .ifne 1; .globl _ffi_closure_VFP;; .endif;; _ffi_closure_VFP:
                                             ^
../src/arm/sysv.S:242:2: error: invalid instruction
 UNWIND .fnstart
 ^
../src/arm/sysv.S:253:6: error: invalid operand for instruction
 stc p11, cr0, [sp], {16} @ vstm sp, {d0-d7}
     ^
../src/arm/sysv.S:257:15: error: unexpected token in argument list
 UNWIND .save {sp,lr}
              ^
../src/arm/sysv.S:269:2: error: invalid instruction
 UNWIND .fnend
 ^
../src/arm/sysv.S:276:11: error: unknown directive
.align 3; .ifne 0; .globl _ffi_closure_ret;; .endif;; _ffi_closure_ret:
          ^
../src/arm/sysv.S:276:46: error: Encountered a .endif that doesn't
follow a .if or .else
.align 3; .ifne 0; .globl _ffi_closure_ret;; .endif;; _ffi_closure_ret:
                                             ^
<instantiation>:1:5: error: expected absolute expression
.if . - 0b - 8*0
    ^
../src/arm/sysv.S:281:1: note: while in macro instantiation
E 0
^
<instantiation>:2:2: error: unknown directive
 .error "type table out of sync"
 ^
../src/arm/sysv.S:281:1: note: while in macro instantiation
E 0
^
../src/arm/sysv.S:282:6: error: invalid operand for instruction
 ldc p10, cr0, [r2] @ vldr s0, [r2]
     ^
<instantiation>:1:5: error: expected absolute expression
.if . - 0b - 8*1
    ^
../src/arm/sysv.S:284:1: note: while in macro instantiation
E 1
^
<instantiation>:2:2: error: unknown directive
 .error "type table out of sync"
 ^
../src/arm/sysv.S:284:1: note: while in macro instantiation
E 1
^
../src/arm/sysv.S:285:6: error: invalid operand for instruction
 ldc p11, cr0, [r2] @ vldr d0, [r2]
     ^
<instantiation>:1:5: error: expected absolute expression
.if . - 0b - 8*2
    ^
../src/arm/sysv.S:287:1: note: while in macro instantiation
E 2
^
<instantiation>:2:2: error: unknown directive
 .error "type table out of sync"
 ^
../src/arm/sysv.S:287:1: note: while in macro instantiation
E 2
^
../src/arm/sysv.S:288:6: error: invalid operand for instruction
 ldc p11, cr0, [r2], {8} @ vldm r2, {d0-d3}
     ^
<instantiation>:1:5: error: expected absolute expression
.if . - 0b - 8*3
    ^
../src/arm/sysv.S:290:1: note: while in macro instantiation
E 3
^
<instantiation>:2:2: error: unknown directive
 .error "type table out of sync"
 ^
../src/arm/sysv.S:290:1: note: while in macro instantiation
E 3
^
<instantiation>:1:5: error: expected absolute expression
.if . - 0b - 8*4
    ^
../src/arm/sysv.S:293:1: note: while in macro instantiation
E 4
^
<instantiation>:2:2: error: unknown directive
 .error "type table out of sync"
 ^
../src/arm/sysv.S:293:1: note: while in macro instantiation
E 4
^
<instantiation>:1:5: error: expected absolute expression
.if . - 0b - 8*5
    ^
../src/arm/sysv.S:296:1: note: while in macro instantiation
E 5
^
<instantiation>:2:2: error: unknown directive
 .error "type table out of sync"
 ^
../src/arm/sysv.S:296:1: note: while in macro instantiation
E 5
^
<instantiation>:1:5: error: expected absolute expression
.if . - 0b - 8*6
    ^
../src/arm/sysv.S:299:1: note: while in macro instantiation
E 6
^
<instantiation>:2:2: error: unknown directive
 .error "type table out of sync"
 ^
../src/arm/sysv.S:299:1: note: while in macro instantiation
E 6
^
../src/arm/sysv.S:316:11: error: unknown directive
.align 3; .ifne 1; .globl _ffi_closure_trampoline_table_page;;
.endif;; _ffi_closure_trampoline_table_page:
          ^
../src/arm/sysv.S:316:64: error: Encountered a .endif that doesn't
follow a .if or .else
.align 3; .ifne 1; .globl _ffi_closure_trampoline_table_page;;
.endif;; _ffi_closure_trampoline_table_page:
                                                               ^
make[2]: *** [src/arm/sysv.lo] Error 1
make[1]: *** [all-recursive] Error 1
make: *** [all] Error 2
===

A lot of these errors look to me like a fundamental difference in the
allowed syntax for ARM assembler - .ifne, .arch and .error look like
the biggest offenders, and comparing the last version of sysv.S that
did compile, that code had none of those directives. However, I
haven't touched assembler since pre-university days, so I could be
completely wrong.

Yours,
Russ Magee %-)

Re: Questions about the libffi development process

[Richard Henderson]

[-- 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

Re: Questions about the libffi development process

[Russell Keith-Magee]

Hi Richard,

On Fri, Apr 24, 2015 at 3:13 AM, Richard Henderson <rth@redhat.com> wrote:
> 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 can't argue with that. Deleting the port would certainly be an
option. It obviously wouldn't be my *preferred* option, but it's
obviously *an* option.

That said, I don't think it's fair to call their contributions
"dumped" or "unmaintained". I can't speak for the person who
originally submitted the iOS port, but iOS support on ARMv7 has been
working fine for several years. It has received 100% of the
maintenance it has required. Prior to this recent refactor, the last
changes to sysv.S were in Feb 2014, Dec 2013, and April 2012. On that
basis, I don't think it's out of the realm of possibility that the
original contributors aren't actively watching trunk libffi
development. They may not even be aware that this change has landed.
To cast blame on them for not being immediately aware that someone
else has merged a patch that massively refactors (and inadvertently
breaks) code that was working fine is a little unfair to everyone
involved.

Is there a good record of who is notionally "responsible" for iOS
support? Based on the git logs, Zachary Waldowski and David Schneider
would seem to be the most likely candidates.

>> 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.

None at all? There's no release candidate process? No release testing
procedures?

>> 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.

It certainly cleans up a lot of the compilation problems. Updated log follows:

===

libtool: compile:  xcrun -sdk iphoneos clang -arch armv7
-DHAVE_CONFIG_H -I. -I.. -I. -I../include -Iinclude -I../src -I.
-I../include -Iinclude -I../src -miphoneos-version-min=7.0 -MT
src/arm/sysv.lo -MD -MP -MF src/arm/.deps/sysv.Tpo -c
../src/arm/sysv.S  -fno-common -DPIC -o src/arm/.libs/sysv.o
../src/arm/sysv.S:111:2: error: .arch directive not valid for Mach-O
 .arch armv5t
 ^
../src/arm/sysv.S:128:8: error: invalid operand for instruction
 ldcle p11, cr0, [r0] @ vldrle d0, [sp]
       ^
../src/arm/sysv.S:129:8: error: invalid operand for instruction
 ldcgt p11, cr0, [r0], {16} @ vldmgt sp, {d0-d7}
       ^
../src/arm/sysv.S:167:6: error: invalid operand for instruction
 stc p10, cr0, [r2] @ vstr s0, [r2]
     ^
../src/arm/sysv.S:170:6: error: invalid operand for instruction
 stc p11, cr0, [r2] @ vstr d0, [r2]
     ^
../src/arm/sysv.S:173:6: error: invalid operand for instruction
 stc p11, cr0, [r2], {8} @ vstm r2, {d0-d3}
     ^
../src/arm/sysv.S:262:6: error: invalid operand for instruction
 stc p11, cr0, [sp], {16} @ vstm sp, {d0-d7}
     ^
../src/arm/sysv.S:291:6: error: invalid operand for instruction
 ldc p10, cr0, [r2] @ vldr s0, [r2]
     ^
../src/arm/sysv.S:294:6: error: invalid operand for instruction
 ldc p11, cr0, [r2] @ vldr d0, [r2]
     ^
../src/arm/sysv.S:297:6: error: invalid operand for instruction
 ldc p11, cr0, [r2], {8} @ vldm r2, {d0-d3}
     ^
===

To my untrained eye, this looks like there is only 2 underlying problems:
 * .arch being an invalid directive.
 * p10 and p11 being invalid operands for ldcle, ldcgt, ldc and stc

Again, I can't thank you enough for taking a look at this problem,
even though it's outside your bailiwick.

Yours,
Russ Magee %-)

Re: Questions about the libffi development process

[Richard Henderson]

On 04/23/2015 03:49 PM, Russell Keith-Magee wrote:
> None at all? There's no release candidate process? No release testing
> procedures?

I Anthony will have to answer that.

> libtool: compile:  xcrun -sdk iphoneos clang -arch armv7
> -DHAVE_CONFIG_H -I. -I.. -I. -I../include -Iinclude -I../src -I.
> -I../include -Iinclude -I../src -miphoneos-version-min=7.0 -MT
> src/arm/sysv.lo -MD -MP -MF src/arm/.deps/sysv.Tpo -c
> ../src/arm/sysv.S  -fno-common -DPIC -o src/arm/.libs/sysv.o
> ../src/arm/sysv.S:111:2: error: .arch directive not valid for Mach-O
>  .arch armv5t

Any chance you know what is the base architecture for iOS on ARM?

> ../src/arm/sysv.S:128:8: error: invalid operand for instruction
>  ldcle p11, cr0, [r0] @ vldrle d0, [sp]
>        ^

Try replacing these with the comment (text after @) and see if it assembles.

The generic "coprocessor load" instructions are used instead of the more
specific vfp load instructions in order to support armv7hf at run-time while
not requiring more than arm5t at compile-time.  Something that makes sense for
ELF, but I guess not for iOS.


r~

Re: Questions about the libffi development process

[Russell Keith-Magee]

(Apologies to Richard - this is a resend; the first version was
silently rejected by the mailing list because of sourceware.org's
anachronistic, broken policies on HTML mail)

On Sat, Apr 25, 2015 at 12:28 AM, Richard Henderson <rth@redhat.com> wrote:
> On 04/23/2015 03:49 PM, Russell Keith-Magee wrote:
>> None at all? There's no release candidate process? No release testing
>> procedures?
>
> I Anthony will have to answer that.
>
>> libtool: compile:  xcrun -sdk iphoneos clang -arch armv7
>> -DHAVE_CONFIG_H -I. -I.. -I. -I../include -Iinclude -I../src -I.
>> -I../include -Iinclude -I../src -miphoneos-version-min=7.0 -MT
>> src/arm/sysv.lo -MD -MP -MF src/arm/.deps/sysv.Tpo -c
>> ../src/arm/sysv.S  -fno-common -DPIC -o src/arm/.libs/sysv.o
>> ../src/arm/sysv.S:111:2: error: .arch directive not valid for Mach-O
>>  .arch armv5t
>
> Any chance you know what is the base architecture for iOS on ARM?

As far as I can make out, they're all ARMv7-A.

The iPhone 5C and 5 both use an Apple A7 Swift [1][2]; The iPhone 4S
and 5th generation iPod Touches use an ARM Cortex-A9 [3][4]; the
iPhone4 is an ARM Cortex-A8 [5]; according to [6].

[1] http://en.wikipedia.org/wiki/IPhone_5C
[2] http://en.wikipedia.org/wiki/IPhone_5
[3] http://en.wikipedia.org/wiki/IPhone_4S
[4] http://en.wikipedia.org/wiki/IPod_Touch_(5th_generation)
[5] http://en.wikipedia.org/wiki/IPhone_4
[6] http://en.wikipedia.org/wiki/List_of_ARM_microarchitectures

>> ../src/arm/sysv.S:128:8: error: invalid operand for instruction
>>  ldcle p11, cr0, [r0] @ vldrle d0, [sp]
>>        ^
>
> Try replacing these with the comment (text after @) and see if it assembles.
>
> The generic "coprocessor load" instructions are used instead of the more
> specific vfp load instructions in order to support armv7hf at run-time while
> not requiring more than arm5t at compile-time.  Something that makes sense for
> ELF, but I guess not for iOS.

With those hints and and some #ifdef __clang__ clauses, I've got the
code compiling. It works for simple function calls (e.g., calls to get
system time), but it fails at runtime with a segfault in
ffi_closure_inner_SYSV when you try to call a function using a
function pointer as an argument (in my test case, a call to the posix
qsort() method to sort a list of integers).

My patch, as it currently stands, is in a branch on Github if anyone
wants to help.

https://github.com/freakboy3742/libffi/tree/t181

Yours,
Russ Magee %-)