commit 6e709a11031c80c96f47da50b3231074334f462c
author Ben Chan <benchan@chromium.org> Wed Feb 29 12:10:44 2012 -0800
committer Gerrit <chrome-bot@google.com> Mon Mar 05 17:14:29 2012 -0800
tree 2d895848309968660723d1954a5ac4948df2f67e
parent 2076b9011d0f217d2c308d28ba13af84cf2e4b17

Improve error diagnostics in UserCollector.

This CL makes the following changes to improve error diagnostics in
UserCollector:
1. Add an error type to describe why the crash reporter fails to
   collects a user crash, which is reported to the crash server along
   with the "crash_reporter-user-collection" error signature.
2. Perform some preliminary validations of the core file before
   converting it to a minidump file using core2md.
3. Identify the case when a 32-bit core file is produced on a 64-bit
   platform as core2md currently cannot handle 32-bit core files on
   a 64-bit platform.

BUG=chromium-os:7871
TEST=Tested the following:
1. emerge-{x86,amd64,arm}-generic crash-reporter
2. FEATURES="test" emerge-{x86,amd64,arm}-generic crash-reporter
3. Run the following autotest tests on x86-mario and stumpy64:
   - logging_CrashSender
   - logging_UserCrash

Change-Id: Ib50b4cc81a91f7cd75f9440005200d4027ce1f6f
Reviewed-on: https://gerrit.chromium.org/gerrit/17166
Reviewed-by: Michael Krebs <mkrebs@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
Commit-Ready: Ben Chan <benchan@chromium.org>

crash_reporter/user_collector.h

diff --git a/crash_reporter/user_collector.h b/crash_reporter/user_collector.h
index 043d38c..01ca2af 100644
--- a/crash_reporter/user_collector.h
+++ b/crash_reporter/user_collector.h
@@ -69,6 +69,7 @@
   FRIEND_TEST(UserCollectorTest, ShouldDumpDeveloperImageOverridesConsent);
   FRIEND_TEST(UserCollectorTest, ShouldDumpUseConsentProductionImage);
   FRIEND_TEST(UserCollectorTest, ValidateProcFiles);
+  FRIEND_TEST(UserCollectorTest, ValidateCoreFile);
 
   // Enumeration to pass to GetIdFromStatus.  Must match the order
   // that the kernel lists IDs in the status file.
@@ -80,8 +81,23 @@
     kIdMax
   };
 
+  enum ErrorType {
+    kErrorNone,
+    kErrorSystemIssue,
+    kErrorReadCoreData,
+    kErrorUnusableProcFiles,
+    kErrorInvalidCoreFile,
+    kErrorUnsupported32BitCoreFile,
+    kErrorCore2MinidumpConversion,
+  };
+
   static const int kForkProblem = 255;
 
+  // Returns an error type signature for a given |error_type| value,
+  // which is reported to the crash server along with the
+  // crash_reporter-user-collection signature.
+  std::string GetErrorTypeSignature(ErrorType error_type) const;
+
   std::string GetPattern(bool enabled) const;
   bool SetUpInternal(bool enabled);
 
@@ -110,7 +126,8 @@
                           std::string *state);
 
   void LogCollectionError(const std::string &error_message);
-  void EnqueueCollectionErrorLog(pid_t pid, const std::string &exec_name);
+  void EnqueueCollectionErrorLog(pid_t pid, ErrorType error_type,
+                                 const std::string &exec_name);
 
   bool CopyOffProcFiles(pid_t pid, const FilePath &container_dir);
 
@@ -119,7 +136,14 @@
   // a process is reaped by the kernel before the copying of its proc files
   // takes place, some proc files like /proc/<pid>/maps may contain nothing
   // and thus become unusable.
-  bool ValidateProcFiles(const FilePath &container_dir);
+  bool ValidateProcFiles(const FilePath &container_dir) const;
+
+  // Validates the core file at |core_path| and returns kErrorNone if
+  // the file contains the ELF magic bytes and an ELF class that matches the
+  // platform (i.e. 32-bit ELF on a 32-bit platform or 64-bit ELF on a 64-bit
+  // platform), which is due to the limitation in core2md. It returns an error
+  // type otherwise.
+  ErrorType ValidateCoreFile(const FilePath &core_path) const;
 
   // Determines the crash directory for given pid based on pid's owner,
   // and creates the directory if necessary with appropriate permissions.
@@ -133,12 +157,12 @@
                          const FilePath &procfs_directory,
                          const FilePath &minidump_path,
                          const FilePath &temp_directory);
-  bool ConvertCoreToMinidump(pid_t pid,
-                             const FilePath &container_dir,
-                             const FilePath &core_path,
-                             const FilePath &minidump_path);
-  bool ConvertAndEnqueueCrash(int pid, const std::string &exec_name,
-                              bool *out_of_capacity);
+  ErrorType ConvertCoreToMinidump(pid_t pid,
+                                  const FilePath &container_dir,
+                                  const FilePath &core_path,
+                                  const FilePath &minidump_path);
+  ErrorType ConvertAndEnqueueCrash(int pid, const std::string &exec_name,
+                                   bool *out_of_capacity);
   bool ParseCrashAttributes(const std::string &crash_attributes,
                             pid_t *pid, int *signal,
                             std::string *kernel_supplied_name);