Various improvements in debug info support (mainly hardening of erroneous data files), and some minor fixes to dbghelp.
From: Eric Pouech eric.pouech@gmail.com
Introduce struct PDB_STRING_TABLE to describe string table's header.
This patch prevents core dumps with PDB:s where string offset are outside of string table.
Signed-off-by: Eric Pouech eric.pouech@gmail.com --- include/wine/mscvpdb.h | 12 ++++++++++ tools/winedump/debug.c | 1 - tools/winedump/msc.c | 5 ++--- tools/winedump/pdb.c | 47 ++++++++++++++++++++++----------------- tools/winedump/winedump.h | 4 +++- 5 files changed, 44 insertions(+), 25 deletions(-)
diff --git a/include/wine/mscvpdb.h b/include/wine/mscvpdb.h index 69dd42f0c09..5e30a6c2beb 100644 --- a/include/wine/mscvpdb.h +++ b/include/wine/mscvpdb.h @@ -2610,6 +2610,18 @@ typedef struct _PDB_FPO_DATA unsigned int flags; } PDB_FPO_DATA;
+typedef struct _PDB_STRING_TABLE +{ + unsigned int magic; + unsigned int hash_version; + unsigned int length; +} +PDB_STRING_TABLE; +/* This header is followed by: + * - a series (of bytes hdr.length) of 0-terminated strings + * - a serialized hash table + */ + #include "poppack.h"
/* =================================================== diff --git a/tools/winedump/debug.c b/tools/winedump/debug.c index 9cc17f62e2b..e62e4436e4d 100644 --- a/tools/winedump/debug.c +++ b/tools/winedump/debug.c @@ -31,7 +31,6 @@ #include "windef.h" #include "winbase.h" #include "winedump.h" -#include "wine/mscvpdb.h"
/* * .DBG File Layout: diff --git a/tools/winedump/msc.c b/tools/winedump/msc.c index 85bbdfc3487..b5708017651 100644 --- a/tools/winedump/msc.c +++ b/tools/winedump/msc.c @@ -30,7 +30,6 @@ #include "winbase.h" #include "winedump.h" #include "cvconst.h" -#include "wine/mscvpdb.h"
#define PSTRING(adr, ofs) \ ((const struct p_string*)((const char*)(adr) + (ofs))) @@ -2017,7 +2016,7 @@ void codeview_dump_linetab(const char* linetab, BOOL pascal_str, const char* pfx } }
-void codeview_dump_linetab2(const char* linetab, DWORD size, const char* strimage, DWORD strsize, const char* pfx) +void codeview_dump_linetab2(const char* linetab, DWORD size, const PDB_STRING_TABLE* strimage, const char* pfx) { unsigned i; const struct CV_DebugSSubsectionHeader_t* hdr; @@ -2094,7 +2093,7 @@ void codeview_dump_linetab2(const char* linetab, DWORD size, const char* strimag const char* meth[] = {"None", "MD5", "SHA1", "SHA256"}; printf("%s %d] name=%s size=%u method=%s checksum=[", pfx, (unsigned)((const char*)chksms - (const char*)(hdr + 1)), - strimage ? strimage + chksms->strOffset : "--none--", + pdb_get_string_table_entry(strimage, chksms->strOffset), chksms->size, chksms->method < ARRAY_SIZE(meth) ? meth[chksms->method] : "<<unknown>>"); for (i = 0; i < chksms->size; ++i) printf("%02x", chksms->checksum[i]); printf("]\n"); diff --git a/tools/winedump/pdb.c b/tools/winedump/pdb.c index 6774cd11876..9fe358aa86a 100644 --- a/tools/winedump/pdb.c +++ b/tools/winedump/pdb.c @@ -29,7 +29,6 @@ #include "windef.h" #include "winbase.h" #include "winedump.h" -#include "wine/mscvpdb.h"
struct pdb_reader { @@ -206,21 +205,32 @@ static unsigned get_stream_by_name(struct pdb_reader* reader, const char* name) return -1; }
-static void *read_string_table(struct pdb_reader* reader) +static PDB_STRING_TABLE* read_string_table(struct pdb_reader* reader) { - unsigned stream_idx; - void* ret; + unsigned stream_idx; + PDB_STRING_TABLE* ret; + unsigned stream_size;
stream_idx = get_stream_by_name(reader, "/names"); if (stream_idx == -1) return NULL; ret = reader->read_file(reader, stream_idx); if (!ret) return NULL; - if(*(const UINT *)ret == 0xeffeeffe) return ret; - printf("wrong header %x expecting 0xeffeeffe\n", *(const UINT *)ret); + stream_size = pdb_get_file_size(reader, stream_idx); + if (ret->magic == 0xeffeeffe && sizeof(*ret) + ret->length < stream_size) return ret; + printf("Improper string table header (magic=%x)\n", ret->magic); + dump_data((const unsigned char*)ret, stream_size, " "); free( ret ); return NULL; }
+const char* pdb_get_string_table_entry(const PDB_STRING_TABLE* table, unsigned ofs) +{ + if (!table) return "<<no string table>>"; + if (ofs >= table->length) return "<<invalid string table offset>>"; + /* strings start after header */ + return (char*)(table + 1) + ofs; +} + static void dump_global_symbol(struct pdb_reader* reader, unsigned file) { void* global = NULL; @@ -253,12 +263,11 @@ static void dump_public_symbol(struct pdb_reader* reader, unsigned file)
static void pdb_dump_symbols(struct pdb_reader* reader, PDB_STREAM_INDEXES* sidx) { - PDB_SYMBOLS* symbols; - unsigned char* modimage; - const char* file; - char* filesimage; - DWORD filessize = 0; - char tcver[32]; + PDB_SYMBOLS* symbols; + unsigned char* modimage; + const char* file; + PDB_STRING_TABLE* filesimage; + char tcver[32];
sidx->FPO = sidx->unk0 = sidx->unk1 = sidx->unk2 = sidx->unk3 = sidx->segments = sidx->unk4 = sidx->unk5 = sidx->unk6 = sidx->FPO_EXT = sidx->unk7 = -1; @@ -332,7 +341,6 @@ static void pdb_dump_symbols(struct pdb_reader* reader, PDB_STREAM_INDEXES* sidx }
if (!(filesimage = read_string_table(reader))) printf("string table not found\n"); - else filessize = *(const DWORD*)(filesimage + 8);
if (symbols->srcmodule_size) { @@ -592,7 +600,7 @@ static void pdb_dump_symbols(struct pdb_reader* reader, PDB_STREAM_INDEXES* sidx codeview_dump_linetab((const char*)modimage + symbol_size, TRUE, " "); else if (lineno2_size) /* actually, only one of the 2 lineno should be present */ codeview_dump_linetab2((const char*)modimage + symbol_size, lineno2_size, - filesimage ? filesimage + 12 : NULL, filessize, " "); + filesimage, " "); /* what's that part ??? */ if (0) dump_data(modimage + symbol_size + lineno_size + lineno2_size, @@ -644,7 +652,7 @@ static void pdb_dump_types_hash(struct pdb_reader* reader, const PDB_TYPES* type void* hash = NULL; unsigned i, strmsize; const unsigned* table; - char* strbase; + PDB_STRING_TABLE* strbase; unsigned *collision; hash = reader->read_file(reader, types->hash_file); if (!hash) return; @@ -746,7 +754,7 @@ static void pdb_dump_types_hash(struct pdb_reader* reader, const PDB_TYPES* type is_bit_set(deleted_bitset, count_deleted, i) ? 'D' : '_'); if (is_bit_set(present_bitset, count_present, i)) { - printf(" %s => ", strbase + 12 + *table++); + printf(" %s => ", pdb_get_string_table_entry(strbase, *table++)); pdb_dump_hash_value((const BYTE*)table, types->hash_size); table = (const unsigned*)((const BYTE*)table + types->hash_size); } @@ -855,14 +863,13 @@ static void pdb_dump_fpo(struct pdb_reader* reader, unsigned stream_idx) static void pdb_dump_fpo_ext(struct pdb_reader* reader, unsigned stream_idx) { PDB_FPO_DATA* fpoext; - unsigned i, size, strsize; - char* strbase; + unsigned i, size; + PDB_STRING_TABLE* strbase;
if (stream_idx == (WORD)-1) return; strbase = read_string_table(reader); if (!strbase) return;
- strsize = *(const DWORD*)(strbase + 8); fpoext = reader->read_file(reader, stream_idx); size = pdb_get_file_size(reader, stream_idx); if (fpoext && (size % sizeof(*fpoext)) == 0) @@ -875,7 +882,7 @@ static void pdb_dump_fpo_ext(struct pdb_reader* reader, unsigned stream_idx) printf("\t%08x %08x %8x %8x %8x %6x %8x %08x %s\n", fpoext[i].start, fpoext[i].func_size, fpoext[i].locals_size, fpoext[i].params_size, fpoext[i].maxstack_size, fpoext[i].prolog_size, fpoext[i].savedregs_size, fpoext[i].flags, - fpoext[i].str_offset < strsize ? strbase + 12 + fpoext[i].str_offset : "<out of bounds>"); + pdb_get_string_table_entry(strbase, fpoext[i].str_offset)); } } free(fpoext); diff --git a/tools/winedump/winedump.h b/tools/winedump/winedump.h index c84a98946b3..361f44b62b0 100644 --- a/tools/winedump/winedump.h +++ b/tools/winedump/winedump.h @@ -45,6 +45,7 @@ #include "../tools.h" #include "windef.h" #include "winbase.h" +#include "wine/mscvpdb.h"
/* Argument type constants */ #define MAX_FUNCTION_ARGS 32 @@ -263,7 +264,8 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, uns BOOL codeview_dump_types_from_offsets(const void* table, const DWORD* offsets, unsigned num_types); BOOL codeview_dump_types_from_block(const void* table, unsigned long len); void codeview_dump_linetab(const char* linetab, BOOL pascal_str, const char* pfx); -void codeview_dump_linetab2(const char* linetab, DWORD size, const char* strimage, DWORD strsize, const char* pfx); +void codeview_dump_linetab2(const char* linetab, DWORD size, const PDB_STRING_TABLE*, const char* pfx); +const char* pdb_get_string_table_entry(const PDB_STRING_TABLE* table, unsigned ofs);
void dump_stabs(const void* pv_stabs, unsigned szstabs, const char* stabstr, unsigned szstr); void dump_codeview(unsigned long ptr, unsigned long len);
From: Eric Pouech eric.pouech@gmail.com
Making use of PDB_STRING_TABLE.
Signed-off-by: Eric Pouech eric.pouech@gmail.com --- dlls/dbghelp/msc.c | 61 +++++++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 31 deletions(-)
diff --git a/dlls/dbghelp/msc.c b/dlls/dbghelp/msc.c index a51f482cc6f..10fcf398140 100644 --- a/dlls/dbghelp/msc.c +++ b/dlls/dbghelp/msc.c @@ -95,8 +95,7 @@ struct cv_module_snarf const struct codeview_type_parse* ipi_ctp; const struct CV_DebugSSubsectionHeader_t* dbgsubsect; unsigned dbgsubsect_size; - const char* strimage; - unsigned strsize; + const PDB_STRING_TABLE* strimage; };
/*======================================================================== @@ -1633,6 +1632,8 @@ static void codeview_snarf_linetab(const struct msc_debug_info* msc_dbg, const B } }
+static const char* pdb_get_string_table_entry(const PDB_STRING_TABLE* table, unsigned offset); + static void codeview_snarf_linetab2(const struct msc_debug_info* msc_dbg, const struct cv_module_snarf* cvmod) { unsigned i; @@ -1684,8 +1685,7 @@ static void codeview_snarf_linetab2(const struct msc_debug_info* msc_dbg, const WARN("Corrupt PDB file: offset in CHKSMS subsection is invalid\n"); break; } - source = source_new(msc_dbg->module, NULL, - (chksms->strOffset < cvmod->strsize) ? cvmod->strimage + chksms->strOffset : "<<stroutofbounds>>"); + source = source_new(msc_dbg->module, NULL, pdb_get_string_table_entry(cvmod->strimage, chksms->strOffset)); lineblk_base = codeview_get_address(msc_dbg, lines_hdr->segCon, lines_hdr->offCon); lines = CV_RECORD_AFTER(files_hdr); for (i = 0; i < files_hdr->nLines; i++) @@ -2013,8 +2013,7 @@ static BOOL cv_dbgsubsect_find_inlinee(const struct msc_debug_info* msc_dbg, { chksms = CV_RECORD_GAP(hdr_files, inlsrc->fileId); if (!CV_IS_INSIDE(chksms, CV_RECORD_GAP(hdr_files, hdr_files->cbLen))) return FALSE; - *srcfile = source_new(msc_dbg->module, NULL, - (chksms->strOffset < cvmod->strsize) ? cvmod->strimage + chksms->strOffset : "<<str-out-of-bounds>>"); + *srcfile = source_new(msc_dbg->module, NULL, pdb_get_string_table_entry(cvmod->strimage, chksms->strOffset)); *srcline = inlsrc->sourceLineNum; return TRUE; } @@ -2029,8 +2028,7 @@ static BOOL cv_dbgsubsect_find_inlinee(const struct msc_debug_info* msc_dbg, { chksms = CV_RECORD_GAP(hdr_files, inlsrcex->fileId); if (!CV_IS_INSIDE(chksms, CV_RECORD_GAP(hdr_files, hdr_files->cbLen))) return FALSE; - *srcfile = source_new(msc_dbg->module, NULL, - (chksms->strOffset < cvmod->strsize) ? cvmod->strimage + chksms->strOffset : "<<str-out-of-bounds>>"); + *srcfile = source_new(msc_dbg->module, NULL, pdb_get_string_table_entry(cvmod->strimage, chksms->strOffset)); *srcline = inlsrcex->sourceLineNum; return TRUE; } @@ -2150,8 +2148,7 @@ static struct symt_inlinesite* codeview_create_inline_site(const struct msc_debu case BA_OP_ChangeFile: chksms = CV_RECORD_GAP(hdr_files, cvba.arg1); if (CV_IS_INSIDE(chksms, CV_RECORD_GAP(hdr_files, hdr_files->cbLen))) - srcfile = source_new(msc_dbg->module, NULL, - (chksms->strOffset < cvmod->strsize) ? cvmod->strimage + chksms->strOffset : "<<str-out-of-bounds>>"); + srcfile = source_new(msc_dbg->module, NULL, pdb_get_string_table_entry(cvmod->strimage, chksms->strOffset)); break; case BA_OP_ChangeLineOffset: line += binannot_getsigned(cvba.arg1); @@ -3085,22 +3082,28 @@ static unsigned pdb_get_stream_by_name(const struct pdb_file_info* pdb_file, con return -1; }
-static void* pdb_read_strings(const struct pdb_file_info* pdb_file) +static PDB_STRING_TABLE* pdb_read_strings(const struct pdb_file_info* pdb_file) { unsigned idx; - void *ret; + PDB_STRING_TABLE *ret;
idx = pdb_get_stream_by_name(pdb_file, "/names"); if (idx != -1) { ret = pdb_read_file( pdb_file, idx ); - if (ret && *(const DWORD *)ret == 0xeffeeffe) return ret; + if (ret && ret->magic == 0xeffeeffe && + sizeof(*ret) + ret->length <= pdb_get_file_size(pdb_file, idx)) return ret; pdb_free( ret ); } WARN("string table not found\n"); return NULL; }
+static const char* pdb_get_string_table_entry(const PDB_STRING_TABLE* table, unsigned offset) +{ + return (!table || offset >= table->length) ? NULL : (const char*)(table + 1) + offset; +} + static void pdb_module_remove(struct process* pcsn, struct module_format* modfmt) { unsigned i; @@ -3564,13 +3567,12 @@ static BOOL pdb_process_internal(const struct process* pcs, struct pdb_module_info* pdb_module_info, unsigned module_index) { - HANDLE hMap = NULL; - char* image = NULL; - BYTE* symbols_image = NULL; - char* files_image = NULL; - DWORD files_size = 0; - unsigned matched; - struct pdb_file_info* pdb_file; + HANDLE hMap = NULL; + char* image = NULL; + BYTE* symbols_image = NULL; + PDB_STRING_TABLE* files_image = NULL; + unsigned matched; + struct pdb_file_info* pdb_file;
TRACE("Processing PDB file %s\n", pdb_lookup->filename);
@@ -3637,7 +3639,6 @@ static BOOL pdb_process_internal(const struct process* pcs, return FALSE; } files_image = pdb_read_strings(pdb_file); - if (files_image) files_size = *(const DWORD*)(files_image + 8);
pdb_process_symbol_imports(pcs, msc_dbg, &symbols, symbols_image, image, pdb_lookup, pdb_module_info, module_index); @@ -3669,7 +3670,7 @@ static BOOL pdb_process_internal(const struct process* pcs, if (modimage) { struct cv_module_snarf cvmod = {ipi_ok ? &ipi_ctp : NULL, (const void*)(modimage + sfile.symbol_size), sfile.lineno2_size, - files_image + 12, files_size}; + files_image}; codeview_snarf(msc_dbg, modimage, sizeof(DWORD), sfile.symbol_size, &cvmod, TRUE);
@@ -4001,6 +4002,7 @@ static BOOL pdb_parse_cmd_string(struct cpu_stack_walk* csw, PDB_FPO_DATA* fpoe BOOL over = FALSE; struct pevaluator pev;
+ if (!cmd) return FALSE; pev_init(&pev, csw, fpoext, cpair); for (ptr = cmd; !over; ptr++) { @@ -4051,8 +4053,8 @@ BOOL pdb_virtual_unwind(struct cpu_stack_walk *csw, DWORD_PTR ip, struct module_pair pair; struct pdb_module_info* pdb_info; PDB_FPO_DATA* fpoext; - unsigned i, size, strsize; - char* strbase; + unsigned i, size; + PDB_STRING_TABLE* strbase; BOOL ret = TRUE;
if (!module_init_pair(&pair, csw->hProcess, ip)) return FALSE; @@ -4063,7 +4065,6 @@ BOOL pdb_virtual_unwind(struct cpu_stack_walk *csw, DWORD_PTR ip,
strbase = pdb_read_strings(&pdb_info->pdb_files[0]); if (!strbase) return FALSE; - strsize = *(const DWORD*)(strbase + 8); fpoext = pdb_read_file(&pdb_info->pdb_files[0], pdb_info->pdb_files[0].fpoext_stream); size = pdb_get_file_size(&pdb_info->pdb_files[0], pdb_info->pdb_files[0].fpoext_stream); if (fpoext && (size % sizeof(*fpoext)) == 0) @@ -4077,12 +4078,10 @@ BOOL pdb_virtual_unwind(struct cpu_stack_walk *csw, DWORD_PTR ip, fpoext[i].start, fpoext[i].func_size, fpoext[i].locals_size, fpoext[i].params_size, fpoext[i].maxstack_size, fpoext[i].prolog_size, fpoext[i].savedregs_size, fpoext[i].flags, - fpoext[i].str_offset < strsize ? - wine_dbgstr_a(strbase + 12 + fpoext[i].str_offset) : "<out of bounds>"); - if (fpoext[i].str_offset < strsize) - ret = pdb_parse_cmd_string(csw, &fpoext[i], strbase + 12 + fpoext[i].str_offset, cpair); - else - ret = FALSE; + wine_dbgstr_a(pdb_get_string_table_entry(strbase, fpoext[i].str_offset))); + ret = pdb_parse_cmd_string(csw, &fpoext[i], + pdb_get_string_table_entry(strbase, fpoext[i].str_offset), + cpair); break; } }
From: Eric Pouech eric.pouech@gmail.com
Signed-off-by: Eric Pouech eric.pouech@gmail.com --- dlls/dbghelp/msc.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/dlls/dbghelp/msc.c b/dlls/dbghelp/msc.c index 10fcf398140..811c8ffa165 100644 --- a/dlls/dbghelp/msc.c +++ b/dlls/dbghelp/msc.c @@ -3119,15 +3119,15 @@ static void pdb_module_remove(struct process* pcsn, struct module_format* modfmt HeapFree(GetProcessHeap(), 0, modfmt); }
-static void pdb_convert_types_header(PDB_TYPES* types, const BYTE* image) +static BOOL pdb_convert_types_header(PDB_TYPES* types, const BYTE* image) { - memset(types, 0, sizeof(PDB_TYPES)); - if (!image) return; + if (!image) return FALSE;
if (*(const DWORD*)image < 19960000) /* FIXME: correct version? */ { /* Old version of the types record header */ const PDB_TYPES_OLD* old = (const PDB_TYPES_OLD*)image; + memset(types, 0, sizeof(PDB_TYPES)); types->version = old->version; types->type_offset = sizeof(PDB_TYPES_OLD); types->type_size = old->type_size; @@ -3140,6 +3140,7 @@ static void pdb_convert_types_header(PDB_TYPES* types, const BYTE* image) /* New version of the types record header */ *types = *(const PDB_TYPES*)image; } + return TRUE; }
static void pdb_convert_symbols_header(PDB_SYMBOLS* symbols, @@ -3255,7 +3256,8 @@ static BOOL pdb_init_type_parse(const struct msc_debug_info* msc_dbg, ctp->offset = NULL; ctp->hash = NULL; ctp->alloc_hash = NULL; - pdb_convert_types_header(&ctp->header, image); + if (!pdb_convert_types_header(&ctp->header, image)) + return FALSE;
/* Check for unknown versions */ switch (ctp->header.version)
From: Eric Pouech eric.pouech@gmail.com
Signed-off-by: Eric Pouech eric.pouech@gmail.com --- dlls/dbghelp/type.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/dlls/dbghelp/type.c b/dlls/dbghelp/type.c index 3a5d3caf09f..9d39d490965 100644 --- a/dlls/dbghelp/type.c +++ b/dlls/dbghelp/type.c @@ -833,10 +833,17 @@ BOOL symt_get_info(struct module* module, const struct symt* type, return FALSE; break; case SymTagData: - if (((const struct symt_data*)type)->kind != DataIsMember || - !((const struct symt_data*)type)->u.member.bit_length) - return FALSE; - X(DWORD64) = ((const struct symt_data*)type)->u.member.bit_length; + switch (((const struct symt_data*)type)->kind) + { + case DataIsMember: + if (!((const struct symt_data*)type)->u.member.bit_length) + return FALSE; + X(DWORD64) = ((const struct symt_data*)type)->u.member.bit_length; + break; + default: + if (!symt_get_info(module, ((const struct symt_data*)type)->type, TI_GET_LENGTH, pInfo)) + return FALSE; + } break; case SymTagArrayType: if (!symt_get_info(module, ((const struct symt_array*)type)->base_type,
From: Eric Pouech eric.pouech@gmail.com
Signed-off-by: Eric Pouech eric.pouech@gmail.com --- dlls/dbghelp/symbol.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/dlls/dbghelp/symbol.c b/dlls/dbghelp/symbol.c index abf6054d0ee..d019eec4e3a 100644 --- a/dlls/dbghelp/symbol.c +++ b/dlls/dbghelp/symbol.c @@ -1897,7 +1897,7 @@ static BOOL get_line_from_function(struct module_pair* pair, struct symt_functio ret = internal_line_set_nameW(pair->pcs, intl, dospath, TRUE); HeapFree( GetProcessHeap(), 0, dospath ); } - if (ret) *pdwDisplacement = addr - found_dli->u.address; + if (ret && pdwDisplacement) *pdwDisplacement = addr - found_dli->u.address; return ret; } } @@ -2684,7 +2684,7 @@ BOOL WINAPI SymFromInlineContext(HANDLE hProcess, DWORD64 addr, ULONG inline_ctx if (inlined) { symt_fill_sym_info(&pair, NULL, &inlined->func.symt, si); - *disp = addr - inlined->func.address; + if (disp) *disp = addr - inlined->func.address; return TRUE; } /* fall through */
-
Eric Pouech -
eric pouech (＠epo)