crash-reporter: Dump crashes outside of cryptohome during autotests.
If the file "/mnt/stateful_partition/etc/collect_chrome_crashes" exists on
the target, dump crashes to /var/spool/crash/ instead of
/home/chronos/user/crash/. This is so they are easily available even after
logout.
BUG=chromium-os:18637
TEST=Ran unit and remote tests.
Change-Id: I340bbb0c1772123192c8bb87872bcda53fae5524
Reviewed-on: http://gerrit.chromium.org/gerrit/5419
Reviewed-by: Ken Mixter <kmixter@chromium.org>
Tested-by: Michael Krebs <mkrebs@chromium.org>
diff --git a/crash_reporter/crash_collector.cc b/crash_reporter/crash_collector.cc
index e2cd281..b60f7ba 100644
--- a/crash_reporter/crash_collector.cc
+++ b/crash_reporter/crash_collector.cc
@@ -20,12 +20,15 @@
#include "base/string_util.h"
#include "chromeos/process.h"
+static const char kCollectChromeFile[] =
+ "/mnt/stateful_partition/etc/collect_chrome_crashes";
+static const char kCrashTestInProgressPath[] = "/tmp/crash-test-in-progress";
static const char kDefaultUserName[] = "chronos";
+static const char kLeaveCoreFile[] = "/root/.leave_core";
static const char kLsbRelease[] = "/etc/lsb-release";
static const char kShellPath[] = "/bin/sh";
static const char kSystemCrashPath[] = "/var/spool/crash";
static const char kUserCrashPath[] = "/home/chronos/user/crash";
-static const char kCrashTestInProgressPath[] = "/tmp/crash-test-in-progress";
// Directory mode of the user crash spool directory.
static const mode_t kUserCrashPathMode = 0755;
@@ -119,7 +122,13 @@
mode_t *mode,
uid_t *directory_owner,
gid_t *directory_group) {
- if (process_euid == default_user_id) {
+ // TODO(mkrebs): This can go away once Chrome crashes are handled
+ // normally (see crosbug.com/5872).
+ // Check if the user crash directory should be used. If we are
+ // collecting chrome crashes during autotesting, we want to put them in
+ // the system crash directory so they are outside the cryptohome -- in
+ // case we are being run during logout (see crosbug.com/18637).
+ if (process_euid == default_user_id && IsUserSpecificDirectoryEnabled()) {
*mode = kUserCrashPathMode;
*directory_owner = default_user_id;
*directory_group = default_user_group;
@@ -355,3 +364,30 @@
bool CrashCollector::IsCrashTestInProgress() {
return file_util::PathExists(FilePath(kCrashTestInProgressPath));
}
+
+bool CrashCollector::IsDeveloperImage() {
+ // If we're testing crash reporter itself, we don't want to special-case
+ // for developer images.
+ if (IsCrashTestInProgress())
+ return false;
+ return file_util::PathExists(FilePath(kLeaveCoreFile));
+}
+
+bool CrashCollector::ShouldHandleChromeCrashes() {
+ // If we're testing crash reporter itself, we don't want to allow an
+ // override for chrome crashes. And, let's be conservative and only
+ // allow an override for developer images.
+ if (!IsCrashTestInProgress() && IsDeveloperImage()) {
+ // Check if there's an override to indicate we should indeed collect
+ // chrome crashes. This allows the crashes to still be tracked when
+ // they occur in autotests. See "crosbug.com/17987".
+ if (file_util::PathExists(FilePath(kCollectChromeFile)))
+ return true;
+ }
+ // We default to ignoring chrome crashes.
+ return false;
+}
+
+bool CrashCollector::IsUserSpecificDirectoryEnabled() {
+ return !ShouldHandleChromeCrashes();
+}
diff --git a/crash_reporter/crash_collector.h b/crash_reporter/crash_collector.h
index 28503a1..8759fc8 100644
--- a/crash_reporter/crash_collector.h
+++ b/crash_reporter/crash_collector.h
@@ -133,6 +133,13 @@
// Returns true if the a crash test is currently running.
bool IsCrashTestInProgress();
+ // Returns true if we should consider ourselves to be running on a
+ // developer image.
+ bool IsDeveloperImage();
+ // Returns true if chrome crashes should be handled.
+ bool ShouldHandleChromeCrashes();
+ // Returns true if user crash directory may be used.
+ bool IsUserSpecificDirectoryEnabled();
CountCrashFunction count_crash_function_;
IsFeedbackAllowedFunction is_feedback_allowed_function_;
diff --git a/crash_reporter/user_collector.cc b/crash_reporter/user_collector.cc
index da42655..f53a61b 100644
--- a/crash_reporter/user_collector.cc
+++ b/crash_reporter/user_collector.cc
@@ -39,9 +39,6 @@
// crashes, but finite to avoid infinitely recursing on crash handling.
static const char kCorePipeLimit[] = "4";
static const char kCoreToMinidumpConverterPath[] = "/usr/bin/core2md";
-static const char kLeaveCoreFile[] = "/root/.leave_core";
-static const char kCollectChromeFile[] =
- "/mnt/stateful_partition/etc/collect_chrome_crashes";
static const char kDefaultLogConfig[] = "/etc/crash_reporter_logs.conf";
@@ -418,32 +415,9 @@
return re.FullMatch(crash_attributes, pid, signal, kernel_supplied_name);
}
-bool UserCollector::IsDeveloperImage() {
- // If we're testing crash reporter itself, we don't want to special-case
- // for developer images.
- if (IsCrashTestInProgress())
- return false;
- return file_util::PathExists(FilePath(kLeaveCoreFile));
-}
-
-bool UserCollector::ShouldIgnoreChromeCrashes() {
- // If we're testing crash reporter itself, we don't want to allow an
- // override for chrome crashes. And, let's be conservative and only
- // allow an override for developer images.
- if (!IsCrashTestInProgress() && IsDeveloperImage()) {
- // Check if there's an override to indicate we should indeed collect
- // chrome crashes. This allows the crashes to still be tracked when
- // they occur in autotests. See "crosbug.com/17987".
- if (file_util::PathExists(FilePath(kCollectChromeFile)))
- return false;
- }
- // We default to ignoring chrome crashes.
- return true;
-}
-
bool UserCollector::ShouldDump(bool has_owner_consent,
bool is_developer,
- bool ignore_chrome_crashes,
+ bool handle_chrome_crashes,
const std::string &exec,
std::string *reason) {
reason->clear();
@@ -452,7 +426,7 @@
// crashes towards user crashes, so user crashes really mean non-Chrome
// user-space crashes.
if ((exec == "chrome" || exec == "supplied_chrome") &&
- ignore_chrome_crashes) {
+ !handle_chrome_crashes) {
*reason = "ignoring - chrome crash";
return false;
}
@@ -512,7 +486,7 @@
std::string reason;
bool dump = ShouldDump(is_feedback_allowed_function_(),
IsDeveloperImage(),
- ShouldIgnoreChromeCrashes(),
+ ShouldHandleChromeCrashes(),
exec,
&reason);
diff --git a/crash_reporter/user_collector.h b/crash_reporter/user_collector.h
index 55fee8d..d041628 100644
--- a/crash_reporter/user_collector.h
+++ b/crash_reporter/user_collector.h
@@ -118,14 +118,9 @@
pid_t *pid, int *signal,
std::string *kernel_supplied_name);
- // Returns true if we should consider ourselves to be running on a
- // developer image.
- bool IsDeveloperImage();
- // Returns true if chrome crashes should be ignored.
- bool ShouldIgnoreChromeCrashes();
bool ShouldDump(bool has_owner_consent,
bool is_developer,
- bool ignore_chrome_crashes,
+ bool handle_chrome_crashes,
const std::string &exec,
std::string *reason);
diff --git a/crash_reporter/user_collector_test.cc b/crash_reporter/user_collector_test.cc
index b18783f..7afb712 100644
--- a/crash_reporter/user_collector_test.cc
+++ b/crash_reporter/user_collector_test.cc
@@ -124,12 +124,12 @@
TEST_F(UserCollectorTest, ShouldDumpDeveloperImageOverridesConsent) {
std::string reason;
- EXPECT_TRUE(collector_.ShouldDump(false, true, true,
+ EXPECT_TRUE(collector_.ShouldDump(false, true, false,
"chrome-wm", &reason));
EXPECT_EQ("developer build - not testing - always dumping", reason);
// When running a crash test, behave as normal.
- EXPECT_FALSE(collector_.ShouldDump(false, false, true,
+ EXPECT_FALSE(collector_.ShouldDump(false, false, false,
"chrome-wm", &reason));
EXPECT_EQ("ignoring - no consent", reason);
}
@@ -137,12 +137,13 @@
TEST_F(UserCollectorTest, ShouldDumpChromeOverridesDeveloperImage) {
std::string reason;
// When running a crash test, behave as normal.
- EXPECT_FALSE(collector_.ShouldDump(false, false, true,
+ EXPECT_FALSE(collector_.ShouldDump(false, false, false,
"chrome", &reason));
EXPECT_EQ("ignoring - chrome crash", reason);
- // When in developer mode, test that chrome crashes are not ignored.
- EXPECT_TRUE(collector_.ShouldDump(false, true, false,
+ // When running a developer image, test that chrome crashes are handled
+ // when the "handle_chrome_crashes" flag is set.
+ EXPECT_TRUE(collector_.ShouldDump(false, true, true,
"chrome", &reason));
EXPECT_EQ("developer build - not testing - always dumping",
reason);
@@ -150,11 +151,11 @@
TEST_F(UserCollectorTest, ShouldDumpUseConsentProductionImage) {
std::string result;
- EXPECT_FALSE(collector_.ShouldDump(false, false, true,
+ EXPECT_FALSE(collector_.ShouldDump(false, false, false,
"chrome-wm", &result));
EXPECT_EQ("ignoring - no consent", result);
- EXPECT_TRUE(collector_.ShouldDump(true, false, true,
+ EXPECT_TRUE(collector_.ShouldDump(true, false, false,
"chrome-wm", &result));
EXPECT_EQ("handling", result);
}