<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">2013/5/30 Ruben Van Boxem <span dir="ltr"><<a href="mailto:vanboxem.ruben@gmail.com" target="_blank">vanboxem.ruben@gmail.com</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">2013/5/25 Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span><br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><div>On Fri, May 24, 2013 at 5:53 PM, Ruben Van Boxem<br>
<<a href="mailto:vanboxem.ruben@gmail.com" target="_blank">vanboxem.ruben@gmail.com</a>> wrote:<br>
> Hi,<br>
><br>
> I submitted this patch a long while ago, together with a bunch of other<br>
> changes (some of which got applied back then).<br>
><br>
> If compiler-rt is in the LLVM tree, a Windows build chokes on this little<br>
> bit.<br>
><br>
> The relevant test passed back then, but I can't check this right now, as<br>
> LLVM is being difficult (some errors I need to figure out) and compiler-rt<br>
> has since moved to be a subproject requiring the LLVM tree to be built.<br>
><br>
><br>
> Please comment or apply. Thanks!<br>
><br>
> Ruben<br>
><br>
> PS: I'm not subscribed, please keep me in the CC list<br>
><br>
</div></div>> Index: lib/enable_execute_stack.c<br>
> ===================================================================<br>
> --- lib/enable_execute_stack.c (revision 182667)<br>
> +++ lib/enable_execute_stack.c (working copy)<br>
> @@ -10,7 +10,11 @@<br>
><br>
>  #include "int_lib.h"<br>
><br>
> +#ifndef _WIN32<br>
>  #include <sys/mman.h><br>
> +#else<br>
> +#include <windows.h><br>
> +#endif<br>
><br>
>  /* #include "config.h"<br>
>   * FIXME: CMake - include when cmake system is ready.<br>
> @@ -38,7 +42,7 @@<br>
><br>
>  void __enable_execute_stack(void* addr)<br>
>  {<br>
> -<br>
> +#ifndef _WIN32<br>
>  #if __APPLE__<br>
>   /* On Darwin, pagesize is always 4096 bytes */<br>
>   const uintptr_t pageSize = 4096;<br>
> @@ -54,6 +58,14 @@<br>
>   unsigned char* endPage = (unsigned char*)((p+TRAMPOLINE_SIZE+pageSize) & pageAlignMask);<br>
>   size_t length = endPage - startPage;<br>
>   (void) mprotect((void *)startPage, length, PROT_READ | PROT_WRITE | PROT_EXEC);<br>
> +#else<br>
<br>
Why not simply #elif defined(_WIN32) and skip the extra level of #ifs?<br></blockquote><div><br></div></div></div><div>There is an #if __APPLE__ for this part:<div class="im"><br>#if __APPLE__<br>    /* On Darwin, pagesize is always 4096 bytes */<br>

    const uintptr_t pageSize = 4096;<br></div>#elif !defined(HAVE_SYSCONF)<br>#error "HAVE_SYSCONF not defined! See enable_execute_stack.c"<br>#else<br>        const uintptr_t pageSize = sysconf(_SC_PAGESIZE);<br>
#endif /* __APPLE__ */<br>
<br></div><div>followed by the generic Unix implementation. Neither applies to Win32, so I ifdef'ed the whole thing.<br><br></div><div class="im"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">


<br>
> +    MEMORY_BASIC_INFORMATION b;<br>
> +<br>
> +    if (!VirtualQuery(addr, &b, sizeof(b)))<br>
> +     exit(1);<br>
> +   if (!VirtualProtect(b.BaseAddress, b.RegionSize, PAGE_EXECUTE_READWRITE, &b.Protect))<br>
> +     exit(1);<br>
> +#endif /* _WIN32 */<br>
>  }<br>
<br>
Aside from that, patch LGTM.<br></blockquote><div><br></div></div><div>Great!<br><br>Ruben<br> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">


<span><font color="#888888"><br>
~Aaron<br>
</font></span></blockquote></div><br></div></div></blockquote><div><br></div><div>Friendly ping :)<br></div></div><br></div></div>