simplify tcmalloc/sbrk/sbrk-hooks integration
Instead of relying on __sbrk (on subset of Linux systems) or invoking
sbrk syscall directly (on subset of FreeBSD systems), we have malloc
invoke special tcmalloc_hooked_sbrk function. Which handles hooking
and then invokes regular system's sbrk. Yes, we loose theoretical
ability to hook into non-tcmalloc uses of sbrk, but we gain portable
simplicity.
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 51664e7..7e969ee 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -187,7 +187,6 @@
check_type_size("struct mallinfo" STRUCT_MALLINFO LANGUAGE CXX)
check_type_size("struct mallinfo2" STRUCT_MALLINFO2 LANGUAGE CXX)
check_function_exists("sbrk" HAVE_SBRK) # for tcmalloc to get memory
-check_function_exists("__sbrk" HAVE___SBRK) # for tcmalloc to get memory
check_function_exists("geteuid" HAVE_GETEUID) # for turning off services when run as root
check_include_file("features.h" HAVE_FEATURES_H) # for vdso_support.h, Where __GLIBC__ is defined
check_include_file("malloc.h" HAVE_MALLOC_H) # some systems define stuff there, others not
diff --git a/cmake/config.h.in b/cmake/config.h.in
index 0c9145f..db9d3f8 100644
--- a/cmake/config.h.in
+++ b/cmake/config.h.in
@@ -143,9 +143,6 @@
/* Define to 1 if compiler supports __environ */
#cmakedefine HAVE___ENVIRON
-/* Define to 1 if you have the `__sbrk' function. */
-#cmakedefine01 HAVE___SBRK
-
/* Always the empty-string on non-windows systems. On windows, should be
"__declspec(dllexport)". This way, when we compile the dll, we export our
functions/classes. It's safe to define this here because config.h is only
diff --git a/configure.ac b/configure.ac
index 85b399e..f5a0ebd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -254,7 +254,6 @@
AC_CHECK_TYPES([struct mallinfo],,, [#include <malloc.h>])
AC_CHECK_TYPES([struct mallinfo2],,, [#include <malloc.h>])
AC_CHECK_FUNCS(sbrk) # for tcmalloc to get memory
-AC_CHECK_FUNCS(__sbrk) # for tcmalloc to get memory
AC_CHECK_FUNCS(geteuid) # for turning off services when run as root
AC_CHECK_HEADERS(features.h) # for vdso_support.h, __GLIBC__ macros
AC_CHECK_HEADERS(malloc.h) # some systems define stuff there, others not
diff --git a/src/mmap_hook.cc b/src/mmap_hook.cc
index e375ad2..9c62209 100644
--- a/src/mmap_hook.cc
+++ b/src/mmap_hook.cc
@@ -161,23 +161,6 @@
}
}
- static MappingEvent FillSbrk(void* result, intptr_t increment) {
- MappingEvent evt;
- evt.is_sbrk = 1;
- if (increment > 0) {
- evt.after_address = result;
- evt.after_length = increment;
- evt.after_valid = 1;
- } else {
- intptr_t res_addr = reinterpret_cast<uintptr_t>(result);
- intptr_t new_brk = res_addr + increment;
- evt.before_address = reinterpret_cast<void*>(new_brk);
- evt.before_length = -increment;
- evt.before_valid = 1;
- }
- return evt;
- }
-
private:
std::atomic<MappingHookDescriptor*> list_head_{};
std::atomic<bool> ran_initial_hooks_{};
@@ -481,62 +464,30 @@
return result;
}
+#endif // __linux__
-#endif
-
-#if defined(__linux__) && HAVE___SBRK
-// glibc's version:
-extern "C" void* __sbrk(intptr_t increment);
-
-#define do_sbrk(i) __sbrk(i)
-
-#define HOOKED_SBRK
-#endif // linux and __sbrk
-
-#if defined(__FreeBSD__) && defined(_LP64) && defined(HAVE_SBRK)
-static void* do_sbrk(intptr_t increment) {
- uintptr_t curbrk = __syscall(SYS_break, nullptr);
- uintptr_t badbrk = static_cast<uintptr_t>(static_cast<intptr_t>(-1));
- if (curbrk == badbrk) {
- nomem:
- errno = ENOMEM;
- return reinterpret_cast<void*>(badbrk);
- }
-
- if (increment == 0) {
- return reinterpret_cast<void*>(curbrk);
- }
-
- if (increment > 0) {
- if (curbrk + static_cast<uintptr_t>(increment) < curbrk) {
- goto nomem;
- }
- } else {
- if (curbrk + static_cast<uintptr_t>(increment) > curbrk) {
- goto nomem;
- }
- }
-
- if (brk(reinterpret_cast<void*>(curbrk + increment)) < 0) {
- goto nomem;
- }
-
- return reinterpret_cast<void*>(curbrk);
-}
-
-#define HOOKED_SBRK
-#endif // FreeBSD
-
-#ifdef HOOKED_SBRK
-extern "C" PERFTOOLS_DLL_DECL ATTRIBUTE_NOINLINE void* sbrk(intptr_t increment) __THROW;
-
-void* sbrk(intptr_t increment) __THROW {
- void *result = do_sbrk(increment);
- if (increment == 0 || result == reinterpret_cast<void*>(static_cast<intptr_t>(-1))) {
+#if defined(HAVE_SBRK)
+extern "C" ATTRIBUTE_VISIBILITY_HIDDEN ATTRIBUTE_NOINLINE
+void* tcmalloc_hooked_sbrk(intptr_t increment) {
+ void *result = sbrk(increment);
+ if (increment == 0 || result == reinterpret_cast<void*>(intptr_t{-1})) {
return result;
}
- tcmalloc::MappingEvent evt = tcmalloc::MappingHooks::FillSbrk(result, increment);
+ tcmalloc::MappingEvent evt;
+ evt.is_sbrk = 1;
+ if (increment > 0) {
+ evt.after_address = result;
+ evt.after_length = increment;
+ evt.after_valid = 1;
+ } else {
+ intptr_t res_addr = reinterpret_cast<uintptr_t>(result);
+ intptr_t new_brk = res_addr + increment;
+ evt.before_address = reinterpret_cast<void*>(new_brk);
+ evt.before_length = -increment;
+ evt.before_valid = 1;
+ }
+
BacktraceHelper helper;
int want_stack = helper.PreInvoke(&evt);
if (want_stack) {
@@ -546,7 +497,7 @@
return result;
}
-#endif // HOOKED_SBRK
+#endif
namespace tcmalloc {
#ifdef HOOKED_MMAP
@@ -554,10 +505,4 @@
#else
const bool mmap_hook_works = false;
#endif
-
-#ifdef HOOKED_SBRK
-const bool sbrk_hook_works = true;
-#else
-const bool sbrk_hook_works = false;
-#endif
} // namespace tcmalloc
diff --git a/src/system-alloc.cc b/src/system-alloc.cc
index e47691e..14c8ae2 100644
--- a/src/system-alloc.cc
+++ b/src/system-alloc.cc
@@ -168,6 +168,17 @@
static const char sbrk_name[] = "SbrkSysAllocator";
static const char mmap_name[] = "MmapSysAllocator";
+#ifdef HAVE_SBRK
+extern "C" {
+ // When we're building "full" tcmalloc with mmap_hook.cc linked-in,
+ // this definition gets overriden by definition in mmap_hook.cc
+ // which handles hooks which is required by heap checker.
+ ATTRIBUTE_VISIBILITY_HIDDEN ATTRIBUTE_WEAK
+ void* tcmalloc_hooked_sbrk(intptr_t increment) {
+ return sbrk(increment);
+ }
+}
+#endif
void* SbrkSysAllocator::Alloc(size_t size, size_t *actual_size,
size_t alignment) {
@@ -205,11 +216,11 @@
// http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libc/port/sys/sbrk.c?a=true
// http://sourceware.org/cgi-bin/cvsweb.cgi/~checkout~/libc/misc/sbrk.c?rev=1.1.2.1&content-type=text/plain&cvsroot=glibc
// Without this check, sbrk may succeed when it ought to fail.)
- if (reinterpret_cast<intptr_t>(sbrk(0)) + size < size) {
+ if (reinterpret_cast<intptr_t>(tcmalloc_hooked_sbrk(0)) + size < size) {
return NULL;
}
- void* result = sbrk(size);
+ void* result = tcmalloc_hooked_sbrk(size);
if (result == reinterpret_cast<void*>(-1)) {
return NULL;
}
@@ -220,7 +231,7 @@
// Try to get more memory for alignment
size_t extra = alignment - (ptr & (alignment-1));
- void* r2 = sbrk(extra);
+ void* r2 = tcmalloc_hooked_sbrk(extra);
if (reinterpret_cast<uintptr_t>(r2) == (ptr + size)) {
// Contiguous with previous result
return reinterpret_cast<void*>(ptr + extra);
@@ -228,7 +239,7 @@
// Give up and ask for "size + alignment - 1" bytes so
// that we can find an aligned region within it.
- result = sbrk(size + alignment - 1);
+ result = tcmalloc_hooked_sbrk(size + alignment - 1);
if (result == reinterpret_cast<void*>(-1)) {
return NULL;
}
diff --git a/src/tests/mmap_hook_test.cc b/src/tests/mmap_hook_test.cc
index a6e205f..206bc49 100644
--- a/src/tests/mmap_hook_test.cc
+++ b/src/tests/mmap_hook_test.cc
@@ -255,13 +255,20 @@
#ifdef HAVE_SBRK
+extern "C" void* tcmalloc_hooked_sbrk(intptr_t increment);
+
+static bool sbrk_works = ([] () {
+ void* result = tcmalloc_hooked_sbrk(8);
+ return result != reinterpret_cast<void*>(intptr_t{-1});
+}());
+
TEST_F(MMapHookTest, Sbrk) {
- if (!tcmalloc::sbrk_hook_works) {
+ if (!sbrk_works) {
puts("sbrk test SKIPPED");
return;
}
- void* addr = sbrk(8);
+ void* addr = tcmalloc_hooked_sbrk(8);
ASSERT_TRUE(got_first_allocation);
@@ -273,7 +280,7 @@
ASSERT_FALSE(HasFatalFailure());
have_last_evt_ = false;
- void* addr2 = sbrk(16);
+ void* addr2 = tcmalloc_hooked_sbrk(16);
EXPECT_TRUE(last_evt_.is_sbrk);
EXPECT_TRUE(!last_evt_.before_valid && !last_evt_.file_valid && last_evt_.after_valid);
@@ -283,7 +290,7 @@
ASSERT_FALSE(HasFatalFailure());
have_last_evt_ = false;
- char* addr3 = static_cast<char*>(sbrk(-13));
+ char* addr3 = static_cast<char*>(tcmalloc_hooked_sbrk(-13));
EXPECT_TRUE(last_evt_.is_sbrk);
EXPECT_TRUE(last_evt_.before_valid && !last_evt_.file_valid && !last_evt_.after_valid);
@@ -295,7 +302,7 @@
}
TEST_F(MMapHookTest, SbrkBacktrace) {
- if (!tcmalloc::sbrk_hook_works) {
+ if (!sbrk_works) {
puts("sbrk backtrace test SKIPPED");
return;
}
@@ -325,7 +332,7 @@
expected_address, reinterpret_cast<void*>(&Helper::trampoline));
// Why cast? Because some OS-es define sbrk as accepting long.
- Helper::trampoline(&addr, reinterpret_cast<void*(*)(intptr_t)>(sbrk));
+ Helper::trampoline(&addr, reinterpret_cast<void*(*)(intptr_t)>(tcmalloc_hooked_sbrk));
ASSERT_NE(nullptr, addr);
ASSERT_EQ(backtrace_address_, expected_address);
}
diff --git a/src/windows/port.cc b/src/windows/port.cc
index a31e866..79043e2 100644
--- a/src/windows/port.cc
+++ b/src/windows/port.cc
@@ -64,11 +64,6 @@
return pagesize;
}
-extern "C" PERFTOOLS_DLL_DECL void* __sbrk(ptrdiff_t increment) {
- LOG(FATAL, "Windows doesn't implement sbrk!\n");
- return NULL;
-}
-
// We need to write to 'stderr' without having windows allocate memory.
// The safest way is via a low-level call like WriteConsoleA(). But
// even then we need to be sure to print in small bursts so as to not
diff --git a/vsprojects/include/config.h b/vsprojects/include/config.h
index db21e16..3e01043 100644
--- a/vsprojects/include/config.h
+++ b/vsprojects/include/config.h
@@ -149,9 +149,6 @@
/* Define to 1 if compiler supports __environ */
/* #undef HAVE___ENVIRON */
-/* Define to 1 if you have the `__sbrk' function. */
-/* #undef HAVE___SBRK */
-
/* Always the empty-string on non-windows systems. On windows, should be
"__declspec(dllexport)". This way, when we compile the dll, we export our
functions/classes. It's safe to define this here because config.h is only
