crash-reporter: when exe symlink read fails send diags and still ignore chrome crashes
Change-Id: I0f43d5f61936ebb6420c357e58df45761facd4d0
BUG=6299
TEST=unit tests and UserCrash/CrashSender
Review URL: http://codereview.chromium.org/6480009
diff --git a/crash_reporter/crash_reporter.cc b/crash_reporter/crash_reporter.cc
index ffe6cdf..ab4c5ba 100644
--- a/crash_reporter/crash_reporter.cc
+++ b/crash_reporter/crash_reporter.cc
@@ -148,10 +148,13 @@
return 0;
}
+ // Accumulate logs to help in diagnosing failures during user collection.
+ s_system_log.set_accumulating(true);
// Handle the crash, get the name of the process from procfs.
- if (!user_collector->HandleCrash(FLAGS_user, NULL)) {
+ bool handled = user_collector->HandleCrash(FLAGS_user, NULL);
+ s_system_log.set_accumulating(false);
+ if (!handled)
return 1;
- }
return 0;
}
diff --git a/crash_reporter/user_collector.cc b/crash_reporter/user_collector.cc
index a6dd091..6e6c765 100644
--- a/crash_reporter/user_collector.cc
+++ b/crash_reporter/user_collector.cc
@@ -114,6 +114,9 @@
buffer.reset(new char[max_size + 1]);
ssize_t size = readlink(symlink.value().c_str(), buffer.get(), max_size);
if (size < 0) {
+ int saved_errno = errno;
+ logger_->LogError("Readlink failed on %s with %d",
+ symlink.value().c_str(), saved_errno);
return false;
}
buffer[size] = 0;
@@ -136,8 +139,25 @@
bool UserCollector::GetExecutableBaseNameFromPid(uid_t pid,
std::string *base_name) {
FilePath target;
- if (!GetSymlinkTarget(GetProcessPath(pid).Append("exe"), &target))
+ FilePath process_path = GetProcessPath(pid);
+ FilePath exe_path = process_path.Append("exe");
+ if (!GetSymlinkTarget(exe_path, &target)) {
+ logger_->LogInfo("GetSymlinkTarget failed - Path %s DirectoryExists: %d",
+ process_path.value().c_str(),
+ file_util::DirectoryExists(process_path));
+ // Try to further diagnose exe readlink failure cause.
+ struct stat buf;
+ int stat_result = stat(exe_path.value().c_str(), &buf);
+ int saved_errno = errno;
+ if (stat_result < 0) {
+ logger_->LogInfo("stat %s failed: %d %d", exe_path.value().c_str(),
+ stat_result, saved_errno);
+ } else {
+ logger_->LogInfo("stat %s succeeded: st_mode=%d",
+ exe_path.value().c_str(), buf.st_mode);
+ }
return false;
+ }
*base_name = target.BaseName().value();
return true;
}
@@ -247,7 +267,7 @@
if (!file_util::ReadFileToString(process_path.Append("status"),
&status)) {
logger_->LogError("Could not read status file");
- logger_->LogInfo("Path %s FileExists: %d",
+ logger_->LogInfo("Path %s DirectoryExists: %d",
process_path.value().c_str(),
file_util::DirectoryExists(process_path));
return false;
@@ -444,7 +464,7 @@
// Treat Chrome crashes as if the user opted-out. We stop counting Chrome
// crashes towards user crashes, so user crashes really mean non-Chrome
// user-space crashes.
- if (exec == "chrome") {
+ if (exec == "chrome" || exec == "supplied_chrome") {
feedback = false;
handling_string = "ignoring - chrome crash";
}
@@ -457,10 +477,8 @@
if (generate_diagnostics_) {
bool out_of_capacity = false;
- logger_->set_accumulating(true);
bool convert_and_enqueue_result =
ConvertAndEnqueueCrash(pid, exec, &out_of_capacity);
- logger_->set_accumulating(false);
if (!convert_and_enqueue_result) {
if (!out_of_capacity)
EnqueueCollectionErrorLog(pid, exec);
diff --git a/crash_reporter/user_collector.h b/crash_reporter/user_collector.h
index 851d587..21ef14b 100644
--- a/crash_reporter/user_collector.h
+++ b/crash_reporter/user_collector.h
@@ -58,6 +58,7 @@
FRIEND_TEST(UserCollectorTest, CopyOffProcFilesBadPath);
FRIEND_TEST(UserCollectorTest, CopyOffProcFilesBadPid);
FRIEND_TEST(UserCollectorTest, CopyOffProcFilesOK);
+ FRIEND_TEST(UserCollectorTest, GetExecutableBaseNameFromPid);
FRIEND_TEST(UserCollectorTest, GetIdFromStatus);
FRIEND_TEST(UserCollectorTest, GetProcessPath);
FRIEND_TEST(UserCollectorTest, GetSymlinkTarget);
diff --git a/crash_reporter/user_collector_test.cc b/crash_reporter/user_collector_test.cc
index 53e227c..9a1d687 100644
--- a/crash_reporter/user_collector_test.cc
+++ b/crash_reporter/user_collector_test.cc
@@ -158,6 +158,17 @@
ASSERT_EQ(s_crashes, 0);
}
+TEST_F(UserCollectorTest, HandleSuppliedChromeCrashWithMetrics) {
+ s_metrics = true;
+ collector_.HandleCrash("0:2:chrome", NULL);
+ ASSERT_NE(std::string::npos,
+ logging_.log().find(
+ "Received crash notification for supplied_chrome[0] sig 2"));
+ ASSERT_NE(std::string::npos,
+ logging_.log().find("(ignoring - chrome crash)"));
+ ASSERT_EQ(s_crashes, 0);
+}
+
TEST_F(UserCollectorTest, GetProcessPath) {
FilePath path = collector_.GetProcessPath(100);
ASSERT_EQ("/proc/100", path.value());
@@ -167,7 +178,9 @@
FilePath result;
ASSERT_FALSE(collector_.GetSymlinkTarget(FilePath("/does_not_exist"),
&result));
-
+ ASSERT_NE(std::string::npos,
+ logging_.log().find(
+ "Readlink failed on /does_not_exist with 2"));
std::string long_link;
for (int i = 0; i < 50; ++i)
long_link += "0123456789";
@@ -185,6 +198,29 @@
}
}
+TEST_F(UserCollectorTest, GetExecutableBaseNameFromPid) {
+ std::string base_name;
+ EXPECT_FALSE(collector_.GetExecutableBaseNameFromPid(0, &base_name));
+ EXPECT_NE(std::string::npos,
+ logging_.log().find(
+ "Readlink failed on /proc/0/exe with 2"));
+ EXPECT_NE(std::string::npos,
+ logging_.log().find(
+ "GetSymlinkTarget failed - Path "
+ "/proc/0 DirectoryExists: 0"));
+ EXPECT_NE(std::string::npos,
+ logging_.log().find(
+ "stat /proc/0/exe failed: -1 2"));
+
+ logging_.clear();
+ pid_t my_pid = getpid();
+ EXPECT_TRUE(collector_.GetExecutableBaseNameFromPid(my_pid, &base_name));
+ EXPECT_EQ(std::string::npos,
+ logging_.log().find(
+ "Readlink failed"));
+ EXPECT_EQ("user_collector_test", base_name);
+}
+
TEST_F(UserCollectorTest, GetIdFromStatus) {
int id = 1;
EXPECT_FALSE(collector_.GetIdFromStatus(UserCollector::kUserId,
