[LIBFFI] Re: Re: [PATCH] Add support for PaX enable kernels (MPROTECT)

[LIBFFI] Re: Re: [PATCH] Add support for PaX enable kernels (MPROTECT)

[Dave Korn]

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

On 07/11/2012 00:14, Magnus Granberg wrote:

> 2012-11-07  Magnus Granberg  <zorry@gentoo...
>                     Pavel Labushev  <pavel.labushev@runbox...
> 
>        * configure.ac: Add --enable-pax_emutramp for PaX enable kernels.
>        * src/closures.c: Add emutramp_enabled_check. Don't mmap with PROT_EXEC
>           on PaX enable Kernels.
>        * README: Add description for --enable-pax_emutramp.
>        * fficonfig.h.in: Rebuilt.
>        * configure.ac: Rebuilt.

    Hi lists,

  There was a small problem with this (upstream relative to gcc) libffi
patch(*).  The entire #ifdef FFI_MMAP_EXEC_EMUTRAMP_PAX clause is contained
within an outer #if !defined(X86_WIN32) && !defined(X86_WIN64) clause.  That
means that Windows platforms don't get the default definition of
is_emutramp_enabled() supplied by the #else clause.  However,
is_emutramp_enabled() is unconditionally referenced in dlmmap(), and without
this default definition Windows targets fail to compile.

  The attached patch fixes this by moving the #else clause with the default
is_emutramp_enabled() definition to a standalone #ifndef clause outside any
enclosing conditional compilation test.  I couldn't think of a better way to
do it; the #if !(windows) clause is followed by a #elif (cygwin/interix)
clause, so I'd have had to put a default definition in there and also a second
one in a subsequent unconditional #else if I didn't move it out of the
enclosing #if scope altogether.

  Gcc-patches: Assuming AG approves, can we commit this without waiting for an
upstream libffi release and doing a full merge?  Currently GCC HEAD won't
build libffi (and hence libjava) without it.

2013-02-21  Dave Korn  <dave.korn.cygwin@gmail.com>

	* src/closures.c (is_emutramp_enabled [!FFI_MMAP_EXEC_EMUTRAMP_PAX]):
	Move default definition outside enclosing #if scope.

    cheers,
      DaveK
-- 
(*) - Patch: http://sourceware.org/ml/libffi-discuss/2012/msg00269.html
- Thread: http://sourceware.org/ml/libffi-discuss/2012/threads.html#00247

[-- Attachment #2: libffi-emutramp-fix.diff --]
[-- Type: text/x-c, Size: 850 bytes --]

Index: src/closures.c
===================================================================
--- src/closures.c	(revision 196167)
+++ src/closures.c	(working copy)
@@ -189,8 +189,6 @@ emutramp_enabled_check (void)
 
 #define is_emutramp_enabled() (emutramp_enabled >= 0 ? emutramp_enabled \
                                : (emutramp_enabled = emutramp_enabled_check ()))
-#else
-#define is_emutramp_enabled() 0
 #endif /* FFI_MMAP_EXEC_EMUTRAMP_PAX */
 
 #elif defined (__CYGWIN__) || defined(__INTERIX)
@@ -202,6 +200,10 @@ emutramp_enabled_check (void)
 
 #endif /* !defined(X86_WIN32) && !defined(X86_WIN64) */
 
+#ifndef FFI_MMAP_EXEC_EMUTRAMP_PAX
+#define is_emutramp_enabled() 0
+#endif /* FFI_MMAP_EXEC_EMUTRAMP_PAX */
+
 /* Declare all functions defined in dlmalloc.c as static.  */
 static void *dlmalloc(size_t);
 static void dlfree(void*);

Re: [LIBFFI] Re: Re: [PATCH] Add support for PaX enable kernels (MPROTECT)

[Anthony Green]

On Thu, Feb 21, 2013 at 2:22 PM, Dave Korn <dave.korn.cygwin@gmail.com> wrote:
>   Gcc-patches: Assuming AG approves, can we commit this without waiting for an
> upstream libffi release and doing a full merge?  Currently GCC HEAD won't
> build libffi (and hence libjava) without it.

This patch looks fine, thanks.  I don't plan to merge back into GCC
for at least a week or two, so I think you should commit it to the GCC
tree independently.

This means that 3.0.12 is broken for Cygwin.  Are you able to produce
test results once you apply this change?  I should probably issue a
quick 3.0.13 if the results are decent.

Thanks,

AG


>
> 2013-02-21  Dave Korn  <dave.korn.cygwin@gmail.com>
>
>         * src/closures.c (is_emutramp_enabled [!FFI_MMAP_EXEC_EMUTRAMP_PAX]):
>         Move default definition outside enclosing #if scope.
>
>     cheers,
>       DaveK
> --
> (*) - Patch: http://sourceware.org/ml/libffi-discuss/2012/msg00269.html
> - Thread: http://sourceware.org/ml/libffi-discuss/2012/threads.html#00247

Re: [LIBFFI] Re: Re: [PATCH] Add support for PaX enable kernels (MPROTECT)

[Dave Korn]

On 21/02/2013 19:35, Anthony Green wrote:
> On Thu, Feb 21, 2013 at 2:22 PM, Dave Korn wrote:
>>   Gcc-patches: Assuming AG approves, can we commit this without waiting for an
>> upstream libffi release and doing a full merge?  Currently GCC HEAD won't
>> build libffi (and hence libjava) without it.
> 
> This patch looks fine, thanks.  I don't plan to merge back into GCC
> for at least a week or two, so I think you should commit it to the GCC
> tree independently.
> 
> This means that 3.0.12 is broken for Cygwin.  Are you able to produce
> test results once you apply this change?  I should probably issue a
> quick 3.0.13 if the results are decent.

  Yes, the tests run fine (using libffi git HEAD from yesterday):

> Native configuration is i686-pc-cygwin
> 
>                 === libffi tests ===
> 
> 
> Running target unix
> FAIL: libffi.call/closure_thiscall.c (test for excess errors)
> WARNING: libffi.call/closure_thiscall.c compilation failed to produce executable
> 
> FAIL: libffi.call/closure_thiscall.c (test for excess errors)
> WARNING: libffi.call/closure_thiscall.c compilation failed to produce executable
> 
> FAIL: libffi.call/closure_thiscall.c (test for excess errors)
> WARNING: libffi.call/closure_thiscall.c compilation failed to produce executable
> 
> FAIL: libffi.call/closure_thiscall.c (test for excess errors)
> WARNING: libffi.call/closure_thiscall.c compilation failed to produce executable
> 
> FAIL: libffi.call/closure_thiscall.c (test for excess errors)
> WARNING: libffi.call/closure_thiscall.c compilation failed to produce executable
> 
> 
>                 === libffi Summary ===
> 
> # of expected passes            1924
> # of unexpected failures        5

  I was using gcc-4.5.3, which is from before thiscall support was added, so
those compile failures are unremarkable and expected.  Given that, we have a
clean sweep.

    cheers,
      DaveK

Re: [LIBFFI] Re: Re: [PATCH] Add support for PaX enable kernels (MPROTECT)

[Dave Korn]

On 21/02/2013 19:35, Anthony Green wrote:

> This patch looks fine, thanks.  I don't plan to merge back into GCC
> for at least a week or two, so I think you should commit it to the GCC
> tree independently.

  Committed to GCC revision 196527.  Thanks!

    cheers,
      DaveK