diff --git a/crash_reporter/crash-reporter.gyp b/crash_reporter/crash-reporter.gyp index b80924b1b..b42657b6e 100644 --- a/crash_reporter/crash-reporter.gyp +++ b/crash_reporter/crash-reporter.gyp @@ -103,6 +103,7 @@ 'chrome_collector_test.cc', 'crash_collector_test.cc', 'crash_collector_test.h', + 'crash_reporter_logs_test.cc', 'kernel_collector_test.cc', 'kernel_collector_test.h', 'testrunner.cc', diff --git a/crash_reporter/crash_collector.cc b/crash_reporter/crash_collector.cc index e009ee4c2..a53bff9f4 100644 --- a/crash_reporter/crash_collector.cc +++ b/crash_reporter/crash_collector.cc @@ -30,19 +30,26 @@ #include #include #include +#include #include -static const char kCollectChromeFile[] = +namespace { + +const char kCollectChromeFile[] = "/mnt/stateful_partition/etc/collect_chrome_crashes"; -static const char kCrashTestInProgressPath[] = "/tmp/crash-test-in-progress"; -static const char kDefaultLogConfig[] = "/etc/crash_reporter_logs.conf"; -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 kUploadVarPrefix[] = "upload_var_"; -static const char kUploadFilePrefix[] = "upload_file_"; +const char kCrashTestInProgressPath[] = "/tmp/crash-test-in-progress"; +const char kDefaultLogConfig[] = "/etc/crash_reporter_logs.conf"; +const char kDefaultUserName[] = "chronos"; +const char kLeaveCoreFile[] = "/root/.leave_core"; +const char kLsbRelease[] = "/etc/lsb-release"; +const char kShellPath[] = "/bin/sh"; +const char kSystemCrashPath[] = "/var/spool/crash"; +const char kUploadVarPrefix[] = "upload_var_"; +const char kUploadFilePrefix[] = "upload_file_"; + +// Key of the lsb-release entry containing the OS version. +const char kLsbVersionKey[] = "CHROMEOS_RELEASE_VERSION"; + // Normally this path is not used. Unfortunately, there are a few edge cases // where we need this. Any process that runs as kDefaultUserName that crashes // is consider a "user crash". That includes the initial Chrome browser that @@ -53,16 +60,18 @@ static const char kUploadFilePrefix[] = "upload_file_"; // This also comes up when running autotests. The GUI is sitting at the login // screen while tests are sshing in, changing users, and triggering crashes as // the user (purposefully). -static const char kFallbackUserCrashPath[] = "/home/chronos/crash"; +const char kFallbackUserCrashPath[] = "/home/chronos/crash"; // Directory mode of the user crash spool directory. -static const mode_t kUserCrashPathMode = 0755; +const mode_t kUserCrashPathMode = 0755; // Directory mode of the system crash spool directory. -static const mode_t kSystemCrashPathMode = 01755; +const mode_t kSystemCrashPathMode = 01755; -static const uid_t kRootOwner = 0; -static const uid_t kRootGroup = 0; +const uid_t kRootOwner = 0; +const uid_t kRootGroup = 0; + +} // namespace // Maximum crash reports per crash spool directory. Note that this is // a separate maximum from the maximum rate at which we upload these @@ -416,65 +425,28 @@ bool CrashCollector::CheckHasCapacity(const FilePath &crash_directory) { return !full; } -bool CrashCollector::IsCommentLine(const std::string &line) { - size_t found = line.find_first_not_of(" "); - return found != std::string::npos && line[found] == '#'; -} - -bool CrashCollector::ReadKeyValueFile( - const FilePath &path, - const char separator, - std::map *dictionary) { - std::string contents; - if (!base::ReadFileToString(path, &contents)) { - return false; - } - typedef std::vector StringVector; - StringVector lines; - base::SplitString(contents, '\n', &lines); - bool any_errors = false; - for (StringVector::iterator line = lines.begin(); line != lines.end(); - ++line) { - // Allow empty strings. - if (line->empty()) - continue; - // Allow comment lines. - if (IsCommentLine(*line)) - continue; - StringVector sides; - base::SplitString(*line, separator, &sides); - if (sides.size() != 2) { - any_errors = true; - continue; - } - dictionary->insert(std::pair(sides[0], sides[1])); - } - return !any_errors; -} - bool CrashCollector::GetLogContents(const FilePath &config_path, const std::string &exec_name, const FilePath &output_file) { - std::map log_commands; - if (!ReadKeyValueFile(config_path, ':', &log_commands)) { + chromeos::KeyValueStore store; + if (!store.Load(config_path)) { LOG(INFO) << "Unable to read log configuration file " << config_path.value(); return false; } - if (log_commands.find(exec_name) == log_commands.end()) + std::string command; + if (!store.GetString(exec_name, &command)) return false; chromeos::ProcessImpl diag_process; diag_process.AddArg(kShellPath); - std::string shell_command = log_commands[exec_name]; - diag_process.AddStringOption("-c", shell_command); + diag_process.AddStringOption("-c", command); diag_process.RedirectOutput(output_file.value()); - int result = diag_process.Run(); + const int result = diag_process.Run(); if (result != 0) { - LOG(INFO) << "Running shell command " << shell_command << "failed with: " - << result; + LOG(INFO) << "Log command \"" << command << "\" exited with " << result; return false; } return true; @@ -500,15 +472,16 @@ void CrashCollector::AddCrashMetaUploadData(const std::string &key, void CrashCollector::WriteCrashMetaData(const FilePath &meta_path, const std::string &exec_name, const std::string &payload_path) { - std::map contents; - if (!ReadKeyValueFile(FilePath(lsb_release_), '=', &contents)) { + chromeos::KeyValueStore store; + if (!store.Load(FilePath(lsb_release_))) { LOG(ERROR) << "Problem parsing " << lsb_release_; // Even though there was some failure, take as much as we could read. } + std::string version("unknown"); - std::map::iterator i; - if ((i = contents.find("CHROMEOS_RELEASE_VERSION")) != contents.end()) { - version = i->second; + if (!store.GetString(kLsbVersionKey, &version)) { + LOG(ERROR) << "Unable to read " << kLsbVersionKey << " from " + << lsb_release_; } int64_t payload_size = -1; base::GetFileSize(FilePath(payload_path), &payload_size); diff --git a/crash_reporter/crash_collector.h b/crash_reporter/crash_collector.h index a1565e3bc..68806d9ed 100644 --- a/crash_reporter/crash_collector.h +++ b/crash_reporter/crash_collector.h @@ -43,10 +43,8 @@ class CrashCollector { FRIEND_TEST(CrashCollectorTest, ForkExecAndPipe); FRIEND_TEST(CrashCollectorTest, FormatDumpBasename); FRIEND_TEST(CrashCollectorTest, Initialize); - FRIEND_TEST(CrashCollectorTest, IsCommentLine); FRIEND_TEST(CrashCollectorTest, IsUserSpecificDirectoryEnabled); FRIEND_TEST(CrashCollectorTest, MetaData); - FRIEND_TEST(CrashCollectorTest, ReadKeyValueFile); FRIEND_TEST(CrashCollectorTest, Sanitize); FRIEND_TEST(CrashCollectorTest, WriteNewFile); FRIEND_TEST(ForkExecAndPipeTest, Basic); @@ -120,15 +118,6 @@ class CrashCollector { // crash. bool CheckHasCapacity(const base::FilePath &crash_directory); - // Checks if the line starts with '#' after optional whitespace. - static bool IsCommentLine(const std::string &line); - - // Read the given file of form [\n...] and return - // a map of its contents. - bool ReadKeyValueFile(const base::FilePath &file, - char separator, - std::map *dictionary); - // Write a log applicable to |exec_name| to |output_file| based on the // log configuration file at |config_path|. bool GetLogContents(const base::FilePath &config_path, diff --git a/crash_reporter/crash_collector_test.cc b/crash_reporter/crash_collector_test.cc index 1548d708a..0ca379262 100644 --- a/crash_reporter/crash_collector_test.cc +++ b/crash_reporter/crash_collector_test.cc @@ -220,59 +220,6 @@ TEST_F(CrashCollectorTest, CheckHasCapacityStrangeNames) { EXPECT_FALSE(CheckHasCapacity()); } -TEST_F(CrashCollectorTest, IsCommentLine) { - EXPECT_FALSE(CrashCollector::IsCommentLine("")); - EXPECT_TRUE(CrashCollector::IsCommentLine("#")); - EXPECT_TRUE(CrashCollector::IsCommentLine("#real comment")); - EXPECT_TRUE(CrashCollector::IsCommentLine(" # real comment")); - EXPECT_FALSE(CrashCollector::IsCommentLine("not comment")); - EXPECT_FALSE(CrashCollector::IsCommentLine(" not comment")); -} - -TEST_F(CrashCollectorTest, ReadKeyValueFile) { - const char *contents = ("a=b\n" - "\n" - " c=d \n"); - FilePath path(test_dir_.Append("keyval")); - std::map dictionary; - std::map::iterator i; - - base::WriteFile(path, contents, strlen(contents)); - - EXPECT_TRUE(collector_.ReadKeyValueFile(path, '=', &dictionary)); - i = dictionary.find("a"); - EXPECT_TRUE(i != dictionary.end() && i->second == "b"); - i = dictionary.find("c"); - EXPECT_TRUE(i != dictionary.end() && i->second == "d"); - - dictionary.clear(); - - contents = ("a=b c d\n" - "e\n" - " f g = h\n" - "i=j\n" - "=k\n" - "#comment=0\n" - "l=\n"); - base::WriteFile(path, contents, strlen(contents)); - - EXPECT_FALSE(collector_.ReadKeyValueFile(path, '=', &dictionary)); - EXPECT_EQ(5, dictionary.size()); - - i = dictionary.find("a"); - EXPECT_TRUE(i != dictionary.end() && i->second == "b c d"); - i = dictionary.find("e"); - EXPECT_TRUE(i == dictionary.end()); - i = dictionary.find("f g"); - EXPECT_TRUE(i != dictionary.end() && i->second == "h"); - i = dictionary.find("i"); - EXPECT_TRUE(i != dictionary.end() && i->second == "j"); - i = dictionary.find(""); - EXPECT_TRUE(i != dictionary.end() && i->second == "k"); - i = dictionary.find("l"); - EXPECT_TRUE(i != dictionary.end() && i->second == ""); -} - TEST_F(CrashCollectorTest, MetaData) { const char kMetaFileBasename[] = "generated.meta"; FilePath meta_file = test_dir_.Append(kMetaFileBasename); @@ -280,7 +227,10 @@ TEST_F(CrashCollectorTest, MetaData) { FilePath payload_file = test_dir_.Append("payload-file"); std::string contents; collector_.lsb_release_ = lsb_release.value(); - const char kLsbContents[] = "CHROMEOS_RELEASE_VERSION=version\n"; + const char kLsbContents[] = + "CHROMEOS_RELEASE_BOARD=lumpy\n" + "CHROMEOS_RELEASE_VERSION=6727.0.2015_01_26_0853\n" + "CHROMEOS_RELEASE_NAME=Chromium OS\n"; ASSERT_TRUE(base::WriteFile(lsb_release, kLsbContents, strlen(kLsbContents))); const char kPayload[] = "foo"; ASSERT_TRUE(base::WriteFile(payload_file, kPayload, strlen(kPayload))); @@ -290,7 +240,7 @@ TEST_F(CrashCollectorTest, MetaData) { const char kExpectedMeta[] = "foo=bar\n" "exec_name=kernel\n" - "ver=version\n" + "ver=6727.0.2015_01_26_0853\n" "payload=test/payload-file\n" "payload_size=3\n" "done=1\n"; @@ -328,7 +278,7 @@ TEST_F(CrashCollectorTest, GetLogContents) { FilePath config_file = test_dir_.Append("crash_config"); FilePath output_file = test_dir_.Append("crash_log"); const char kConfigContents[] = - "foobar:echo hello there | sed -e \"s/there/world/\""; + "foobar=echo hello there | \\\n sed -e \"s/there/world/\""; ASSERT_TRUE( base::WriteFile(config_file, kConfigContents, strlen(kConfigContents))); base::DeleteFile(FilePath(output_file), false); diff --git a/crash_reporter/crash_reporter_logs.conf b/crash_reporter/crash_reporter_logs.conf index 7f543684a..1a69032fb 100644 --- a/crash_reporter/crash_reporter_logs.conf +++ b/crash_reporter/crash_reporter_logs.conf @@ -2,64 +2,102 @@ # Use of this source code is governed by a BSD-style license that can # be found in the LICENSE file. -# This file has the format: -# :\n +# This file is parsed by chromeos::KeyValueStore. It has the format: # -# Where when any executable with the basename crashes, the -# given is executed and its standard output and -# standard error is sent along with the crash report. +# =\n # -# Use caution in modifying this file. Only run common unix commands -# here as these commands will be run when a crash has recently -# occurred and we should avoid running anything that might cause -# another crash. Similarly these command block the notification of -# the crash to parent processes, so commands should execute quickly. -# Commands cannot contain colons. +# Commands may be split across multiple lines using trailing backslashes. +# +# When an executable named crashes, the corresponding command is +# executed and its standard output and standard error are attached to the crash +# report. +# +# Use caution in modifying this file. Only run common Unix commands here, as +# these commands will be run when a crash has recently occurred and we should +# avoid running anything that might cause another crash. Similarly, these +# commands block notification of the crash to parent processes, so commands +# should execute quickly. -update_engine:cat $(ls -1tr /var/log/update_engine | tail -5 | sed s.^./var/log/update_engine/.) | tail -c 50000 +update_engine=cat $(ls -1tr /var/log/update_engine | tail -5 | \ + sed s.^./var/log/update_engine/.) | tail -c 50000 # The cros_installer output is logged into the update engine log file, # so it is handled in the same way as update_engine. -cros_installer:cat $(ls -1tr /var/log/update_engine | tail -5 | sed s.^./var/log/update_engine/.) | tail -c 50000 +cros_installer=cat $(ls -1tr /var/log/update_engine | tail -5 | \ + sed s.^./var/log/update_engine/.) | tail -c 50000 # Dump the last 20 lines of the last two files in Chrome's system and user log # directories, along with the last 20 messages from the session manager. -chrome:for f in $(ls -1rt /var/log/chrome/chrome_[0-9]* | tail -2) $(ls -1rt /home/chronos/u-*/log/chrome_[0-9]* 2>/dev/null | tail -2); do echo "===$f (tail)==="; tail -20 $f; echo EOF; echo; done; echo "===session_manager (tail)==="; awk '$3 ~ "^session_manager\[" { print }' /var/log/messages | tail -20; echo EOF +chrome=\ + for f in $(ls -1rt /var/log/chrome/chrome_[0-9]* | tail -2) \ + $(ls -1rt /home/chronos/u-*/log/chrome_[0-9]* 2>/dev/null | tail -2); do \ + echo "===$f (tail)==="; \ + tail -20 $f; \ + echo EOF; \ + echo; \ + done; \ + echo "===session_manager (tail)==="; \ + awk '$3 ~ "^session_manager\[" { print }' /var/log/messages | tail -20; \ + echo EOF # The following rule is used for generating additional diagnostics when # collection of user crashes fails. This output should not be too large # as it is stored in memory. The output format specified for 'ps' is the # same as with the "u" ("user-oriented") option, except it doesn't show # the commands' arguments (i.e. "comm" instead of "command"). -crash_reporter-user-collection:echo "===ps output==="; ps axw -o user,pid,%cpu,%mem,vsz,rss,tname,stat,start_time,bsdtime,comm | tail -c 25000; echo "===meminfo==="; cat /proc/meminfo +crash_reporter-user-collection=\ + echo "===ps output==="; \ + ps axw -o user,pid,%cpu,%mem,vsz,rss,tname,stat,start_time,bsdtime,comm | \ + tail -c 25000; \ + echo "===meminfo==="; \ + cat /proc/meminfo # This rule is similar to the crash_reporter-user-collection rule, except it is # run for kernel errors reported through udev events. -crash_reporter-udev-collection-change-card0-drm:for dri in /sys/kernel/debug/dri/*; do echo "===$dri/i915_error_state==="; cat $dri/i915_error_state; done +crash_reporter-udev-collection-change-card0-drm=\ + for dri in /sys/kernel/debug/dri/*; do \ + echo "===$dri/i915_error_state==="; \ + cat $dri/i915_error_state; \ + done # When trackpad driver cyapa detects some abnormal behavior, we collect # additional logs from kernel messages. -crash_reporter-udev-collection-change--i2c-cyapa:/usr/sbin/kernel_log_collector.sh cyapa 30 -# When trackpad/touchscreen driver atmel_mxt_ts detects some abnormal behavior, we collect -# additional logs from kernel messages. -crash_reporter-udev-collection-change--i2c-atmel_mxt_ts:/usr/sbin/kernel_log_collector.sh atmel 30 -# When touch device noise are detected, we collect relevant logs. (crosbug.com/p/16788) -crash_reporter-udev-collection---TouchNoise:cat /var/log/touch_noise.log +crash_reporter-udev-collection-change--i2c-cyapa=\ + /usr/sbin/kernel_log_collector.sh cyapa 30 +# When trackpad/touchscreen driver atmel_mxt_ts detects some abnormal behavior, +# we collect additional logs from kernel messages. +crash_reporter-udev-collection-change--i2c-atmel_mxt_ts=\ + /usr/sbin/kernel_log_collector.sh atmel 30 +# When touch device noise are detected, we collect relevant logs. +# (crosbug.com/p/16788) +crash_reporter-udev-collection---TouchNoise=cat /var/log/touch_noise.log # Periodically collect touch event log for debugging (crosbug.com/p/17244) -crash_reporter-udev-collection---TouchEvent:cat /var/log/touch_event.log +crash_reporter-udev-collection---TouchEvent=cat /var/log/touch_event.log # Dump the last 50 lines of the last two powerd log files -- if the job has # already restarted, we want to see the end of the previous instance's logs. -powerd:for f in $(ls -1tr /var/log/power_manager/powerd.[0-9]* | tail -2); do echo "===$(basename $f) (tail)==="; tail -50 $f; echo EOF; done +powerd=\ + for f in $(ls -1tr /var/log/power_manager/powerd.[0-9]* | tail -2); do \ + echo "===$(basename $f) (tail)==="; \ + tail -50 $f; \ + echo EOF; \ + done # If power_supply_info aborts (due to e.g. a bad battery), its failure message # could end up in various places depending on which process was running it. # Attach the end of powerd's log since it might've also logged the underlying # problem. -power_supply_info:echo "===powerd.LATEST (tail)==="; tail -50 /var/log/power_manager/powerd.LATEST; echo EOF +power_supply_info=\ + echo "===powerd.LATEST (tail)==="; \ + tail -50 /var/log/power_manager/powerd.LATEST; \ + echo EOF # powerd_setuid_helper gets run by powerd, so its stdout/stderr will be mixed in # with powerd's stdout/stderr. -powerd_setuid_helper:echo "===powerd.OUT (tail)==="; tail -50 /var/log/powerd.out; echo EOF +powerd_setuid_helper=\ + echo "===powerd.OUT (tail)==="; \ + tail -50 /var/log/powerd.out; \ + echo EOF # The following rules are only for testing purposes. -crash_log_test:echo hello world -crash_log_recursion_test:sleep 1 && /usr/local/autotest/tests/crash_log_recursion_test +crash_log_test=echo hello world +crash_log_recursion_test=sleep 1 && \ + /usr/local/autotest/tests/crash_log_recursion_test diff --git a/crash_reporter/crash_reporter_logs_test.cc b/crash_reporter/crash_reporter_logs_test.cc new file mode 100644 index 000000000..987947093 --- /dev/null +++ b/crash_reporter/crash_reporter_logs_test.cc @@ -0,0 +1,28 @@ +// Copyright 2015 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include + +#include +#include +#include + +namespace { + +// Name of the checked-in configuration file containing log-collection commands. +const char kConfigFile[] = "crash_reporter_logs.conf"; + +// Executable name for Chrome. kConfigFile is expected to contain this entry. +const char kChromeExecName[] = "chrome"; + +} // namespace + +// Tests that the config file is parsable and that Chrome is listed. +TEST(CrashReporterLogsTest, ReadConfig) { + chromeos::KeyValueStore store; + ASSERT_TRUE(store.Load(base::FilePath(kConfigFile))); + std::string command; + EXPECT_TRUE(store.GetString(kChromeExecName, &command)); + EXPECT_FALSE(command.empty()); +} diff --git a/crash_reporter/udev_collector_test.cc b/crash_reporter/udev_collector_test.cc index 66daaf12f..f41b06f22 100644 --- a/crash_reporter/udev_collector_test.cc +++ b/crash_reporter/udev_collector_test.cc @@ -19,9 +19,9 @@ const char kLogConfigFileName[] = "log_config_file"; // A bunch of random rules to put into the dummy log config file. const char kLogConfigFileContents[] = - "crash_reporter-udev-collection-change-card0-drm:echo change card0 drm\n" - "crash_reporter-udev-collection-add-state0-cpu:echo change state0 cpu\n" - "cros_installer:echo not for udev"; + "crash_reporter-udev-collection-change-card0-drm=echo change card0 drm\n" + "crash_reporter-udev-collection-add-state0-cpu=echo change state0 cpu\n" + "cros_installer=echo not for udev"; void CountCrash() {}