[Mac] Implement base::EnableTerminationOnHeapCorruption() by overriding malloc_error_break().

base/process_util_mac.mm

Index: base/process_util_mac.mm
diff --git a/base/process_util_mac.mm b/base/process_util_mac.mm
index 7db44d900530d7d79150ca819194393bdb961b76..9bd394fe34a5a179e2440ad0acc94bbe9fda809f 100644
--- a/base/process_util_mac.mm
+++ b/base/process_util_mac.mm
@@ -12,6 +12,8 @@
 #include <mach/mach_vm.h>
 #include <mach/shared_region.h>
 #include <mach/task.h>
+#include <mach-o/dyld.h>
+#include <mach-o/nlist.h>
 #include <malloc/malloc.h>
 #import <objc/runtime.h>
 #include <spawn.h>
@@ -34,6 +36,7 @@
 #include "base/time.h"
 #include "third_party/apple_apsl/CFBase.h"
 #include "third_party/apple_apsl/malloc.h"
+#include "third_party/mach_override/mach_override.h"
 
 namespace base {
 
@@ -483,6 +486,85 @@ size_t GetSystemCommitCharge() {
   return (data.active_count * page_size) / 1024;
 }
 
+namespace {
+
+typedef void(*malloc_error_break_t)(void);

[Mark Mentovai, 2011/08/17 16:44:55]

Move the typedef and declaration down so that they’re above CrMallocErrorBreak,
the first place they’re needed.

+malloc_error_break_t g_original_malloc_error_break = NULL;
+
+// Finds the library path for malloc() and thus libSystem, or at least a part
+// of it. libSystem was split into parts on Lion.
+const char* LookupLibSystemPath() {

[Mark Mentovai, 2011/08/17 16:44:55]

Since the “part of libSystem” is what Apple calls Libc (or
system/libsystem_c.dylib), this should probably refer to Libc or LibC,
LibSystemC, or LibSystemLibc or LibSystemLibC. (Libc has always been distinct in
source, but is only distinct in runtime binary as of Lion.)

“Look up” as a verb is two words, so InterCapped, this should be LookUpLibcPath
(or LookUpLibCPath). Same on line 510.

+  const void* addr = reinterpret_cast<void*>(&malloc);
+
+  Dl_info info;
+  if (dladdr(addr, &info))
+    return info.dli_fname;
+
+  LOG(WARNING) << "Could not find image path for malloc()";
+  return NULL;
+}
+
+// Returns the function pointer for malloc_error_break. This symbol is declared
+// as __private_extern__ and cannot be dlsym()ed. Instead, use nlist() to
+// get it.

[Mark Mentovai, 2011/08/17 16:44:55]

I think I’ll have to (or someone else will have to) write an nlist equivalent
that handles the 64-bit world. When I (or someone else) does this, it should be
possible to do without the stupid interface that opens a new copy of the
library. We should be able to do a better job and write something with a better
interface.

I actually kind of want to do this to give myself an nlist equivalent I can use
to test for the presence of symbols with N_UNDF for the
closure_blocks_leopard_compat unit test.

+malloc_error_break_t LookupMallocErrorBreak() {

[Mark Mentovai, 2011/08/17 16:44:55]

LookUp (as above).

+#if ARCH_CPU_32_BITS
+  const char* lib_system_path = LookupLibSystemPath();

[Mark Mentovai, 2011/08/17 16:44:55]

Rename the variable if you rename the function.

+  if (!lib_system_path)
+    return NULL;
+
+  // Only need to lookup two symbols, but nlist() requires a NULL-terminated

[Mark Mentovai, 2011/08/17 16:44:55]

Look up should be two words here too.

+  // array and takes no count. So do that.

[Mark Mentovai, 2011/08/17 16:44:55]

Nix “So do that.”

+  struct nlist nl[3];
+  bzero(&nl, sizeof(nl));
+
+  // The symbol to find.
+  nl[0].n_un.n_name = const_cast<char*>("_malloc_error_break");

[Mark Mentovai, 2011/08/17 16:44:55]

Wow, what a terrible interface.

+
+  // A reference symbol by which the address of the desired symbol will be
+  // calculated.
+  nl[1].n_un.n_name = const_cast<char*>("_malloc");
+
+  int rv = nlist(lib_system_path, nl);
+  if (rv < 0 || nl[0].n_type == N_UNDF || nl[1].n_type == N_UNDF) {

[Mark Mentovai, 2011/08/17 16:44:55]

Shouldn’t you explicitly test |rv == 0| here?

+    return NULL;
+  }
+
+  // nlist() returns addresses as offsets in the image, not the instruction
+  // pointer in memory. Use the known in-memory address of malloc()
+  // to compute the offset for malloc_error_break().
+  uintptr_t reference_addr = reinterpret_cast<uintptr_t>(&malloc);
+  reference_addr -= nl[1].n_value;
+  reference_addr += nl[0].n_value;
+
+  return reinterpret_cast<malloc_error_break_t>(reference_addr);
+#endif  // ARCH_CPU_32_BITS
+
+  return NULL;
+}
+
+void CrMallocErrorBreak() {
+  g_original_malloc_error_break();
+  base::debug::BreakDebugger();
+}
+
+}  // namespace
+
+void EnableTerminationOnHeapCorruption() {
+  malloc_error_break_t error_break = LookupMallocErrorBreak();

[Mark Mentovai, 2011/08/17 16:44:55]

I’d call this malloc_error_break.

+  if (!error_break) {
+    LOG(WARNING) << "Could not find malloc_error_break";
+    return;
+  }
+
+  mach_error_t err = mach_override_ptr(
+     (void*)error_break,
+     (void*)&CrMallocErrorBreak,
+     (void**)&g_original_malloc_error_break);
+
+  if (err != err_none)
+    LOG(WARNING) << "Could not override malloc_error_break; error = " << err;

[Mark Mentovai, 2011/08/17 16:44:55]

bpoop has a custom set of logging streams to handle mach_error_t. :)

(No change required, just sayin’.)

+}
 // ------------------------------------------------------------------------
 
 namespace {