[Mac] Remove potential for malloc in CrMallocErrorBreak().

base/process_util_mac.mm

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/process_util.h"
 
 #import <Cocoa/Cocoa.h>
 #include <crt_externs.h>
 #include <dlfcn.h>
 #include <errno.h>
@@ skipping 17 matching lines @@
 #include <string>
 
 #include "base/debug/debugger.h"
 #include "base/eintr_wrapper.h"
 #include "base/file_util.h"
 #include "base/hash_tables.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/mac/mac_util.h"
 #include "base/mac/scoped_mach_port.h"
+#include "base/safe_strerror_posix.h"
 #include "base/string_util.h"
 #include "base/sys_info.h"
 #include "base/threading/thread_local.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 {
 
 void RestoreDefaultExceptionHandler() {
@@ skipping 535 matching lines @@
  private:
   ThreadLocalBoolean* scoped_tlb_;
   bool original_value_;
 
   DISALLOW_COPY_AND_ASSIGN(ThreadLocalBooleanAutoReset);
 };
 
 base::LazyInstance<ThreadLocalBoolean>::Leaky
     g_unchecked_malloc = LAZY_INSTANCE_INITIALIZER;
 
+// NOTE(shess): This is called when the malloc library noticed that the heap
+// is fubar.  Avoid calls which will re-enter the malloc library.
 void CrMallocErrorBreak() {
   g_original_malloc_error_break();
 
   // Out of memory is certainly not heap corruption, and not necessarily
   // something for which the process should be terminated. Leave that decision
   // to the OOM killer.  The EBADF case comes up because the malloc library
   // attempts to log to ASL (syslog) before calling this code, which fails
   // accessing a Unix-domain socket because of sandboxing.
   if (errno == ENOMEM || (errno == EBADF && g_unchecked_malloc.Get().Get()))
     return;
 
   // A unit test checks this error message, so it needs to be in release builds.
-  PLOG(ERROR) <<
-      "Terminating process due to a potential for future heap corruption";
+  char buf[1024] =
+      "Terminating process due to a potential for future heap corruption: ";
+  size_t bufLen = strlen(buf);

[Mark Mentovai, 2012/11/12 22:45:10]

Nit: buf_len.

+  safe_strerror_r(errno, buf + bufLen, sizeof(buf) - bufLen);

[Mark Mentovai, 2012/11/12 22:45:10]

[safe_]strerror[_r] might wind up entering the allocator to access localized
messages. You can use the deprecated sys_errlist variable (along with sys_nerr),
but I’d just as soon drop the friendly string in this case and just use the
numeric value of errno.

For that matter, it’s a real pain to prove that the printf family can’t enter
the allocator for a given use either. snprintf with only integral and string
types SHOULD be safe.

[Scott Hess - ex-Googler, 2012/11/12 23:06:43]

Seems reasonable.
I actually recently had opportunity to audit that code, and AFAICT it will only
dynamically allocate in certain cases of floating-point output, and vector
output.  But, even so ... there's always MATH.

+  RAW_LOG(ERROR, buf);
 
   // Crash by writing to NULL+errno to allow analyzing errno from
   // crash dump info (setting a breakpad key would re-enter the malloc
   // library).  Max documented errno in intro(2) is actually 102, but
   // it really just needs to be "small" to stay on the right vm page.
   const int kMaxErrno = 256;
   char* volatile death_ptr = NULL;
   death_ptr += std::min(errno, kMaxErrno);
   *death_ptr = '!';
 }
 
 }  // namespace
 
 void EnableTerminationOnHeapCorruption() {
 #ifdef ADDRESS_SANITIZER
   // Don't do anything special on heap corruption, because it should be handled
   // by AddressSanitizer.
   return;
 #endif
   malloc_error_break_t malloc_error_break = LookUpMallocErrorBreak();
   if (!malloc_error_break) {
     DLOG(WARNING) << "Could not find malloc_error_break";
     return;
   }
 
+  // Warm this up so that it doesn't require allocation when
+  // |CrMallocErrorBreak()| calls it.
+  ignore_result(g_unchecked_malloc.Get().Get());
+
   mach_error_t err = mach_override_ptr(
      (void*)malloc_error_break,
      (void*)&CrMallocErrorBreak,
      (void**)&g_original_malloc_error_break);
 
   if (err != err_none)
     DLOG(WARNING) << "Could not override malloc_error_break; error = " << err;
 }
 
 // ------------------------------------------------------------------------
@@ skipping 596 matching lines @@
   }
 }
 
 }  // namespace
 
 void EnsureProcessTerminated(ProcessHandle process) {
   WaitForChildToDie(process, kWaitBeforeKillSeconds);
 }
 
 }  // namespace base