diff options
| -rw-r--r-- | DEPS | 2 | ||||
| -rw-r--r-- | breakpad/breakpad.gyp | 2 | ||||
| -rw-r--r-- | breakpad/pending/src/common/module.cc | 93 | ||||
| -rw-r--r-- | chrome/app/breakpad_mac.h | 3 | ||||
| -rw-r--r-- | chrome/app/breakpad_mac.mm | 7 | ||||
| -rw-r--r-- | chrome/app/breakpad_mac_stubs.mm | 3 | ||||
| -rw-r--r-- | chrome/app/chrome_main.cc | 5 | ||||
| -rw-r--r-- | chrome/renderer/chrome_render_process_observer.cc | 140 | ||||
| -rwxr-xr-x | chrome/tools/build/mac/dump_product_syms | 7 |
9 files changed, 55 insertions, 207 deletions
@@ -42,7 +42,7 @@ vars = { deps = { "src/breakpad/src": - (Var("googlecode_url") % "google-breakpad") + "/trunk/src@828", + (Var("googlecode_url") % "google-breakpad") + "/trunk/src@842", "src/build/util/support": "/trunk/deps/support@20411", diff --git a/breakpad/breakpad.gyp b/breakpad/breakpad.gyp index d9832a1..70906cd 100644 --- a/breakpad/breakpad.gyp +++ b/breakpad/breakpad.gyp @@ -390,7 +390,9 @@ 'src/client/linux/minidump_writer/minidump_writer_unittest.cc', 'src/common/linux/file_id_unittest.cc', 'src/common/linux/linux_libc_support_unittest.cc', + 'src/common/linux/synth_elf.cc', 'src/common/memory_unittest.cc', + 'src/common/test_assembler.cc', ], 'include_dirs': [ diff --git a/breakpad/pending/src/common/module.cc b/breakpad/pending/src/common/module.cc index 1ce09c8..cd92770 100644 --- a/breakpad/pending/src/common/module.cc +++ b/breakpad/pending/src/common/module.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 Google Inc. +// Copyright (c) 2011 Google Inc. // All rights reserved. // // Redistribution and use in source and binary forms, with or without @@ -38,7 +38,8 @@ #include <stdio.h> #include <string.h> -#include <ostream> +#include <iostream> +#include <utility> namespace google_breakpad { @@ -56,16 +57,17 @@ Module::Module(const string &name, const string &os, load_address_(0) { } Module::~Module() { - for (FileByNameMap::iterator it = files_.begin(); it != files_.end(); it++) + for (FileByNameMap::iterator it = files_.begin(); it != files_.end(); ++it) delete it->second; for (FunctionSet::iterator it = functions_.begin(); - it != functions_.end(); it++) + it != functions_.end(); ++it) { delete *it; + } for (vector<StackFrameEntry *>::iterator it = stack_frame_entries_.begin(); - it != stack_frame_entries_.end(); it++) + it != stack_frame_entries_.end(); ++it) { delete *it; - for (ExternSet::iterator it = externs_.begin(); - it != externs_.end(); it++) + } + for (ExternSet::iterator it = externs_.begin(); it != externs_.end(); ++it) delete *it; } @@ -84,7 +86,7 @@ void Module::AddFunction(Function *function) { void Module::AddFunctions(vector<Function *>::iterator begin, vector<Function *>::iterator end) { - for (vector<Function *>::iterator it = begin; it != end; it++) + for (vector<Function *>::iterator it = begin; it != end; ++it) AddFunction(*it); } @@ -146,7 +148,7 @@ Module::File *Module::FindExistingFile(const string &name) { void Module::GetFiles(vector<File *> *vec) { vec->clear(); - for (FileByNameMap::iterator it = files_.begin(); it != files_.end(); it++) + for (FileByNameMap::iterator it = files_.begin(); it != files_.end(); ++it) vec->push_back(it->second); } @@ -157,16 +159,17 @@ void Module::GetStackFrameEntries(vector<StackFrameEntry *> *vec) { void Module::AssignSourceIds() { // First, give every source file an id of -1. for (FileByNameMap::iterator file_it = files_.begin(); - file_it != files_.end(); file_it++) + file_it != files_.end(); ++file_it) { file_it->second->source_id = -1; + } // Next, mark all files actually cited by our functions' line number // info, by setting each one's source id to zero. for (FunctionSet::const_iterator func_it = functions_.begin(); - func_it != functions_.end(); func_it++) { + func_it != functions_.end(); ++func_it) { Function *func = *func_it; for (vector<Line>::iterator line_it = func->lines.begin(); - line_it != func->lines.end(); line_it++) + line_it != func->lines.end(); ++line_it) line_it->file->source_id = 0; } @@ -176,9 +179,10 @@ void Module::AssignSourceIds() { // lexicographical order by name, which is neat. int next_source_id = 0; for (FileByNameMap::iterator file_it = files_.begin(); - file_it != files_.end(); file_it++) + file_it != files_.end(); ++file_it) { if (!file_it->second->source_id) file_it->second->source_id = next_source_id++; + } } bool Module::ReportError() { @@ -189,7 +193,7 @@ bool Module::ReportError() { bool Module::WriteRuleMap(const RuleMap &rule_map, std::ostream &stream) { for (RuleMap::const_iterator it = rule_map.begin(); - it != rule_map.end(); it++) { + it != rule_map.end(); ++it) { if (it != rule_map.begin()) stream << ' '; stream << it->first << ": " << it->second; @@ -197,7 +201,7 @@ bool Module::WriteRuleMap(const RuleMap &rule_map, std::ostream &stream) { return stream.good(); } -bool Module::Write(std::ostream &stream) { +bool Module::Write(std::ostream &stream, bool cfi) { stream << "MODULE " << os_ << " " << architecture_ << " " << id_ << " " << name_ << endl; if (!stream.good()) @@ -207,7 +211,7 @@ bool Module::Write(std::ostream &stream) { // Write out files. for (FileByNameMap::iterator file_it = files_.begin(); - file_it != files_.end(); file_it++) { + file_it != files_.end(); ++file_it) { File *file = file_it->second; if (file->source_id >= 0) { stream << "FILE " << file->source_id << " " << file->name << endl; @@ -218,7 +222,7 @@ bool Module::Write(std::ostream &stream) { // Write out functions and their lines. for (FunctionSet::const_iterator func_it = functions_.begin(); - func_it != functions_.end(); func_it++) { + func_it != functions_.end(); ++func_it) { Function *func = *func_it; assert(!func->name.empty()); stream << "FUNC " << hex @@ -226,11 +230,11 @@ bool Module::Write(std::ostream &stream) { << func->size << " " << func->parameter_size << " " << func->name << dec << endl; - + if (!stream.good()) return ReportError(); for (vector<Line>::iterator line_it = func->lines.begin(); - line_it != func->lines.end(); line_it++) { + line_it != func->lines.end(); ++line_it) { stream << hex << (line_it->address - load_address_) << " " << line_it->size << " " @@ -244,7 +248,7 @@ bool Module::Write(std::ostream &stream) { // Write out 'PUBLIC' records. for (ExternSet::const_iterator extern_it = externs_.begin(); - extern_it != externs_.end(); extern_it++) { + extern_it != externs_.end(); ++extern_it) { Extern *ext = *extern_it; stream << "PUBLIC " << hex << (ext->address - load_address_) << " 0 " @@ -253,39 +257,36 @@ bool Module::Write(std::ostream &stream) { return ReportError(); } -#if 0 - // Bypass all of the "STACK CFI" stuff for the time being to work around - // http://code.google.com/p/google-breakpad/issues/detail?id=443. - // This should be fine as long as -fomit-frame-pointer is not in use. - // Write out 'STACK CFI INIT' and 'STACK CFI' records. - vector<StackFrameEntry *>::const_iterator frame_it; - for (frame_it = stack_frame_entries_.begin(); - frame_it != stack_frame_entries_.end(); frame_it++) { - StackFrameEntry *entry = *frame_it; - stream << "STACK CFI INIT " << hex - << (entry->address - load_address_) << " " - << entry->size << " " << dec; - if (!stream.good() - || !WriteRuleMap(entry->initial_rules, stream)) - return ReportError(); - - stream << endl; - - // Write out this entry's delta rules as 'STACK CFI' records. - for (RuleChangeMap::const_iterator delta_it = entry->rule_changes.begin(); - delta_it != entry->rule_changes.end(); delta_it++) { - stream << "STACK CFI " << hex - << (delta_it->first - load_address_) << " " << dec; + if (cfi) { + // Write out 'STACK CFI INIT' and 'STACK CFI' records. + vector<StackFrameEntry *>::const_iterator frame_it; + for (frame_it = stack_frame_entries_.begin(); + frame_it != stack_frame_entries_.end(); ++frame_it) { + StackFrameEntry *entry = *frame_it; + stream << "STACK CFI INIT " << hex + << (entry->address - load_address_) << " " + << entry->size << " " << dec; if (!stream.good() - || !WriteRuleMap(delta_it->second, stream)) + || !WriteRuleMap(entry->initial_rules, stream)) return ReportError(); stream << endl; + + // Write out this entry's delta rules as 'STACK CFI' records. + for (RuleChangeMap::const_iterator delta_it = entry->rule_changes.begin(); + delta_it != entry->rule_changes.end(); ++delta_it) { + stream << "STACK CFI " << hex + << (delta_it->first - load_address_) << " " << dec; + if (!stream.good() + || !WriteRuleMap(delta_it->second, stream)) + return ReportError(); + + stream << endl; + } } } -#endif return true; } -} // namespace google_breakpad +} // namespace google_breakpad diff --git a/chrome/app/breakpad_mac.h b/chrome/app/breakpad_mac.h index b737f0d..8ff1d26 100644 --- a/chrome/app/breakpad_mac.h +++ b/chrome/app/breakpad_mac.h @@ -19,7 +19,4 @@ void InitCrashProcessInfo(); // Is Breakpad enabled? bool IsCrashReporterEnabled(); -// Call on clean process shutdown. -void DestructCrashReporter(); - #endif // CHROME_APP_BREAKPAD_MAC_H_ diff --git a/chrome/app/breakpad_mac.mm b/chrome/app/breakpad_mac.mm index 4f4d11c..f078271 100644 --- a/chrome/app/breakpad_mac.mm +++ b/chrome/app/breakpad_mac.mm @@ -56,13 +56,6 @@ bool IsCrashReporterEnabled() { return gBreakpadRef != NULL; } -void DestructCrashReporter() { - if (gBreakpadRef) { - BreakpadRelease(gBreakpadRef); - gBreakpadRef = NULL; - } -} - // Only called for a branded build of Chrome.app. void InitCrashReporter() { DCHECK(!gBreakpadRef); diff --git a/chrome/app/breakpad_mac_stubs.mm b/chrome/app/breakpad_mac_stubs.mm index db2db6b..f8a651a 100644 --- a/chrome/app/breakpad_mac_stubs.mm +++ b/chrome/app/breakpad_mac_stubs.mm @@ -16,8 +16,5 @@ bool IsCrashReporterEnabled() { void InitCrashProcessInfo() { } -void DestructCrashReporter() { -} - void InitCrashReporter() { } diff --git a/chrome/app/chrome_main.cc b/chrome/app/chrome_main.cc index 4d7fbe4..a24e1b3 100644 --- a/chrome/app/chrome_main.cc +++ b/chrome/app/chrome_main.cc @@ -673,11 +673,6 @@ class ChromeMainDelegate : public content::ContentMainDelegate { ResourceBundle::CleanupSharedInstance(); logging::CleanupChromeLogging(); - -#if defined(OS_MACOSX) && defined(GOOGLE_CHROME_BUILD) - // TODO(mark): See the TODO(mark) at InitCrashReporter. - DestructCrashReporter(); -#endif // OS_MACOSX && GOOGLE_CHROME_BUILD } #if defined(OS_MACOSX) diff --git a/chrome/renderer/chrome_render_process_observer.cc b/chrome/renderer/chrome_render_process_observer.cc index 8288b73..ad41476 100644 --- a/chrome/renderer/chrome_render_process_observer.cc +++ b/chrome/renderer/chrome_render_process_observer.cc @@ -47,11 +47,6 @@ #include "base/win/iat_patch_function.h" #endif -#if defined(OS_MACOSX) -#include "base/eintr_wrapper.h" -#include "chrome/app/breakpad_mac.h" -#endif - using WebKit::WebCache; using WebKit::WebCrossOriginPreflightResultCache; using WebKit::WebFontCache; @@ -190,121 +185,11 @@ class SuicideOnChannelErrorFilter : public IPC::ChannelProxy::MessageFilter { // So, we install a filter on the channel so that we can process this event // here and kill the process. -#if defined(OS_MACOSX) - // TODO(viettrungluu): crbug.com/28547: The following is needed, as a - // stopgap, to avoid leaking due to not releasing Breakpad properly. - // TODO(viettrungluu): Investigate why this is being called. - if (IsCrashReporterEnabled()) { - VLOG(1) << "Cleaning up Breakpad."; - DestructCrashReporter(); - } else { - VLOG(1) << "Breakpad not enabled; no clean-up needed."; - } -#endif // OS_MACOSX - _exit(0); } }; #endif // OS_POSIX -#if defined(OS_MACOSX) -// TODO(viettrungluu): crbug.com/28547: The following signal handling is needed, -// as a stopgap, to avoid leaking due to not releasing Breakpad properly. -// Without this problem, this could all be eliminated. Remove when Breakpad is -// fixed? -// TODO(viettrungluu): Code taken from browser_main.cc (with a bit of editing). -// The code should be properly shared (or this code should be eliminated). -int g_shutdown_pipe_write_fd = -1; - -void SIGTERMHandler(int signal) { - RAW_CHECK(signal == SIGTERM); - - // Reinstall the default handler. We had one shot at graceful shutdown. - struct sigaction action; - memset(&action, 0, sizeof(action)); - action.sa_handler = SIG_DFL; - CHECK(sigaction(signal, &action, NULL) == 0); - - RAW_CHECK(g_shutdown_pipe_write_fd != -1); - size_t bytes_written = 0; - do { - int rv = HANDLE_EINTR( - write(g_shutdown_pipe_write_fd, - reinterpret_cast<const char*>(&signal) + bytes_written, - sizeof(signal) - bytes_written)); - RAW_CHECK(rv >= 0); - bytes_written += rv; - } while (bytes_written < sizeof(signal)); -} - -class ShutdownDetector : public base::PlatformThread::Delegate { - public: - explicit ShutdownDetector(int shutdown_fd) : shutdown_fd_(shutdown_fd) { - CHECK(shutdown_fd_ != -1); - } - - virtual void ThreadMain() { - int signal; - size_t bytes_read = 0; - ssize_t ret; - do { - ret = HANDLE_EINTR( - read(shutdown_fd_, - reinterpret_cast<char*>(&signal) + bytes_read, - sizeof(signal) - bytes_read)); - if (ret < 0) { - NOTREACHED() << "Unexpected error: " << strerror(errno); - break; - } else if (ret == 0) { - NOTREACHED() << "Unexpected closure of shutdown pipe."; - break; - } - bytes_read += ret; - } while (bytes_read < sizeof(signal)); - - if (bytes_read == sizeof(signal)) - VLOG(1) << "Handling shutdown for signal " << signal << "."; - else - VLOG(1) << "Handling shutdown for unknown signal."; - - // Clean up Breakpad if necessary. - if (IsCrashReporterEnabled()) { - VLOG(1) << "Cleaning up Breakpad."; - DestructCrashReporter(); - } else { - VLOG(1) << "Breakpad not enabled; no clean-up needed."; - } - - // Something went seriously wrong, so get out. - if (bytes_read != sizeof(signal)) { - LOG(WARNING) << "Failed to get signal. Quitting ungracefully."; - _exit(1); - } - - // Re-raise the signal. - kill(getpid(), signal); - - // The signal may be handled on another thread. Give that a chance to - // happen. - sleep(3); - - // We really should be dead by now. For whatever reason, we're not. Exit - // immediately, with the exit status set to the signal number with bit 8 - // set. On the systems that we care about, this exit status is what is - // normally used to indicate an exit by this signal's default handler. - // This mechanism isn't a de jure standard, but even in the worst case, it - // should at least result in an immediate exit. - LOG(WARNING) << "Still here, exiting really ungracefully."; - _exit(signal | (1 << 7)); - } - - private: - const int shutdown_fd_; - - DISALLOW_COPY_AND_ASSIGN(ShutdownDetector); -}; -#endif // OS_MACOSX - } // namespace bool ChromeRenderProcessObserver::is_incognito_process_ = false; @@ -330,31 +215,6 @@ ChromeRenderProcessObserver::ChromeRenderProcessObserver( thread->AddFilter(new SuicideOnChannelErrorFilter()); #endif -#if defined(OS_MACOSX) - // TODO(viettrungluu): Code taken from browser_main.cc. - int pipefd[2]; - int ret = pipe(pipefd); - if (ret < 0) { - PLOG(DFATAL) << "Failed to create pipe"; - } else { - int shutdown_pipe_read_fd = pipefd[0]; - g_shutdown_pipe_write_fd = pipefd[1]; - const size_t kShutdownDetectorThreadStackSize = 4096; - if (!base::PlatformThread::CreateNonJoinable( - kShutdownDetectorThreadStackSize, - new ShutdownDetector(shutdown_pipe_read_fd))) { - LOG(DFATAL) << "Failed to create shutdown detector task."; - } - } - - // crbug.com/28547: When Breakpad is in use, handle SIGTERM to avoid leaking - // Mach ports. - struct sigaction action; - memset(&action, 0, sizeof(action)); - action.sa_handler = SIGTERMHandler; - CHECK(sigaction(SIGTERM, &action, NULL) == 0); -#endif - // Configure modules that need access to resources. net::NetModule::SetResourceProvider(chrome_common_net::NetResourceProvider); diff --git a/chrome/tools/build/mac/dump_product_syms b/chrome/tools/build/mac/dump_product_syms index 0ad1447..c46ffac 100755 --- a/chrome/tools/build/mac/dump_product_syms +++ b/chrome/tools/build/mac/dump_product_syms @@ -105,9 +105,12 @@ for SRC_NAME in "${SRC_NAMES[@]}"; do BPAD_SYM_NAME="${SRC_NAME}-${FULL_VERSION}-${ARCH}.breakpad" BPAD_SYM_PATH="${BUILT_PRODUCTS_DIR}/${BPAD_SYM_NAME}" - # Only run dump_syms if the file has changed since the last dump. + # Only run dump_syms if the file has changed since the last dump. Use -c to + # avoid dumping CFI, because the Breakpad stackwalk is incompatible with CFI + # produced by clang. + # http://code.google.com/p/google-breakpad/issues/detail?id=443 if [ "${DWARF_PATH}" -nt "${BPAD_SYM_PATH}" ] ; then - "${BREAKPAD_DUMP_SYMS}" -a "${ARCH}" "${DWARF_PATH}" > "${BPAD_SYM_PATH}" + "${BREAKPAD_DUMP_SYMS}" -a "${ARCH}" -c "${DWARF_PATH}" > "${BPAD_SYM_PATH}" fi # Some executables will show up with variant names. The Breakpad symbol |