diff --git a/crash_reporter/system_logging_mock.h b/crash_reporter/system_logging_mock.h index 639fe4257..f4fb13343 100644 --- a/crash_reporter/system_logging_mock.h +++ b/crash_reporter/system_logging_mock.h @@ -18,6 +18,8 @@ class SystemLoggingMock : public SystemLogging { const std::string &log() { return log_; } + void clear() { log_.clear(); } + private: static std::string identity_; std::string log_; diff --git a/crash_reporter/user_collector.cc b/crash_reporter/user_collector.cc index 4047eb49c..bc414ab7a 100644 --- a/crash_reporter/user_collector.cc +++ b/crash_reporter/user_collector.cc @@ -4,12 +4,17 @@ #include "crash-reporter/user_collector.h" +#include // For creat. #include // For struct group. #include // For struct passwd. -#include // For getpwuid_r and getgrnam_r. +#include // For getpwuid_r, getgrnam_r, WEXITSTATUS. +#include // For waitpid. +#include // For execv and fork. #include +#include +#include "base/eintr_wrapper.h" #include "base/file_util.h" #include "base/logging.h" #include "base/string_util.h" @@ -204,27 +209,81 @@ bool UserCollector::CopyStdinToCoreFile(const FilePath &core_path) { return false; } +int UserCollector::ForkExecAndPipe(std::vector &arguments, + const char *output_file) { + // Copy off a writeable version of arguments. + scoped_array argv(new char *[arguments.size() + 1]); + int total_args_size = 0; + for (size_t i = 0; i < arguments.size(); ++i) { + if (arguments[i] == NULL) { + logger_->LogError("Bad parameter"); + return -1; + } + total_args_size += strlen(arguments[i]) + 1; + } + scoped_array buffer(new char[total_args_size]); + char *buffer_pointer = &buffer[0]; + + for (size_t i = 0; i < arguments.size(); ++i) { + argv[i] = buffer_pointer; + strcpy(buffer_pointer, arguments[i]); + buffer_pointer += strlen(arguments[i]); + *buffer_pointer = '\0'; + ++buffer_pointer; + } + argv[arguments.size()] = NULL; + + int pid = fork(); + if (pid < 0) { + logger_->LogError("Fork failed: %d", errno); + return -1; + } + + if (pid == 0) { + int output_handle = creat(output_file, 0700); + if (output_handle < 0) { + logger_->LogError("Could not create %s: %d", output_file, errno); + // Avoid exit() to avoid atexit handlers from parent. + _exit(127); + } + dup2(output_handle, 1); + dup2(output_handle, 2); + execv(argv[0], &argv[0]); + logger_->LogError("Exec failed: %d", errno); + _exit(127); + } + + int status = 0; + if (HANDLE_EINTR(waitpid(pid, &status, 0)) < 0) { + logger_->LogError("Problem waiting for pid: %d", errno); + return -1; + } + if (!WIFEXITED(status)) { + logger_->LogError("Process did not exit normally: %x", status); + return -1; + } + return WEXITSTATUS(status); +} + bool UserCollector::ConvertCoreToMinidump(const FilePath &core_path, const FilePath &procfs_directory, const FilePath &minidump_path, const FilePath &temp_directory) { - // TODO(kmixter): Rewrite to use process_util once it's included in - // libchrome. FilePath output_path = temp_directory.Append("output"); - std::string core2md_command = - StringPrintf("\"%s\" \"%s\" \"%s\" \"%s\" > \"%s\" 2>&1", - kCoreToMinidumpConverterPath, - core_path.value().c_str(), - procfs_directory.value().c_str(), - minidump_path.value().c_str(), - output_path.value().c_str()); - int errorlevel = system(core2md_command.c_str()); + std::vector core2md_arguments; + core2md_arguments.push_back(kCoreToMinidumpConverterPath); + core2md_arguments.push_back(core_path.value().c_str()); + core2md_arguments.push_back(procfs_directory.value().c_str()); + core2md_arguments.push_back(minidump_path.value().c_str()); + + int errorlevel = ForkExecAndPipe(core2md_arguments, + output_path.value().c_str()); std::string output; file_util::ReadFileToString(output_path, &output); if (errorlevel != 0) { logger_->LogInfo("Problem during %s [result=%d]: %s", - core2md_command.c_str(), + kCoreToMinidumpConverterPath, errorlevel, output.c_str()); return false; diff --git a/crash_reporter/user_collector.h b/crash_reporter/user_collector.h index 484445fcb..7d6c7110f 100644 --- a/crash_reporter/user_collector.h +++ b/crash_reporter/user_collector.h @@ -6,6 +6,7 @@ #define _CRASH_REPORTER_USER_COLLECTOR_H_ #include +#include #include "crash-reporter/crash_collector.h" #include "gtest/gtest_prod.h" // for FRIEND_TEST @@ -51,6 +52,7 @@ class UserCollector : public CrashCollector { FRIEND_TEST(UserCollectorTest, CopyOffProcFilesBadPath); FRIEND_TEST(UserCollectorTest, CopyOffProcFilesBadPid); FRIEND_TEST(UserCollectorTest, CopyOffProcFilesOK); + FRIEND_TEST(UserCollectorTest, ForkExecAndPipe); FRIEND_TEST(UserCollectorTest, GetIdFromStatus); FRIEND_TEST(UserCollectorTest, GetProcessPath); FRIEND_TEST(UserCollectorTest, GetSymlinkTarget); @@ -66,6 +68,8 @@ class UserCollector : public CrashCollector { kIdMax }; + static const int kForkProblem = 255; + std::string GetPattern(bool enabled) const; bool SetUpInternal(bool enabled); @@ -86,6 +90,8 @@ class UserCollector : public CrashCollector { bool GetCreatedCrashDirectory(pid_t pid, FilePath *crash_file_path); bool CopyStdinToCoreFile(const FilePath &core_path); + int ForkExecAndPipe(std::vector &arguments, + const char *output_file); bool ConvertCoreToMinidump(const FilePath &core_path, const FilePath &procfs_directory, const FilePath &minidump_path, diff --git a/crash_reporter/user_collector_test.cc b/crash_reporter/user_collector_test.cc index 3390caab9..9dc0d6098 100644 --- a/crash_reporter/user_collector_test.cc +++ b/crash_reporter/user_collector_test.cc @@ -15,6 +15,12 @@ static bool s_metrics = false; static const char kFilePath[] = "/my/path"; +// This test assumes the following standard binaries are installed. +static const char kBinBash[] = "/bin/bash"; +static const char kBinCp[] = "/bin/cp"; +static const char kBinEcho[] = "/bin/echo"; +static const char kBinFalse[] = "/bin/false"; + void CountCrash() { ++s_crashes; } @@ -38,17 +44,22 @@ class UserCollectorTest : public ::testing::Test { protected: void TestEnableOK(bool generate_diagnostics); + void ExpectFileEquals(const char *golden, + const char *file_path) { + std::string contents; + EXPECT_TRUE(file_util::ReadFileToString(FilePath(file_path), + &contents)); + EXPECT_EQ(golden, contents); + } + SystemLoggingMock logging_; UserCollector collector_; pid_t pid_; }; TEST_F(UserCollectorTest, EnableOK) { - std::string contents; ASSERT_TRUE(collector_.Enable()); - ASSERT_TRUE(file_util::ReadFileToString(FilePath("test/core_pattern"), - &contents)); - ASSERT_EQ("|/my/path --signal=%s --pid=%p", contents); + ExpectFileEquals("|/my/path --signal=%s --pid=%p", "test/core_pattern"); ASSERT_EQ(s_crashes, 0); ASSERT_NE(logging_.log().find("Enabling user crash handling"), std::string::npos); @@ -65,11 +76,8 @@ TEST_F(UserCollectorTest, EnableNoFileAccess) { } TEST_F(UserCollectorTest, DisableOK) { - std::string contents; ASSERT_TRUE(collector_.Disable()); - ASSERT_TRUE(file_util::ReadFileToString(FilePath("test/core_pattern"), - &contents)); - ASSERT_EQ("core", contents); + ExpectFileEquals("core", "test/core_pattern"); ASSERT_EQ(s_crashes, 0); ASSERT_NE(logging_.log().find("Disabling user crash handling"), std::string::npos); @@ -85,6 +93,69 @@ TEST_F(UserCollectorTest, DisableNoFileAccess) { std::string::npos); } +TEST_F(UserCollectorTest, ForkExecAndPipe) { + std::vector args; + char output_file[] = "test/fork_out"; + + // Test basic call with stdout. + args.clear(); + args.push_back(kBinEcho); + args.push_back("hello world"); + EXPECT_EQ(0, collector_.ForkExecAndPipe(args, output_file)); + ExpectFileEquals("hello world\n", output_file); + EXPECT_EQ("", logging_.log()); + + // Test non-zero return value + logging_.clear(); + args.clear(); + args.push_back(kBinFalse); + EXPECT_EQ(1, collector_.ForkExecAndPipe(args, output_file)); + ExpectFileEquals("", output_file); + EXPECT_EQ("", logging_.log()); + + // Test bad output_file. + EXPECT_EQ(127, collector_.ForkExecAndPipe(args, "/bad/path")); + + // Test bad executable. + logging_.clear(); + args.clear(); + args.push_back("false"); + EXPECT_EQ(127, collector_.ForkExecAndPipe(args, output_file)); + + // Test stderr captured. + std::string contents; + logging_.clear(); + args.clear(); + args.push_back(kBinCp); + EXPECT_EQ(1, collector_.ForkExecAndPipe(args, output_file)); + EXPECT_TRUE(file_util::ReadFileToString(FilePath(output_file), + &contents)); + EXPECT_NE(std::string::npos, contents.find("missing file operand")); + EXPECT_EQ("", logging_.log()); + + // NULL parameter. + logging_.clear(); + args.clear(); + args.push_back(NULL); + EXPECT_EQ(-1, collector_.ForkExecAndPipe(args, output_file)); + EXPECT_NE(std::string::npos, + logging_.log().find("Bad parameter")); + + // No parameters. + args.clear(); + EXPECT_EQ(127, collector_.ForkExecAndPipe(args, output_file)); + + // Segmentation faulting process. + logging_.clear(); + args.clear(); + args.push_back(kBinBash); + args.push_back("-c"); + args.push_back("kill -SEGV $$"); + EXPECT_EQ(-1, collector_.ForkExecAndPipe(args, output_file)); + EXPECT_NE(std::string::npos, + logging_.log().find("Process did not exit normally")); +} + TEST_F(UserCollectorTest, HandleCrashWithoutMetrics) { s_metrics = false; collector_.HandleCrash(10, 20, "foobar");