From a87ee2a7a98dfa2bd4174045071efe5da8663df2 Mon Sep 17 00:00:00 2001 From: Peter Wang Date: Fri, 25 Jul 2014 11:41:49 +1000 Subject: [PATCH] Use mmap for Boehm GC in threaded grades on Linux. The glibc implementation of malloc (commonly used on Linux) can use sbrk to acquire more memory, and so does Boehm GC in its default configuration on Linux. When both allocators are invoked simultaneously from different threads then memory corruption may result. configure.ac: Add --enable-gc-mmap option. Make --enable-gc-munmap imply --enable-gc-mmap. Detect mmap and set MR_HAVE_MMAP. Detect sbrk and set MR_HAVE_SBRK. On Linux (and potentially other platforms), build Boehm GC for threaded grades with -DUSE_MMAP if mmap was found. Mmake.common.in: Add ENABLE_BOEHM_USE_MMAP. runtime/mercury_conf.h.in: Add MR_HAVE_MMAP, although it is not used anywhere yet. Add MR_HAVE_SBRK. It is only for used by the test program. tests/hard_coded/Mmakefile: tests/hard_coded/thread_sbrk.exp: tests/hard_coded/thread_sbrk.exp2: tests/hard_coded/thread_sbrk.m: Add test program. NEWS: Announce changes. (cherry picked from commit d63a294e1fa4b1a91b6c7acd9fe8ba45d9cffb6d) --- Mmake.common.in | 6 +- NEWS | 3 + configure.ac | 58 ++++++++++++--- runtime/mercury_conf.h.in | 6 +- tests/hard_coded/Mmakefile | 1 + tests/hard_coded/thread_sbrk.exp | 21 ++++++ tests/hard_coded/thread_sbrk.exp2 | 1 + tests/hard_coded/thread_sbrk.m | 114 ++++++++++++++++++++++++++++++ 8 files changed, 199 insertions(+), 11 deletions(-) create mode 100644 tests/hard_coded/thread_sbrk.exp create mode 100644 tests/hard_coded/thread_sbrk.exp2 create mode 100644 tests/hard_coded/thread_sbrk.m diff --git a/Mmake.common.in b/Mmake.common.in index c40889110..851b74388 100644 --- a/Mmake.common.in +++ b/Mmake.common.in @@ -169,8 +169,10 @@ GRADESTRING = $(shell $(SCRIPTS_DIR)/canonical_grade $(ALL_GRADEFLAGS)) # MCFLAGS += --no-infer-all --halt-at-warn --no-warn-inferred-erroneous # Options to pass to the C compiler when building Boehm-GC. -BOEHM_CFLAGS = @ENABLE_BOEHM_LARGE_CONFIG@ @ENABLE_BOEHM_USE_MUNMAP@ \ - @ENABLE_BOEHM_XOPEN_SOURCE@ +BOEHM_CFLAGS = @ENABLE_BOEHM_LARGE_CONFIG@ \ + @ENABLE_BOEHM_USE_MMAP@ \ + @ENABLE_BOEHM_USE_MUNMAP@ \ + @ENABLE_BOEHM_XOPEN_SOURCE@ # Additional options to pass to the C compiler when building Boehm-GC for # threads. diff --git a/NEWS b/NEWS index 3ac83d929..15c8eebe9 100644 --- a/NEWS +++ b/NEWS @@ -18,6 +18,9 @@ This is a bug-fix release. * Low-level C parallel grades now work on Windows instead of crashing at startup. (Bug #338) * We now use thread-safe alternatives to strerror(). (Bug #340) +* We have added the configure option --enable-gc-mmap. +* We configure Boehm GC to use mmap in threaded grades on Linux to avoid + conflicts with glibc malloc leading to memory corruption. * A problem that caused string.format/[23] to sometimes return incorrect results when formatting floats with the 'g' conversion specifier has been fixed. This bug only affected the non-C backends. (Bug #342) diff --git a/configure.ac b/configure.ac index bf74335e2..0117d1b41 100644 --- a/configure.ac +++ b/configure.ac @@ -657,9 +657,14 @@ AC_SUBST(ENABLE_BOEHM_LARGE_CONFIG) #-----------------------------------------------------------------------------# # -# Determine whether to define USE_MUNMAP when compiling boehm_gc. +# Determine whether to define USE_MMAP, USE_MUNMAP when compiling boehm_gc. # +AC_ARG_ENABLE(gc-mmap, + AC_HELP_STRING([--enable-gc-mmap], + [use mmap instead of sbrk when using Boehm GC]), + enable_gc_mmap="$enableval",enable_gc_mmap=no) + AC_ARG_ENABLE(gc-munmap, AC_HELP_STRING([--enable-gc-munmap], [enable unmapping of unused pages when using Boehm GC]), @@ -668,20 +673,39 @@ AC_ARG_ENABLE(gc-munmap, AC_MSG_CHECKING(whether to enable unmapping of unused pages when using Boehm GC) case "$enable_gc_munmap" in yes) + # --enable-gc-munmap implies --enable-gc-mmap + enable_gc_mmap=yes AC_MSG_RESULT(yes) - # USE_MUNMAP requires USE_MMAP. - ENABLE_BOEHM_USE_MUNMAP="-DUSE_MMAP -DUSE_MUNMAP" ;; no) AC_MSG_RESULT(no) - ENABLE_BOEHM_USE_MUNMAP= ;; *) - AC_MSG_RESULT(no) - AC_MSG_WARN([unexpected argument to --enable-gc-munmap]) - ENABLE_BOEHM_USE_MUNMAP= + AC_MSG_ERROR([unexpected argument to --enable-gc-munmap]) ;; esac + +AC_MSG_CHECKING(whether to use mmap for Boehm GC) +case "$enable_gc_mmap" in + yes|no) + AC_MSG_RESULT($enable_gc_mmap) + ;; + *) + AC_MSG_ERROR([unexpected argument to --enable-gc-mmap]) + ;; +esac + +if test "$enable_gc_mmap" = yes; then + ENABLE_BOEHM_USE_MMAP="-DUSE_MMAP" +else + ENABLE_BOEHM_USE_MMAP= +fi +if test "$enable_gc_munmap" = yes; then + ENABLE_BOEHM_USE_MUNMAP="-DUSE_MUNMAP" +else + ENABLE_BOEHM_USE_MUNMAP= +fi +AC_SUBST(ENABLE_BOEHM_USE_MMAP) AC_SUBST(ENABLE_BOEHM_USE_MUNMAP) #-----------------------------------------------------------------------------# @@ -1283,7 +1307,7 @@ esac mercury_check_for_functions \ sysconf getpagesize gethostname \ - mprotect memalign posix_memalign memmove \ + mmap mprotect memalign posix_memalign sbrk memmove \ sigaction siginterrupt setitimer \ snprintf _snprintf vsnprintf _vsnprintf \ strerror strerror_r strerror_s \ @@ -3069,6 +3093,10 @@ ENABLE_BOEHM_PARALLEL_MARK= # BOEHM_MISC_CFLAGS_FOR_THREADS= +# This should be set if both the system malloc and Boehm GC (in the default +# configuration) may both call sbrk. +avoid_sbrk= + case "$host" in *solaris*) CFLAGS_FOR_THREADS="-DGC_SOLARIS_PTHREADS -D_REENTRANT" @@ -3093,6 +3121,7 @@ case "$host" in # correctly on Linux.) # XXX disabled as it aborts on GC_free when --enable-gc-munmap is used # BOEHM_MISC_CFLAGS_FOR_THREADS="-DHBLKSIZE=32768 -DCPP_LOG_HBLKSIZE=15" + avoid_sbrk=yes ;; *cygwin*) @@ -3109,6 +3138,7 @@ case "$host" in ENABLE_BOEHM_PARALLEL_MARK="-DPARALLEL_MARK" ;; esac + # avoid_sbrk? ;; i*86-pc-mingw*) @@ -3186,6 +3216,18 @@ case "$host" in CFLAGS_FOR_THREADS="" ;; esac + +if test "$avoid_sbrk" = yes; then + if test "$ac_cv_func_mmap" = yes; then + BOEHM_MISC_CFLAGS_FOR_THREADS="$BOEHM_MISC_CFLAGS_FOR_THREADS -DUSE_MMAP" + if test "$enable_gc_mmap" = no; then + AC_MSG_NOTICE([using mmap for Boehm GC in threaded grades]) + fi + else + AC_MSG_WARN([malloc and Boehm GC both use sbrk; probably unsafe!]) + fi +fi + AC_SUBST(CFLAGS_FOR_THREADS) AC_SUBST(THREAD_LIBS) AC_SUBST(LDFLAGS_FOR_THREADS) diff --git a/runtime/mercury_conf.h.in b/runtime/mercury_conf.h.in index d44ce455b..a4df655cd 100644 --- a/runtime/mercury_conf.h.in +++ b/runtime/mercury_conf.h.in @@ -237,9 +237,11 @@ ** MR_HAVE_SYSCONF we have the sysconf() system call. ** MR_HAVE_SIGACTION we have the sigaction() system call. ** MR_HAVE_GETPAGESIZE we have the getpagesize() system call. +** MR_HAVE_MMAP we have the mmap() system call. ** MR_HAVE_MPROTECT we have the mprotect() system call. ** MR_HAVE_MEMALIGN we have the memalign() function. ** MR_HAVE_POSIX_MEMALIGN we have the posix_memalign() function. +** MR_HAVE_SBRK we have the sbrk() function. ** MR_HAVE_STRERROR we have the strerror() function. ** MR_HAVE_STRERROR_R we have the strerror_r() function. ** MR_HAVE_STRERROR_S we have the strerror_s() function. @@ -312,9 +314,11 @@ #undef MR_HAVE_SYSCONF #undef MR_HAVE_SIGACTION #undef MR_HAVE_GETPAGESIZE +#undef MR_HAVE_MMAP +#undef MR_HAVE_MPROTECT #undef MR_HAVE_MEMALIGN #undef MR_HAVE_POSIX_MEMALIGN -#undef MR_HAVE_MPROTECT +#undef MR_HAVE_SBRK #undef MR_HAVE_STRERROR #undef MR_HAVE_STRERROR_R #undef MR_HAVE_STRERROR_S diff --git a/tests/hard_coded/Mmakefile b/tests/hard_coded/Mmakefile index e65f9de99..4403c7b66 100644 --- a/tests/hard_coded/Mmakefile +++ b/tests/hard_coded/Mmakefile @@ -313,6 +313,7 @@ ORDINARY_PROGS= \ test_semaphore \ test_tree_bitset \ test_yield \ + thread_sbrk \ tim_qual1 \ time_test \ trace_goal_1 \ diff --git a/tests/hard_coded/thread_sbrk.exp b/tests/hard_coded/thread_sbrk.exp new file mode 100644 index 000000000..0aff703f0 --- /dev/null +++ b/tests/hard_coded/thread_sbrk.exp @@ -0,0 +1,21 @@ +depth 1, size 1 +depth 2, size 3 +depth 3, size 7 +depth 4, size 15 +depth 5, size 31 +depth 6, size 63 +depth 7, size 127 +depth 8, size 255 +depth 9, size 511 +depth 10, size 1023 +depth 11, size 2047 +depth 12, size 4095 +depth 13, size 8191 +depth 14, size 16383 +depth 15, size 32767 +depth 16, size 65535 +depth 17, size 131071 +depth 18, size 262143 +depth 19, size 524287 +depth 20, size 1048575 +done. diff --git a/tests/hard_coded/thread_sbrk.exp2 b/tests/hard_coded/thread_sbrk.exp2 new file mode 100644 index 000000000..900f82ada --- /dev/null +++ b/tests/hard_coded/thread_sbrk.exp2 @@ -0,0 +1 @@ +spawn not supported. diff --git a/tests/hard_coded/thread_sbrk.m b/tests/hard_coded/thread_sbrk.m new file mode 100644 index 000000000..7ddf5f40c --- /dev/null +++ b/tests/hard_coded/thread_sbrk.m @@ -0,0 +1,114 @@ +%-----------------------------------------------------------------------------% + +% This program performs Mercury allocations in one thread (e.g. using Boehm GC) +% and simulates malloc calls in another thread that indirectly call sbrk. +% If both allocators use sbrk and are invoked simulataneously then +% memory corruption can result. + +:- module thread_sbrk. +:- interface. + +:- import_module io. + +:- pred main(io::di, io::uo) is cc_multi. + +%-----------------------------------------------------------------------------% +%-----------------------------------------------------------------------------% + +:- implementation. + +:- import_module bool. +:- import_module int. +:- import_module list. +:- import_module string. +:- import_module thread. +:- import_module thread.semaphore. + +:- pragma foreign_decl("C", " +#ifdef MR_HAVE_SBRK + #include +#endif +"). + +:- type tree + ---> nil + ; node(int, tree, tree). + +%-----------------------------------------------------------------------------% + +main(!IO) :- + ( can_spawn -> + semaphore.init(Sem, !IO), + thread.spawn(alloc_thread(Sem), !IO), + semaphore.wait(Sem, !IO), + sbrk_loop(Sem, !IO), + io.write_string("done.\n", !IO) + ; + io.write_string("spawn not supported.\n", !IO) + ). + +:- pred sbrk_loop(semaphore::in, io::di, io::uo) is det. + +sbrk_loop(Sem, !IO) :- + thread.yield(!IO), + semaphore.try_wait(Sem, Success, !IO), + ( + Success = yes + ; + Success = no, + % It is hard to trigger a crash by calling malloc because not every + % call ends up calling sbrk. Therefore we call sbrk directly. + sbrk(0x100, !IO), + sbrk_loop(Sem, !IO) + ). + +:- pred sbrk(int::in, io::di, io::uo) is det. + +sbrk(_, !IO). + +:- pragma foreign_proc("C", + sbrk(Increment::in, _IO0::di, _IO::uo), + [will_not_call_mercury, promise_pure, thread_safe, tabled_for_io], +" +#ifdef MR_HAVE_SBRK + sbrk(Increment); +#endif +"). + +:- pred alloc_thread(semaphore::in, io::di, io::uo) is cc_multi. + +alloc_thread(Sem, !IO) :- + semaphore.signal(Sem, !IO), + alloc_loop(Sem, 1, !IO). + +:- pred alloc_loop(semaphore::in, int::in, io::di, io::uo) is cc_multi. + +alloc_loop(Sem, Depth, !IO) :- + ( Depth > 20 -> + semaphore.signal(Sem, !IO) + ; + build(Depth, T, 0, _Id), + io.format("depth %d, size %d\n", [i(Depth), i(size(T))], !IO), + thread.yield(!IO), + alloc_loop(Sem, Depth + 1, !IO) + ). + +:- pred build(int::in, tree::out, int::in, int::out) is det. + +build(Depth, T, Id0, Id) :- + ( Depth = 1 -> + T = nil, + Id = Id0 + ; + build(Depth - 1, L, Id0 + 1, Id2), + build(Depth - 1, R, Id2, Id), + T = node(Id0, L, R) + ). + +:- func size(tree) = int. + +size(nil) = 1. +size(node(_, L, R)) = 1 + size(L) + size(R). + +%-----------------------------------------------------------------------------% +% vim: ft=mercury ts=4 sts=4 sw=4 et