llvm dev - Jun 2013 - [LLVMdev] [PATCH] Windows implementation of enable_execute_stack

2013/5/30 Ruben Van Boxem <vanboxem.ruben at gmail.com>
> 2013/5/25 Aaron Ballman <aaron at aaronballman.com>
>
>> On Fri, May 24, 2013 at 5:53 PM, Ruben Van Boxem
>> <vanboxem.ruben at gmail.com> wrote:
>> > Hi,
>> >
>> > I submitted this patch a long while ago, together with a bunch of
other
>> > changes (some of which got applied back then).
>> >
>> > If compiler-rt is in the LLVM tree, a Windows build chokes on this
>> little
>> > bit.
>> >
>> > The relevant test passed back then, but I can't check this
right now, as
>> > LLVM is being difficult (some errors I need to figure out) and
>> compiler-rt
>> > has since moved to be a subproject requiring the LLVM tree to be
built.
>> >
>> >
>> > Please comment or apply. Thanks!
>> >
>> > Ruben
>> >
>> > PS: I'm not subscribed, please keep me in the CC list
>> >
>> > Index: lib/enable_execute_stack.c
>> >
==================================================================>> >
--- lib/enable_execute_stack.c (revision 182667)
>> > +++ lib/enable_execute_stack.c (working copy)
>> > @@ -10,7 +10,11 @@
>> >
>> >  #include "int_lib.h"
>> >
>> > +#ifndef _WIN32
>> >  #include <sys/mman.h>
>> > +#else
>> > +#include <windows.h>
>> > +#endif
>> >
>> >  /* #include "config.h"
>> >   * FIXME: CMake - include when cmake system is ready.
>> > @@ -38,7 +42,7 @@
>> >
>> >  void __enable_execute_stack(void* addr)
>> >  {
>> > -
>> > +#ifndef _WIN32
>> >  #if __APPLE__
>> >   /* On Darwin, pagesize is always 4096 bytes */
>> >   const uintptr_t pageSize = 4096;
>> > @@ -54,6 +58,14 @@
>> >   unsigned char* endPage = (unsigned
>> char*)((p+TRAMPOLINE_SIZE+pageSize) & pageAlignMask);
>> >   size_t length = endPage - startPage;
>> >   (void) mprotect((void *)startPage, length, PROT_READ |
PROT_WRITE |
>> PROT_EXEC);
>> > +#else
>>
>> Why not simply #elif defined(_WIN32) and skip the extra level of #ifs?
>>
>
> There is an #if __APPLE__ for this part:
>
> #if __APPLE__
>     /* On Darwin, pagesize is always 4096 bytes */
>     const uintptr_t pageSize = 4096;
> #elif !defined(HAVE_SYSCONF)
> #error "HAVE_SYSCONF not defined! See enable_execute_stack.c"
> #else
>         const uintptr_t pageSize = sysconf(_SC_PAGESIZE);
> #endif /* __APPLE__ */
>
> followed by the generic Unix implementation. Neither applies to Win32, so
> I ifdef'ed the whole thing.
>
>
>> > +    MEMORY_BASIC_INFORMATION b;
>> > +
>> > +    if (!VirtualQuery(addr, &b, sizeof(b)))
>> > +     exit(1);
>> > +   if (!VirtualProtect(b.BaseAddress, b.RegionSize,
>> PAGE_EXECUTE_READWRITE, &b.Protect))
>> > +     exit(1);
>> > +#endif /* _WIN32 */
>> >  }
>>
>> Aside from that, patch LGTM.
>>
>
> Great!
>
> Ruben
>
>
>>
>> ~Aaron
>>
>
>
Friendly ping :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130625/b7baef68/attachment.html>