aboutsummaryrefslogtreecommitdiff
path: root/src/common/windows
diff options
context:
space:
mode:
authorScott Graham <scottmg@chromium.org>2017-02-23 10:26:46 -0800
committerScott Graham <scottmg@chromium.org>2017-02-23 18:28:02 +0000
commit19af23e3c033fb79e6157d7c113d50b6473e1243 (patch)
tree23fedc3a7467afdec68e1b7c7678686f3a8dba97 /src/common/windows
parentMake stack sanitization elide pointers to non-executable mappings. (diff)
downloadbreakpad-19af23e3c033fb79e6157d7c113d50b6473e1243.tar.xz
Handle ntdll only emitting PUBLIC at func entry
This handles a case encountered in ntdll.dll symbols for Windows 7, where a PUBLIC would be emitted only for the entry point to the function. The body of the function, however, is split in a PGO-ish fashion to another remote location in the binary. Because of this, there were large gaps in the RVA space that would be attributed to the "last" function that happened to have an entry point before the gap. In practice, something like this: 0x100 Func1 0x110 Func2 0x120 Func3 0x130 Func4 ... 0x800 LaterFuncs The bodies of Func1/2/3 tend to be implemented as a fast-path check, followed by a jmp to somewhere in the range between 0x130 and 0x800. Because no symbols are emitted for this range, everything is attributed to Func4, causing crash misattribution. In this CL, the change is: after emitting the entry point symbol, also walk in the original OMAP entries through the untranslated binary, and for each block until we resolve to a new symbol (via the same mechanism as we found the entry point) emit another PUBLIC indicating that there's another block that belongs to that symbol. This effectively breaks up the "0x130 - 0x800" range above. R=mark@chromium.org BUG=chromium:678874 Change-Id: Ib3741abab2e7158c81e3e34bca4340ce4d3153a1 Reviewed-on: https://chromium-review.googlesource.com/446717 Reviewed-by: Mark Mentovai <mark@chromium.org>
Diffstat (limited to 'src/common/windows')
-rw-r--r--src/common/windows/omap.cc24
-rw-r--r--src/common/windows/omap_internal.h3
-rw-r--r--src/common/windows/pdb_source_line_writer.cc43
3 files changed, 69 insertions, 1 deletions
diff --git a/src/common/windows/omap.cc b/src/common/windows/omap.cc
index 554a57c2..ba3ce86b 100644
--- a/src/common/windows/omap.cc
+++ b/src/common/windows/omap.cc
@@ -449,6 +449,27 @@ void BuildEndpointIndexMap(ImageMap* image_map) {
}
}
+void BuildSubsequentRVAMap(const OmapData &omap_data,
+ std::map<DWORD, DWORD> *subsequent) {
+ assert(subsequent->empty());
+ const OmapFromTable &orig2tran =
+ reinterpret_cast<const OmapFromTable &>(omap_data.omap_from);
+
+ if (orig2tran.empty())
+ return;
+
+ for (size_t i = 0; i < orig2tran.size() - 1; ++i) {
+ // Expect that orig2tran is sorted.
+ if (orig2tran[i].rva_original >= orig2tran[i + 1].rva_original) {
+ fprintf(stderr, "OMAP 'from' table unexpectedly unsorted\n");
+ subsequent->clear();
+ return;
+ }
+ subsequent->insert(std::make_pair(orig2tran[i].rva_original,
+ orig2tran[i + 1].rva_original));
+ }
+}
+
// Clips the given mapped range.
void ClipMappedRangeOriginal(const AddressRange& clip_range,
MappedRange* mapped_range) {
@@ -576,6 +597,7 @@ void BuildImageMap(const OmapData& omap_data, ImageMap* image_map) {
BuildMapping(omap_data, &image_map->mapping);
BuildEndpointIndexMap(image_map);
+ BuildSubsequentRVAMap(omap_data, &image_map->subsequent_rva_block);
}
void MapAddressRange(const ImageMap& image_map,
@@ -691,4 +713,4 @@ void MapAddressRange(const ImageMap& image_map,
return;
}
-} // namespace google_breakpad \ No newline at end of file
+} // namespace google_breakpad
diff --git a/src/common/windows/omap_internal.h b/src/common/windows/omap_internal.h
index 3f904d7a..2a4713d9 100644
--- a/src/common/windows/omap_internal.h
+++ b/src/common/windows/omap_internal.h
@@ -35,6 +35,7 @@
#include <windows.h>
#include <dia2.h>
+#include <map>
#include <vector>
namespace google_breakpad {
@@ -130,6 +131,8 @@ struct ImageMap {
// an interval in |mapping| that contains the endpoint. Useful for doing
// interval intersection queries.
EndpointIndexMap endpoint_index_map;
+
+ std::map<DWORD, DWORD> subsequent_rva_block;
};
} // namespace google_breakpad
diff --git a/src/common/windows/pdb_source_line_writer.cc b/src/common/windows/pdb_source_line_writer.cc
index 01f4ce3b..c2e22e40 100644
--- a/src/common/windows/pdb_source_line_writer.cc
+++ b/src/common/windows/pdb_source_line_writer.cc
@@ -122,6 +122,16 @@ class AutoImage {
PLOADED_IMAGE img_;
};
+bool SymbolsMatch(IDiaSymbol* a, IDiaSymbol* b) {
+ DWORD a_section, a_offset, b_section, b_offset;
+ if (FAILED(a->get_addressSection(&a_section)) ||
+ FAILED(a->get_addressOffset(&a_offset)) ||
+ FAILED(b->get_addressSection(&b_section)) ||
+ FAILED(b->get_addressOffset(&b_offset)))
+ return false;
+ return a_section == b_section && a_offset == b_offset;
+}
+
bool CreateDiaDataSourceInstance(CComPtr<IDiaDataSource> &data_source) {
if (SUCCEEDED(data_source.CoCreateInstance(CLSID_DiaSource))) {
return true;
@@ -856,6 +866,39 @@ bool PDBSourceLineWriter::PrintCodePublicSymbol(IDiaSymbol *symbol) {
stack_param_size > 0 ? stack_param_size : 0,
name.m_str);
}
+
+ // Now walk the function in the original untranslated space, asking DIA
+ // what function is at that location, stepping through OMAP blocks. If
+ // we're still in the same function, emit another entry, because the
+ // symbol could have been split into multiple pieces. If we've gotten to
+ // another symbol in the original address space, then we're done for
+ // this symbol. See https://crbug.com/678874.
+ for (;;) {
+ // This steps to the next block in the original image. Simply doing
+ // rva++ would also be correct, but would emit tons of unnecessary
+ // entries.
+ rva = image_map_.subsequent_rva_block[rva];
+ if (rva == 0)
+ break;
+
+ CComPtr<IDiaSymbol> next_sym = NULL;
+ LONG displacement;
+ if (FAILED(session_->findSymbolByRVAEx(rva, SymTagPublicSymbol, &next_sym,
+ &displacement))) {
+ break;
+ }
+
+ if (!SymbolsMatch(symbol, next_sym))
+ break;
+
+ AddressRangeVector next_ranges;
+ MapAddressRange(image_map_, AddressRange(rva, 1), &next_ranges);
+ for (size_t i = 0; i < next_ranges.size(); ++i) {
+ fprintf(output_, "PUBLIC %x %x %ws\n", next_ranges[i].rva,
+ stack_param_size > 0 ? stack_param_size : 0, name.m_str);
+ }
+ }
+
return true;
}