From 45da2fcbcd0ecaba673275d22b04fac3e4376e22 Mon Sep 17 00:00:00 2001 From: Sergei Trofimovich Date: Sat, 17 Feb 2018 18:53:02 +0000 Subject: [PATCH 1/2] new test: return small struct The bug originally was discovered in https://bugs.gentoo.org/634190 where complicated callback was returning invalid data on ia64. This change adds minimal reproducer that fails only on ia64 as: FAIL: libffi.call/struct10.c -W -Wall -Wno-psabi -O0 execution test FAIL: libffi.call/struct10.c -W -Wall -Wno-psabi -O2 execution test FAIL: libffi.call/struct10.c -W -Wall -Wno-psabi -O3 execution test FAIL: libffi.call/struct10.c -W -Wall -Wno-psabi -Os execution test Test passes on amd64. The fix is in the following commit. Bug: https://bugs.gentoo.org/634190 Signed-off-by: Sergei Trofimovich --- testsuite/Makefile.am | 1 + testsuite/libffi.call/struct10.c | 57 ++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 testsuite/libffi.call/struct10.c diff --git a/testsuite/Makefile.am b/testsuite/Makefile.am index 5eecc57..b367b46 100644 --- a/testsuite/Makefile.am +++ b/testsuite/Makefile.am @@ -68,6 +68,7 @@ libffi.call/float4.c \ libffi.call/return_ldl.c \ libffi.call/closure_fn5.c \ libffi.call/struct6.c libffi.call/return_ll.c libffi.call/struct9.c \ +libffi.call/struct10.c \ libffi.call/return_sc.c libffi.call/struct7.c \ libffi.call/cls_align_uint64.c libffi.call/cls_4byte.c \ libffi.call/cls_6_1_byte.c \ diff --git a/testsuite/libffi.call/struct10.c b/testsuite/libffi.call/struct10.c new file mode 100644 index 0000000..17b1377 --- /dev/null +++ b/testsuite/libffi.call/struct10.c @@ -0,0 +1,57 @@ +/* Area: ffi_call + Purpose: Check structures. + Limitations: none. + PR: none. + Originator: Sergei Trofimovich + + The test originally discovered in ruby's bindings + for ffi in https://bugs.gentoo.org/634190 */ + +/* { dg-do run } */ +#include "ffitest.h" + +struct s { + int s32; + float f32; + signed char s8; +}; + +struct s make_s(void) { + struct s r; + r.s32 = 0x1234; + r.f32 = 7.0; + r.s8 = 0x78; + return r; +} + +int main() { + ffi_cif cif; + struct s r; + ffi_type rtype; + ffi_type* s_fields[] = { + &ffi_type_sint, + &ffi_type_float, + &ffi_type_schar, + NULL, + }; + + rtype.size = 0; + rtype.alignment = 0, + rtype.type = FFI_TYPE_STRUCT, + rtype.elements = s_fields, + + r.s32 = 0xbad; + r.f32 = 999.999; + r.s8 = 0x51; + + // Here we emulate the following call: + //r = make_s(); + + CHECK(ffi_prep_cif(&cif, FFI_DEFAULT_ABI, 0, &rtype, NULL) == FFI_OK); + ffi_call(&cif, FFI_FN(make_s), &r, NULL); + + CHECK(r.s32 == 0x1234); + CHECK(r.f32 == 7.0); + CHECK(r.s8 == 0x78); + exit(0); +} From b58caef7fd620408be9239ac24ea89d5bc84f30b Mon Sep 17 00:00:00 2001 From: Sergei Trofimovich Date: Sat, 17 Feb 2018 19:00:40 +0000 Subject: [PATCH 2/2] ia64: fix small struct return This change fixes libffi.call/struct10.c failure on ia64: FAIL: libffi.call/struct10.c -W -Wall -Wno-psabi -O0 execution test .Lst_small_struct handles returns for structs less than 32 bytes (following ia64 return value ABI [1]). Subroutine does roughly the following: ``` mov [sp+0] = r8 mov [sp+8] = r9 mov [sp+16] = r10 mov [sp+24] = r11 memcpy(destination, source=sp, 12); ``` The problem: ia64 ABI guarantees that top 16 bytes of stack are scratch space for callee function. Thus it can clobber it. [1] says (7.1 Procedure Frames): """ * Scratch area. This 16-byte region is provided as scratch storage for procedures that are called by the current procedure. Leaf procedures do not need to allocate this region. A procedure may use the 16 bytes at the top of its own frame as scratch memory, but the contents of this area are not preserved by a procedure call. """ In our case 16 top bytes are clobbered by a PLT resolver when memcpy() is called for the first time. As a result memcpy implementation reads already clobbered data frop top of stack. The fix is simple: allocate 16 bytes of scrats space prior to memcpy() call. [1]: https://www.intel.com/content/dam/www/public/us/en/documents/guides/itanium-software-runtime-architecture-guide.pdf Bug: https://bugs.gentoo.org/634190 Signed-off-by: Sergei Trofimovich --- src/ia64/unix.S | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/ia64/unix.S b/src/ia64/unix.S index 4733377..e2547e0 100644 --- a/src/ia64/unix.S +++ b/src/ia64/unix.S @@ -175,7 +175,6 @@ ffi_call_unix: ;; .Lst_small_struct: - add sp = -16, sp cmp.lt p6, p0 = 8, in3 cmp.lt p7, p0 = 16, in3 cmp.lt p8, p0 = 24, in3 @@ -191,6 +190,12 @@ ffi_call_unix: (p8) st8 [r18] = r11 mov out1 = sp mov out2 = in3 + ;; + // ia64 software calling convention requires + // top 16 bytes of stack to be scratch space + // PLT resolver uses that scratch space at + // 'memcpy' symbol reolution time + add sp = -16, sp br.call.sptk.many b0 = memcpy# ;; mov ar.pfs = loc0