Always read CFI section, even when not outputing CFI information. (issue 596002)

[q...@chromium.org]

Reviewers: Mark Mentovai,

Message:
Mark for review, as this is needed for Chrome on iOS current beta.

+Ted as this is reverting part of https://breakpad.appspot.com/517002

Description:
Always read CFI section, even when not outputing CFI information.

On iOS at least, the CFI section contains information needed to
correctly dump the symbols. Even if the CFI section is not dumped,
reading it is necessary to get correcty symbolication.


Please review this at https://breakpad.appspot.com/596002/

Affected files:
M src/common/mac/dump_syms.mm


Index: src/common/mac/dump_syms.mm
diff --git a/src/common/mac/dump_syms.mm b/src/common/mac/dump_syms.mm
index
113a16c0afea64c5066c2a0b47906b995a3b00d7..5c5e13c9ac00006b03d5e0b40703b5c1e030766d
100644
--- a/src/common/mac/dump_syms.mm
+++ b/src/common/mac/dump_syms.mm
@@ -404,7 +404,7 @@ bool
DumpSymbols::LoadCommandDumper::SegmentCommand(const Segment &segment) {
if (!reader_.MapSegmentSections(segment, &section_map))
return false;

- if (segment.name == "__TEXT" && symbol_data_ != NO_CFI) {
+ if (segment.name == "__TEXT") {
module_->SetLoadAddress(segment.vmaddr);
mach_o::SectionMap::const_iterator eh_frame =
section_map.find("__eh_frame");
@@ -422,13 +422,11 @@ bool
DumpSymbols::LoadCommandDumper::SegmentCommand(const Segment &segment) {
return false;
}
}
- if (symbol_data_ != NO_CFI) {
- mach_o::SectionMap::const_iterator debug_frame
- = section_map.find("__debug_frame");
- if (debug_frame != section_map.end()) {
- // If there is a problem reading this, don't treat it as a fatal
error.
- dumper_.ReadCFI(module_, reader_, debug_frame->second, false);
- }
+ mach_o::SectionMap::const_iterator debug_frame
+ = section_map.find("__debug_frame");
+ if (debug_frame != section_map.end()) {
+ // If there is a problem reading this, don't treat it as a fatal
error.
+ dumper_.ReadCFI(module_, reader_, debug_frame->second, false);
}
}

[ted.mie...@gmail.com]

LGTM, I'm not relying on this behavior, it just seemed like the right
thing to do. (We're using the inverse, just loading CFI.)

https://breakpad.appspot.com/596002/

[ma...@chromium.org]

LGTM

https://breakpad.appspot.com/596002/

[q...@chromium.org]

Committed patchset #1 manually as r1182 (presubmit successful).

https://breakpad.appspot.com/596002/