324181 mmap does not handle MAP_32BIT (handle it now, rather than fail it)

324181 was previously closed with a solution to always make
MAP_32BIT fail. This is technically correct/according to the doc,
but is not very usable.
This patch ensures that MAP_32BIT mmap is succesful, as long as
aspacemgr gives a range in the first 2GB
(so, compared to a native run, MAP_32BIT will fail much more quickly
as aspacemgr does not reserve the address space below 2GB on a 64 bits).

Far to be perfect, but this is better than nothing.

Added a regression test that test succesful mmap 32 bits till
the 2GB limit is reached.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@15341

diff --git a/NEWS b/NEWS
index f9ae0ba8fc25b29884a97d9a8c3e57502d55dd64..c5eb0c286ac664bb247d189af7cd31af4e614458 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -115,6 +115,7 @@ where XXXXXX is the bug number as listed below.
         'sizeof(UWord) == sizeof(UInt)' failed in m_syscall.c
         == 326797
 319274  Fix unhandled syscall: unix:410 (sigsuspend_nocancel) on OS X
+324181  mmap does not handle MAP_32BIT (handle it now, rather than fail it)
 333051  mmap of huge pages fails due to incorrect alignment
         == 339163
 334802  valgrind does not always explain why a given option is bad

diff --git a/coregrind/m_syswrap/syswrap-generic.c b/coregrind/m_syswrap/syswrap-generic.c
index fcf5687b21e74b9cea7d88eddaab232339de335b..3e1a826d09a39cfb89c5ac0d965c1ea33a0d5b2d 100644 (file)
--- a/coregrind/m_syswrap/syswrap-generic.c
+++ b/coregrind/m_syswrap/syswrap-generic.c
@@ -2150,15 +2150,6 @@ ML_(generic_PRE_sys_mmap) ( ThreadId tid,
       return VG_(mk_SysRes_Error)( VKI_EINVAL );
    }
 
-#  if defined(VKI_MAP_32BIT)
-   /* We can't support MAP_32BIT (at least, not without significant
-      complication), and it's royally unportable, so if the client
-      asks for it, just fail it. */
-   if (arg4 & VKI_MAP_32BIT) {
-      return VG_(mk_SysRes_Error)( VKI_ENOMEM );
-   }
-#  endif
-
    /* Figure out what kind of allocation constraints there are
       (fixed/hint/any), and ask aspacem what we should do. */
    mreq.start = arg1;
@@ -2179,12 +2170,37 @@ ML_(generic_PRE_sys_mmap) ( ThreadId tid,
       return VG_(mk_SysRes_Error)( VKI_EINVAL );
    }
 
+#  if defined(VKI_MAP_32BIT)
+   /* MAP_32BIT is royally unportable, so if the client asks for it, try our
+      best to make it work (but without complexifying aspacemgr).
+      If the user requested MAP_32BIT, the mmap-ed space must be in the
+      first 2GB of the address space. So, return ENOMEM if aspacemgr
+      advisory is above the first 2GB. If MAP_FIXED is also requested,
+      MAP_32BIT has to be ignored.
+      Assumption about aspacemgr behaviour: aspacemgr scans the address space
+      from low addresses to find a free segment. No special effort is done
+      to keep the first 2GB 'free' for this MAP_32BIT. So, this will often
+      fail once the program has already allocated significant memory. */
+   if ((arg4 & VKI_MAP_32BIT) && !(arg4 & VKI_MAP_FIXED)) {
+      if (advised + arg2 >= 0x80000000)
+         return VG_(mk_SysRes_Error)( VKI_ENOMEM );
+   }
+#  endif
+
    /* Otherwise we're OK (so far).  Install aspacem's choice of
       address, and let the mmap go through.  */
    sres = VG_(am_do_mmap_NO_NOTIFY)(advised, arg2, arg3,
                                     arg4 | VKI_MAP_FIXED,
                                     arg5, arg6);
 
+#  if defined(VKI_MAP_32BIT)
+   /* No recovery trial if the advisory was not accepted. */
+   if ((arg4 & VKI_MAP_32BIT) && !(arg4 & VKI_MAP_FIXED)
+       && sr_isError(sres)) {
+      return VG_(mk_SysRes_Error)( VKI_ENOMEM );
+   }
+#  endif
+
    /* A refinement: it may be that the kernel refused aspacem's choice
       of address.  If we were originally asked for a hinted mapping,
       there is still a last chance: try again at any address.

diff --git a/none/tests/amd64-linux/Makefile.am b/none/tests/amd64-linux/Makefile.am
index 39d7bac13ede9526c99edd17553375a262be2d88..bfe27ab0bba78d1115d738bfcf3ca0ebf1629714 100644 (file)
--- a/none/tests/amd64-linux/Makefile.am
+++ b/none/tests/amd64-linux/Makefile.am
@@ -5,10 +5,11 @@ dist_noinst_SCRIPTS = \
        filter_stderr filter_minimal
 
 EXTRA_DIST = \
-       bug345887.stderr.exp bug345887.vgtest
+       bug345887.stderr.exp bug345887.vgtest \
+       map_32bits.stderr.exp map_32bits.vgtest
 
 check_PROGRAMS = \
-       bug345887
+       bug345887 map_32bits
 
 AM_CFLAGS    += @FLAG_M64@
 AM_CXXFLAGS  += @FLAG_M64@

diff --git a/none/tests/amd64-linux/map_32bits.c b/none/tests/amd64-linux/map_32bits.c
new file mode 100644 (file)
index 0000000..fbd1d8b
--- /dev/null
+++ b/none/tests/amd64-linux/map_32bits.c
@@ -0,0 +1,43 @@
+#include <stdio.h>
+#include "tests/sys_mman.h"
+#include <stdlib.h>
+#include <unistd.h>
+
+int main()
+{
+
+   void *first = NULL;
+   void *last;
+   void *res;
+
+   while (1) {
+      res = mmap (NULL, 64 * 1024,
+                  PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_32BIT,
+                  -1, 0);
+      if (first == NULL) {
+         first = res;
+         if (first == (void *)-1) {
+            perror ("first mmap");
+            exit (1);
+         }
+         fprintf(stderr, "first mmap : %p\n", first);
+      }
+      if (res == (void *)-1) {
+         fprintf(stderr, "last mmap ok: %p\n", last);
+         break;
+      }
+      last = res;
+   }
+
+   /* And now, retry without MAP_32BIT */
+   res = mmap (NULL, 64 * 1024,
+               PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS,
+               -1, 0);
+   if (res == (void *)-1) {
+      perror ("retry mmap");
+      exit (1);
+   }
+   fprintf(stderr, "retry mmap ok: %p\n", res);
+
+   return 0;
+}

diff --git a/none/tests/amd64-linux/map_32bits.stderr.exp b/none/tests/amd64-linux/map_32bits.stderr.exp
new file mode 100644 (file)
index 0000000..fa15567
--- /dev/null
+++ b/none/tests/amd64-linux/map_32bits.stderr.exp
@@ -0,0 +1,3 @@
+first mmap : 0x........
+last mmap ok: 0x........
+retry mmap ok: 0x........

diff --git a/none/tests/amd64-linux/map_32bits.vgtest b/none/tests/amd64-linux/map_32bits.vgtest
new file mode 100644 (file)
index 0000000..2d03f13
--- /dev/null
+++ b/none/tests/amd64-linux/map_32bits.vgtest
@@ -0,0 +1,5 @@
+prog: map_32bits
+# take a big aspacemgr minaddr, to quickly reach the 2GB limit
+vgopts: -q --aspace-minaddr=0x7ff60000
+stderr_filter: filter_minimal
+