From: Erik M. Bray <erik.bray@lri.fr>
Date: Tue, 17 Oct 2017 08:50:03 +0000 (+0300)
Subject: Use mprotect instead of mmap in GC_unmap() on Cygwin
X-Git-Tag: v8.0.0~546
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=97579ed6bc1946247d99ae599c6418b38aa54578;p=gc

Use mprotect instead of mmap in GC_unmap() on Cygwin

Issue #187 (BDWGC).

* configure.ac [$enable_munmap!="" && $enable_munmap!=no]
(USE_WINALLOC): Replace AC_DEFINE with AH_TEMPLATE; remove comment
about a workaround for Cygwin; remove "if $host=*-*-cygwin*".
* os_dep.c [CYGWIN32 && USE_MUNMAP] (GC_setpagesize): Set GC_page_size
to dwAllocationGranularity (instead of dwPageSize); add comment; add
assertion that dwAllocationGranularity is not less than dwPageSize.
* os_dep.c [USE_MUNMAP && CYGWIN32 && !USE_WINALLOC] (GC_unmap,
GC_unmap_gap): Call mprotect() instead of mmap(); add comment.
---

diff --git a/configure.ac b/configure.ac
index cb314a65..c379695f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -887,14 +887,9 @@ AC_ARG_ENABLE(munmap,
 if test x$enable_munmap != x -a x$enable_munmap != xno; then
     AC_DEFINE([USE_MMAP], 1,
               [Define to use mmap instead of sbrk to expand the heap.])
-    case "$host" in
-      *-*-cygwin*)
-        # Workaround for Cygwin: use VirtualAlloc since mmap(PROT_NONE) fails
-        AC_DEFINE([USE_WINALLOC], 1,
+    AH_TEMPLATE([USE_WINALLOC],
                   [Define to use Win32 VirtualAlloc (instead of sbrk or
                    mmap) to expand the heap.])
-        ;;
-    esac
     AC_DEFINE([USE_MUNMAP], 1,
               [Define to return memory to OS with munmap calls
                (see doc/README.macros).])
diff --git a/os_dep.c b/os_dep.c
index 6b009fca..2ca6f0a0 100644
--- a/os_dep.c
+++ b/os_dep.c
@@ -713,7 +713,20 @@ GC_INNER size_t GC_page_size = 0;
   GC_INNER void GC_setpagesize(void)
   {
     GetSystemInfo(&GC_sysinfo);
-    GC_page_size = (size_t)GC_sysinfo.dwPageSize;
+#   if defined(CYGWIN32) && defined(USE_MUNMAP)
+      /* Allocations made with mmap() are aligned to the allocation     */
+      /* granularity, which (at least on 64-bit Windows OS) is not the  */
+      /* same as the page size.  Probably a separate variable could     */
+      /* be added to distinguish the allocation granularity from the    */
+      /* actual page size, but in practice there is no good reason to   */
+      /* make allocations smaller than dwAllocationGranularity, so we   */
+      /* just use it instead of the actual page size here (as Cygwin    */
+      /* itself does in many cases).                                    */
+      GC_page_size = (size_t)GC_sysinfo.dwAllocationGranularity;
+      GC_ASSERT(GC_page_size >= (size_t)GC_sysinfo.dwPageSize);
+#   else
+      GC_page_size = (size_t)GC_sysinfo.dwPageSize;
+#   endif
 #   if defined(MSWINCE) && !defined(_WIN32_WCE_EMULATION)
       {
         OSVERSIONINFO verInfo;
@@ -2481,17 +2494,25 @@ GC_INNER void GC_unmap(ptr_t start, size_t bytes)
       /* We immediately remap it to prevent an intervening mmap from    */
       /* accidentally grabbing the same address space.                  */
       {
-        void * result;
-
-        result = mmap(start_addr, len, PROT_NONE,
-                      MAP_PRIVATE | MAP_FIXED | OPT_MAP_ANON,
-                      zero_fd, 0/* offset */);
-        if (result != (void *)start_addr)
-          ABORT("mmap(PROT_NONE) failed");
-#       if defined(CPPCHECK) || defined(LINT2)
-          /* Explicitly store the resource handle to a global variable. */
-          GC_noop1((word)result);
-#       endif
+#       ifdef CYGWIN32
+          /* Calling mmap() with the new protection flags on an         */
+          /* existing memory map with MAP_FIXED is broken on Cygwin.    */
+          /* However, calling mprotect() on the given address range     */
+          /* with PROT_NONE seems to work fine.                         */
+          if (mprotect(start_addr, len, PROT_NONE))
+            ABORT("mprotect(PROT_NONE) failed");
+#       else
+          void * result = mmap(start_addr, len, PROT_NONE,
+                               MAP_PRIVATE | MAP_FIXED | OPT_MAP_ANON,
+                               zero_fd, 0/* offset */);
+
+          if (result != (void *)start_addr)
+            ABORT("mmap(PROT_NONE) failed");
+#         if defined(CPPCHECK) || defined(LINT2)
+            /* Explicitly store the resource handle to a global variable. */
+            GC_noop1((word)result);
+#         endif
+#       endif /* !CYGWIN32 */
       }
       GC_unmapped_bytes += len;
 #   endif
@@ -2598,15 +2619,20 @@ GC_INNER void GC_unmap_gap(ptr_t start1, size_t bytes1, ptr_t start2,
 #   else
       if (len != 0) {
         /* Immediately remap as above. */
-        void * result;
-        result = mmap(start_addr, len, PROT_NONE,
-                      MAP_PRIVATE | MAP_FIXED | OPT_MAP_ANON,
-                      zero_fd, 0/* offset */);
-        if (result != (void *)start_addr)
-          ABORT("mmap(PROT_NONE) failed");
-#       if defined(CPPCHECK) || defined(LINT2)
-          GC_noop1((word)result);
-#       endif
+#       ifdef CYGWIN32
+          if (mprotect(start_addr, len, PROT_NONE))
+            ABORT("mprotect(PROT_NONE) failed");
+#       else
+          void * result = mmap(start_addr, len, PROT_NONE,
+                               MAP_PRIVATE | MAP_FIXED | OPT_MAP_ANON,
+                               zero_fd, 0/* offset */);
+
+          if (result != (void *)start_addr)
+            ABORT("mmap(PROT_NONE) failed");
+#         if defined(CPPCHECK) || defined(LINT2)
+            GC_noop1((word)result);
+#         endif
+#       endif /* !CYGWIN32 */
         GC_unmapped_bytes += len;
       }
 #   endif