diff options
author | mark@chromium.org <mark@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-10-03 21:25:44 +0000 |
---|---|---|
committer | mark@chromium.org <mark@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-10-03 21:25:44 +0000 |
commit | 5ef85a1e92fd9c338ec0eaa7cff8174a2c1076a8 (patch) | |
tree | e9dee63b8ade726bccbb742fc9f782b8a695355a /breakpad | |
parent | 18dab373b1ea0cadae9aac8c2406972e26c613dd (diff) | |
download | chromium_src-5ef85a1e92fd9c338ec0eaa7cff8174a2c1076a8.zip chromium_src-5ef85a1e92fd9c338ec0eaa7cff8174a2c1076a8.tar.gz chromium_src-5ef85a1e92fd9c338ec0eaa7cff8174a2c1076a8.tar.bz2 |
Update Breakpad to r842, picking up:
------------------------------------------------------------------------
r842 | mark@chromium.org | 2011-10-03 15:54:28 -0400 (Mon, 03 Oct 2011) | 11 lines
Use a bootstrap subset port for the inspector, tying the subset to the
lifetime of the task to be monitored, the invoking task. This allows the
bootstrap server (in launchd) to automatically clean up the Mach server
registration when the task being monitored exits, avoiding leaks of
com.Breakpad.Inspector(pid) ports in "launchctl bslist".
BUG=chromium:28547
TEST=Handler should still crash catches, but inspector ports should no longer
show up in "launchctl bslist". They should show up under a subset port in
"launchctl bstree" instead. "launchctl bstree" must be invoked as root.
Review URL: http://breakpad.appspot.com/306001
------------------------------------------------------------------------
This gives each Breakpad instance its own bootstrap subset port as a subset
of the process' bootstrap port, and places the Breakpad inspector port into
the bootstrap subset port.
This completely eliminates leaks of on-demand Mach services advertised via the
bootstrap server.
This also reverts r34318 and r34534, "temporary" (21-month) hacks to mitigate
the leak. The temporary hacks were never completely effective against Breakpad
ports leaking from child processes.
DestructCrashReporter at process shutdown was deemed unnecessary and is being
removed. As this was the last caller to that function, the implementation is
removed as well.
This is addressed in Breakpad rather than Chrome because a mechanism to fix it
without changing the process' bootstrap port was discovered. It was determined
that the rohitfork port is already immune to leaks of the sort that the
Breakpad inspector port experiences. A previous attempt to fix this bug was
r103089, but that caused bug 98550. This strategy is safer.
The forked Breakpad module.cc is updated to match upstream r835, which allows
the unforking of the CFI-disabling code. A new CFI-disabling "-c" option to
dump_syms is now available and used by the dump_product_syms script.
BUG=28547
TEST=1. "launchctl bslist" should no longer show on-demand
com.Breakpad.Inspector ports (in Breakpad-enabled builds with Breakpad
on) ports.
2. "launchctl bstree" (as root) should reveal a bootstrap subset for each
process as a child of the per-user/per-session bootstrap namespace if
crash reporting is on. One com.Breakpad.Inspector port should show up
as a child of each bootstrap subset. As each process exits (even if
mercilessly killed by "kill -9"), the associated subsets and inspector
ports should disappear.
3. Breakpad reports should be generated on crashes. For example,
about:crash and about:inducebrowsercrashforrealz should each cause a
minidump to be written in
~/Library/Application Support/Google/Chrome/Crash Reports
when Breakpad is enabled. This tests that the Breakpad ports are
functioning properly.
4. The browser process should be able to access child process data.
Window:Task Manager should show valid values for the Memory, CPU, and
Network columns for all child processes. This tests that the rohitfork
port is functioning properly. Note that this version of the change
does not change the handling of rohitfork ports at all, but this test
case was useful in a previous version of this patch.
5. Test case from bug 98550: Have a link in a web browser window (in a
different browser) and drag it into Chrome. Expect the drag operation
to operate properly in both the tab strip and in web content. See bug
98550 for details.
6. Unreported test case that failed in r103089: browser relaunch should
work. Visit chrome://flags, change some flags to get the "Relaunch Now"
button at the bottom (you can put the flags back to how you found them
initially), and click "Relaunch Now". The browser should close and then
be relaunched.
Review URL: http://codereview.chromium.org/8120007
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@103778 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'breakpad')
-rw-r--r-- | breakpad/breakpad.gyp | 2 | ||||
-rw-r--r-- | breakpad/pending/src/common/module.cc | 93 |
2 files changed, 49 insertions, 46 deletions
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 |