Commit e58e22b22386ed0e0a95e97eb8eed016e3f01b02
2023-02-02T07:02:53
From Dave Anglin: A couple of years ago the 32-bit hppa targets were converted from using a trampoline executed on the stack to the function descriptor technique used by ia64. This is more efficient and avoids having to have an executable stack. However, function pointers on 32-bit need the PLABEL bit set in the pointer. It distinguishes between pointers that point directly to the executable code and pointer that point to a function descriptor. We need the later for libffi. But as a result, it is not possible to convert using casts data pointers to function pointers. The solution at the time was to set the PLABEL bit in hppa closure pointers using FFI_CLOSURE_PTR. However, I realized recently that this was a bad choice. Packages like python-cffi allocate their own closure pointers, so this isn't going to work well there. A better solution is to leave closure pointers unchanged and only set the PLABEL bit in pointers used to point to executable code. The attached patch drops the FFI_CLOSURE_PTR and FFI_RESTORE_PTR defines. This allows some cleanup in the hppa closure routines. The FFI_FN define is now used to set the PLABEL bit on hppa. ffi_closure_alloc is modified to set the PLABEL bit in the value set in *code. I also added a FFI_CL define to convert a function pointer to a closure pointer. It is only used in one test case.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161
diff --git a/include/ffi.h.in b/include/ffi.h.in
index 8467135..e7fe963 100644
--- a/include/ffi.h.in
+++ b/include/ffi.h.in
@@ -361,14 +361,6 @@ typedef struct {
FFI_API void *ffi_closure_alloc (size_t size, void **code);
FFI_API void ffi_closure_free (void *);
-#if defined(PA_LINUX) || defined(PA_HPUX)
-#define FFI_CLOSURE_PTR(X) ((void *)((unsigned int)(X) | 2))
-#define FFI_RESTORE_PTR(X) ((void *)((unsigned int)(X) & ~3))
-#else
-#define FFI_CLOSURE_PTR(X) (X)
-#define FFI_RESTORE_PTR(X) (X)
-#endif
-
FFI_API ffi_status
ffi_prep_closure (ffi_closure*,
ffi_cif *,
@@ -515,8 +507,14 @@ FFI_API
ffi_status ffi_get_struct_offsets (ffi_abi abi, ffi_type *struct_type,
size_t *offsets);
-/* Useful for eliminating compiler warnings. */
+/* Convert between closure and function pointers. */
+#if defined(PA_LINUX) || defined(PA_HPUX)
+#define FFI_FN(f) ((void (*)(void))((unsigned int)(f) | 2))
+#define FFI_CL(f) ((void *)((unsigned int)(f) & ~3))
+#else
#define FFI_FN(f) ((void (*)(void))f)
+#define FFI_CL(f) ((void *)(f))
+#endif
/* ---- Definitions shared with assembly code ---------------------------- */
diff --git a/src/closures.c b/src/closures.c
index c425277..5c43313 100644
--- a/src/closures.c
+++ b/src/closures.c
@@ -993,23 +993,23 @@ ffi_closure_alloc (size_t size, void **code)
if (!code)
return NULL;
- ptr = FFI_CLOSURE_PTR (dlmalloc (size));
+ ptr = dlmalloc (size);
if (ptr)
{
msegmentptr seg = segment_holding (gm, ptr);
- *code = add_segment_exec_offset (ptr, seg);
+ *code = FFI_FN (add_segment_exec_offset (ptr, seg));
if (!ffi_tramp_is_supported ())
return ptr;
ftramp = ffi_tramp_alloc (0);
if (ftramp == NULL)
{
- dlfree (FFI_RESTORE_PTR (ptr));
+ dlfree (ptr);
return NULL;
}
- *code = ffi_tramp_get_addr (ftramp);
+ *code = FFI_FN (ffi_tramp_get_addr (ftramp));
((ffi_closure *) ptr)->ftramp = ftramp;
}
@@ -1050,7 +1050,7 @@ ffi_closure_free (void *ptr)
if (ffi_tramp_is_supported ())
ffi_tramp_free (((ffi_closure *) ptr)->ftramp);
- dlfree (FFI_RESTORE_PTR (ptr));
+ dlfree (ptr);
}
int
@@ -1070,16 +1070,20 @@ ffi_tramp_is_present (void *ptr)
void *
ffi_closure_alloc (size_t size, void **code)
{
+ void *c;
+
if (!code)
return NULL;
- return *code = FFI_CLOSURE_PTR (malloc (size));
+ c = malloc (size);
+ *code = FFI_FN (c);
+ return c;
}
void
ffi_closure_free (void *ptr)
{
- free (FFI_RESTORE_PTR (ptr));
+ free (ptr);
}
void *
diff --git a/src/pa/ffi.c b/src/pa/ffi.c
index 186bf69..b99a668 100644
--- a/src/pa/ffi.c
+++ b/src/pa/ffi.c
@@ -445,7 +445,6 @@ ffi_status ffi_closure_inner_pa32(ffi_closure *closure, UINT32 *stack)
int i, avn;
unsigned int slot = FIRST_ARG_SLOT;
register UINT32 r28 asm("r28");
- ffi_closure *c = (ffi_closure *)FFI_RESTORE_PTR (closure);
cif = closure->cif;
@@ -548,7 +547,7 @@ ffi_status ffi_closure_inner_pa32(ffi_closure *closure, UINT32 *stack)
}
/* Invoke the closure. */
- (c->fun) (cif, rvalue, avalue, c->user_data);
+ (closure->fun) (cif, rvalue, avalue, closure->user_data);
debug(3, "after calling function, ret[0] = %08x, ret[1] = %08x\n", u.ret[0],
u.ret[1]);
@@ -649,8 +648,6 @@ ffi_prep_closure_loc (ffi_closure* closure,
void *user_data,
void *codeloc)
{
- ffi_closure *c = (ffi_closure *)FFI_RESTORE_PTR (closure);
-
/* The layout of a function descriptor. A function pointer with the PLABEL
bit set points to a function descriptor. */
struct pa32_fd
@@ -676,14 +673,14 @@ ffi_prep_closure_loc (ffi_closure* closure,
fd = (struct pa32_fd *)((UINT32)ffi_closure_pa32 & ~3);
/* Setup trampoline. */
- tramp = (struct ffi_pa32_trampoline_struct *)c->tramp;
+ tramp = (struct ffi_pa32_trampoline_struct *)closure->tramp;
tramp->code_pointer = fd->code_pointer;
tramp->fake_gp = (UINT32)codeloc & ~3;
tramp->real_gp = fd->gp;
- c->cif = cif;
- c->user_data = user_data;
- c->fun = fun;
+ closure->cif = cif;
+ closure->user_data = user_data;
+ closure->fun = fun;
return FFI_OK;
}
diff --git a/testsuite/libffi.closures/closure_loc_fn0.c b/testsuite/libffi.closures/closure_loc_fn0.c
index 4f2f4f8..6f2e3d5 100644
--- a/testsuite/libffi.closures/closure_loc_fn0.c
+++ b/testsuite/libffi.closures/closure_loc_fn0.c
@@ -85,7 +85,7 @@ int main (void)
#ifndef FFI_EXEC_STATIC_TRAMP
/* With static trampolines, the codeloc does not point to closure */
- CHECK(memcmp(pcl, codeloc, sizeof(*pcl)) == 0);
+ CHECK(memcmp(pcl, FFI_CL(codeloc), sizeof(*pcl)) == 0);
#endif
res = (*((closure_loc_test_type0)codeloc))