From 8c8cd6c9fcc4fa141a436c57523c741b7f8c4916 Mon Sep 17 00:00:00 2001 From: Philippe Waroquiers Date: Wed, 17 Jun 2015 19:57:09 +0000 Subject: [PATCH] 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 --- NEWS | 1 + coregrind/m_syswrap/syswrap-generic.c | 34 ++++++++++++---- none/tests/amd64-linux/Makefile.am | 5 ++- none/tests/amd64-linux/map_32bits.c | 43 ++++++++++++++++++++ none/tests/amd64-linux/map_32bits.stderr.exp | 3 ++ none/tests/amd64-linux/map_32bits.vgtest | 5 +++ 6 files changed, 80 insertions(+), 11 deletions(-) create mode 100644 none/tests/amd64-linux/map_32bits.c create mode 100644 none/tests/amd64-linux/map_32bits.stderr.exp create mode 100644 none/tests/amd64-linux/map_32bits.vgtest diff --git a/NEWS b/NEWS index f9ae0ba8fc..c5eb0c286a 100644 --- 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 fcf5687b21..3e1a826d09 100644 --- 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 39d7bac13e..bfe27ab0bb 100644 --- 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 index 0000000000..fbd1d8beb3 --- /dev/null +++ b/none/tests/amd64-linux/map_32bits.c @@ -0,0 +1,43 @@ +#include +#include "tests/sys_mman.h" +#include +#include + +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 index 0000000000..fa15567027 --- /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 index 0000000000..2d03f13f8b --- /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 + -- 2.47.3