A hackish patch to make Breakpad upload the log if it's impossible to create a file in the crashed …

client/mac/Framework/Breakpad.mm

 // Copyright (c) 2006, Google Inc.
 // All rights reserved.
 //
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 // notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
@@ skipping 138 matching lines @@
     return breakpad;
   }
 
   ~Breakpad();
 
   void SetKeyValue(NSString *key, NSString *value);
   NSString *KeyValue(NSString *key);
   void RemoveKeyValue(NSString *key);
 
   void GenerateAndSendReport();
+  void SetCrashLogData(void* data, size_t size) {
+    extra_crash_log_data_ = data;
+    extra_crash_log_size_ = size;
+    assert(strlen(extra_crash_log_data_) == extra_crash_log_size_);

[jeremy, 2012/05/16 08:13:07]

* Are we sure we want to limit input data to a c-string?
* Do you do any sanity checks on the input data size?  What if a corrupt/owned
renderer tries to give you a 1GB data file?

[glider, 2012/05/17 12:15:12]

Probably no. Removed this check. You're right, I'd better not use strlen.
I've put a check for the size to be less than 4 Mb. This seems to be the only
place where we need it right now.

+  }
+
+  void *GetCrashLogData() {
+    return extra_crash_log_data_;
+  }
 
   void SetFilterCallback(BreakpadFilterCallback callback, void *context) {
     filter_callback_ = callback;
     filter_callback_context_ = context;
   }
 
  private:
   Breakpad()
     : handler_(NULL),
       config_params_(NULL),
       send_and_exit_(true),
       filter_callback_(NULL),
-      filter_callback_context_(NULL) {
+      filter_callback_context_(NULL),
+      extra_crash_log_data_(NULL),
+      extra_crash_log_size_(0) {
     inspector_path_[0] = 0;
   }
 
   bool Initialize(NSDictionary *parameters);
 
   bool ExtractParameters(NSDictionary *parameters);
 
   // Dispatches to HandleException()
   static bool ExceptionHandlerDirectCallback(void *context,
                                              int exception_type,
@@ skipping 13 matching lines @@
   char                    inspector_path_[PATH_MAX];  // Path to inspector tool
 
   SimpleStringDictionary  *config_params_; // Create parameters (STRONG)
 
   OnDemandServer          inspector_;
 
   bool                    send_and_exit_;  // Exit after sending, if true
 
   BreakpadFilterCallback  filter_callback_;
   void                    *filter_callback_context_;
+
+  void                    *extra_crash_log_data_;  // Extra crash log, if present
+  size_t                  extra_crash_log_size_;
 };
 
 #pragma mark -
 #pragma mark Helper functions
 
 //=============================================================================
 // Helper functions
 
 //=============================================================================
 static BOOL IsDebuggerActive() {
@@ skipping 478 matching lines @@
       MachSendMessage keyvalue_message(kMsgType_InspectorKeyValuePair);
       keyvalue_message.SetData(&keyvalue_data, sizeof(keyvalue_data));
 
       result = sender.SendMessage(keyvalue_message, 2000);
 
       if (result != KERN_SUCCESS) {
         break;
       }
     }
 
+    if (result == KERN_SUCCESS && extra_crash_log_data_) {
+      // Pass the log pointer to the inspector process that is able to create
+      // log files.
+      MachSendMessage logptr_message(kMsgType_InspectorLogStringPtr);
+      size_t crash_len = strlen((char*)extra_crash_log_data_);
+      size_t min_len = (crash_len < extra_crash_log_size_) ?
+                        crash_len : extra_crash_log_size_;

[jeremy, 2012/05/16 08:13:07]

Why is this min(strlen(data), data_len) rather than just data_len ?

[glider, 2012/05/17 12:15:12]

Done.

+      logptr_message.AddOOLDescriptor(
+          extra_crash_log_data_, min_len,
+          /* deallocate */ 0, /* copy options */ 0);
+      logptr_message.SetData(&extra_crash_log_data_,
+                             sizeof(extra_crash_log_data_));  // sizeof(void*)
+      result = sender.SendMessage(logptr_message, 2000);
+    }
+
     if (result == KERN_SUCCESS) {
       // Wait for acknowledgement that the inspection has finished.
       MachReceiveMessage acknowledge_messsage;
       result = acknowledge_port.WaitForMessage(&acknowledge_messsage, 5000);
     }
   }
 
 #if VERBOSE
   PRINT_MACH_RESULT(result, "Breakpad: SendMessage ");
   printf("Breakpad: Inspector service port = %#x\n",
@@ skipping 273 matching lines @@
     }
     logFileCounter++;
     logFileKey = [NSString stringWithFormat:@"%@%d",
                            @BREAKPAD_LOGFILE_KEY_PREFIX,
                            logFileCounter];
     existingLogFilename = BreakpadKeyValue(ref, logFileKey);
   }
 
   BreakpadSetKeyValue(ref, logFileKey, logPathname);
 }
+
+//=============================================================================
+void BreakpadCreateAndAddLogFile(BreakpadRef ref,
+                                 void *logContents,
+                                 size_t size) {

[jeremy, 2012/05/16 08:13:07]

Size should also undergo a sanity check.

[glider, 2012/05/17 12:15:12]

This is done in SetCrashLogData

+  if (logContents) {
+    ProtectedMemoryLocker locker(&gDictionaryMutex, gBreakpadAllocator);
+    Breakpad *breakpad = (Breakpad *)ref;
+    breakpad->SetCrashLogData(logContents, size);
+    NSString *key = @BREAKPAD_HAS_EXTRA_LOG;
+    NSString *value = @"YES";
+    BreakpadSetKeyValue(ref, key, value);
+  }
+}
+