From ab9b65e1fd6a1821f0169b2338dd096bc7075742 Mon Sep 17 00:00:00 2001 From: sebmarchand Date: Tue, 2 May 2017 14:35:23 -0700 Subject: [PATCH] Add support for the VS2017 built binaries. There's 2 things that had to be fixed: - There's a new flag value for the 32 bit offset fixups, we used to always see a flag value of 0 but it's now set to 0x2000 in the VS2017 binaries. - Add a workaround for what seems to be a toolchain bug in VS2017, the PDB contain some invalid symbols, see https://developercommunity.visualstudio.com/content/problem/47386/invalid-symbols-when-using-rtti.html With these 2 changes we fully support the VS2017 built binaries. BUG=chromium:683729 Review-Url: https://codereview.chromium.org/2856933003 --- syzygy/pdb/pdb_data.cc | 3 +-- syzygy/pdb/pdb_data.h | 3 +++ syzygy/pe/decomposer.cc | 19 +++++++++++++++++-- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/syzygy/pdb/pdb_data.cc b/syzygy/pdb/pdb_data.cc index e8253553..80fb655c 100644 --- a/syzygy/pdb/pdb_data.cc +++ b/syzygy/pdb/pdb_data.cc @@ -30,8 +30,7 @@ bool PdbFixup::ValidHeader() const { case TYPE_OFFSET_32BIT: case TYPE_OFFSET_8BIT: { - // We've only ever seen offset fixups without flags. - return flags == 0; + return (flags & ~FLAG_OFFSET_32BIT_VS2017) == 0; } default: { diff --git a/syzygy/pdb/pdb_data.h b/syzygy/pdb/pdb_data.h index c490591e..406009d6 100644 --- a/syzygy/pdb/pdb_data.h +++ b/syzygy/pdb/pdb_data.h @@ -289,6 +289,9 @@ struct PdbFixup { }; enum Flags { + // Flag value that we observe for the TYPE_OFFSET_32BIT fixups produced by + // VS2017. + FLAG_OFFSET_32BIT_VS2017 = 0x2000, FLAG_IS_DATA = 0x4000, FLAG_REFERS_TO_CODE = 0x8000, FLAG_UNKNOWN = 0x3fff, diff --git a/syzygy/pe/decomposer.cc b/syzygy/pe/decomposer.cc index 9314d455..1c81f4b8 100644 --- a/syzygy/pe/decomposer.cc +++ b/syzygy/pe/decomposer.cc @@ -1792,6 +1792,7 @@ DiaBrowser::BrowserDirective Decomposer::OnDataSymbol( // Verify that the data symbol does not exceed the size of the block. if (addr + length > block_addr + block->size()) { + base::StringPiece spname(name); // The data symbol can exceed the size of the block in the case of data // imports. For some reason the toolchain emits a global data symbol with // type information equal to the type of the data *pointed* to by the import @@ -1802,10 +1803,24 @@ DiaBrowser::BrowserDirective Decomposer::OnDataSymbol( // generated. This won't be part of the IAT, so we can't even filter based // on that. Instead, we simply ignore global data symbols that exceed the // block size. - base::StringPiece spname(name); - if (sym_tags.size() == 1 && spname.starts_with("_imp_")) { + bool is_imported_data_symbol = (sym_tags.size() == 1 && + spname.starts_with("_imp_")); + // In VS2017 we've noticed that the size returned by IDiaSymbol::get_length + // function is invalid for the objects using RTTI in VS2017. This has been + // reported here: + // https://developercommunity.visualstudio.com/content/problem/47386/invalid-symbols-when-using-rtti.html + // + // In this situation the data symbol that we get always starts 4 bytes after + // the beginning of its parent block and has an identical size. + bool is_vtable_symbol = spname.ends_with("::`vftable'") && + (addr - block_addr == 4) && + length == block->size(); + if (is_imported_data_symbol) { VLOG(1) << "Encountered an imported data symbol \"" << name << "\" that " << "extends past its parent block \"" << block->name() << "\"."; + } else if (is_vtable_symbol) { + VLOG(1) << "Encountered a vtable data symbol \"" << name << "\" that " + << "extends past its parent block \"" << block->name() << "\"."; } else { LOG(ERROR) << "Received data symbol \"" << name << "\" that extends past " << "its parent block \"" << block->name() << "\".";