crash-reporter: Avoid using system to invoke core2md

Change-Id: I1fc0b4dd6e4e84cb999ba61fedd43fc74a8fa1ba

BUG=6941
TEST=bvts

Review URL: http://codereview.chromium.org/3755011
This commit is contained in:
Ken Mixter 2010-10-18 14:42:20 -07:00
parent c909b69446
commit 2953c3a87a
4 changed files with 158 additions and 20 deletions

View file

@ -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_;

View file

@ -4,12 +4,17 @@
#include "crash-reporter/user_collector.h"
#include <fcntl.h> // For creat.
#include <grp.h> // For struct group.
#include <pwd.h> // For struct passwd.
#include <sys/types.h> // For getpwuid_r and getgrnam_r.
#include <sys/types.h> // For getpwuid_r, getgrnam_r, WEXITSTATUS.
#include <sys/wait.h> // For waitpid.
#include <unistd.h> // For execv and fork.
#include <string>
#include <vector>
#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<const char *> &arguments,
const char *output_file) {
// Copy off a writeable version of arguments.
scoped_array<char*> 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<char> 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<const char *> 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;

View file

@ -6,6 +6,7 @@
#define _CRASH_REPORTER_USER_COLLECTOR_H_
#include <string>
#include <vector>
#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<const char *> &arguments,
const char *output_file);
bool ConvertCoreToMinidump(const FilePath &core_path,
const FilePath &procfs_directory,
const FilePath &minidump_path,

View file

@ -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<const char *> 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");