[Mac] Log CHECK() file, line and message into breakpad.

chrome/app/breakpad_mac.mm

Index: chrome/app/breakpad_mac.mm
diff --git a/chrome/app/breakpad_mac.mm b/chrome/app/breakpad_mac.mm
index f078271683fa448de75f40818ed7a9491d7ab3ac..f73d06add082528210dbb90741aa93e96768ff5f 100644
--- a/chrome/app/breakpad_mac.mm
+++ b/chrome/app/breakpad_mac.mm
@@ -50,6 +50,36 @@ void ClearCrashKeyValue(NSString* key) {
   BreakpadRemoveUploadParameter(gBreakpadRef, key);
 }
 
+bool FatalMessageHandler(int severity, const char* file, int line,
+                         size_t message_start, const std::string& str) {
+  // Do not handle non-FATAL.
+  if (severity != logging::LOG_FATAL)
+    return false;
+
+  // Only log last path component.  This matches logging.cc.
+  // NOTE(shess): Could look for the shared prefix between |file| and

[Mark Mentovai, 2011/10/26 23:50:27]

If this were to become a fully-cooked idea, it would result in different
treatment of files in, say, chrome and chrome/app compared to elsewhere.

Xcode always builds things so that __FILE__ contains a full path, but make
probably doesn’t, so relying on this to do anything special is probably a bad
idea. Relying on anything other than being able to grab the last component is
fragile.

[Scott Hess - ex-Googler, 2011/10/27 00:15:09]

removed.

+  // |__FILE__|, and store what's left.
+  if (file) {
+    const char* slash = rindex(file, '/');

[Mark Mentovai, 2011/10/26 23:50:27]

strrchr is the modern name for rindex. Use that.

[Scott Hess - ex-Googler, 2011/10/27 00:15:09]

GET OFF MY LAWN!

+    if (slash)
+      file = slash + 1;

[Mark Mentovai, 2011/10/26 23:50:27]

If file is an empty string, this is dangerous. Be defensive about that.

[dmac, 2011/10/27 00:07:11]

if file is an empty string, won't slash be NULL? and if file ends with "/" file
will end up as "". Maybe I'm missing something?

[Scott Hess - ex-Googler, 2011/10/27 00:15:09]

If file were empty, slash would be NULL.

+  }
+
+  // What happens if we ran out of memory and the construction does a

[Mark Mentovai, 2011/10/26 23:50:27]

You know how I feel about “we.”

[Scott Hess - ex-Googler, 2011/10/27 00:15:09]

Bother.

+  // malloc?  Infinite loop!  By putting a CHECK() in this code, I
+  // verified that it still crashed after running out of stack...
+  // Solving would maybe need a pre-allocated mutable string?

[Mark Mentovai, 2011/10/26 23:50:27]

You can have a static bool that gets set when you enter this function and
cleared when you leave it. You can gate entry on its being clear.

That’s not thread-safe, though. Thread-safe would be to use a TLS slot instead
of a static.

The mutable-string idea’s interesting too but it still doesn’t protect against
bad things that might happen inside SetCrashKeyValue.

[Scott Hess - ex-Googler, 2011/10/27 00:15:09]

I think that's reasonable.  I'll put that up as a change, let me know if you'd
rather go with an explicit lock.
Could using the TLS slot hit OOM?
Honestly, I assume that once I've hit objc_msgSend(), I can kiss any guarantees
goodbye.

The static can be placed after the FATAL check, in which case it only is a
problem if multiple threads are hitting fatal conditions at once.  Worst case,
neither of them sets the value, but the program should still crash.  Or am I not
being paranoid enough?  Is worst case my house burning down?  OMG, my stuff!

+  NSString* fatal_key = @"LOG_FATAL";
+  NSString* fatal_value =
+      [NSString stringWithFormat:@"%s:%d: %s",
+                                 file, line, str.c_str() + message_start];
+  SetCrashKeyValue(fatal_key, fatal_value);
+
+  // Rather than including the code to force the crash here, allow the
+  // caller to do it.
+  return false;
+}
+
 }  // namespace
 
 bool IsCrashReporterEnabled() {
@@ -176,7 +206,9 @@ void InitCrashReporter() {
     std::string guid =
         command_line->GetSwitchValueASCII(switches::kEnableCrashReporter);
     child_process_logging::SetClientId(guid);
-   }
+  }
+
+  logging::SetLogMessageHandler(&FatalMessageHandler);
 }
 
 void InitCrashProcessInfo() {