This MR improves PDB load time by: - no longer loading every symbol at once from a compiland, but rather on demand - controlling compiland enumeration from PDB side - not using stale DBI entries generated by MSVC linker in incremental mode. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10087
From: Eric Pouech <epouech@codeweavers.com> Meaning that: - we no longer load all symbols from a compiland when it's first accessed, - but rather build a shadow map of the symbols that could be loaded, - and actually create the sym_function when that symbol is accessed. Signed-off-by: Eric Pouech <epouech@codeweavers.com> --- dlls/dbghelp/pdb.c | 347 ++++++++++++++++++++++++++++++++------------- 1 file changed, 246 insertions(+), 101 deletions(-) diff --git a/dlls/dbghelp/pdb.c b/dlls/dbghelp/pdb.c index 3cee47248d5..ec19306df9f 100644 --- a/dlls/dbghelp/pdb.c +++ b/dlls/dbghelp/pdb.c @@ -112,11 +112,20 @@ struct pdb_dbi_hash_bucket struct pdb_compiland { pdbsize_t stream_offset; /* in DBI stream for compiland description */ - unsigned short are_symbols_loaded; unsigned short compiland_stream_id; pdbsize_t compiland_symbols_size; pdbsize_t compiland_linetab2_offset; pdbsize_t compiland_linetab2_size; /* in compiland stream id (starting at compiland_linetab2_offset) */ + /* following fields are for transitionning from symt to symref */ + unsigned short is_shadow_table_loaded; + unsigned num_shadow_entries; + struct pdb_compiland_shadow_entry + { + struct symt *symt; + unsigned stream_offset; + unsigned symbol_size; + unsigned rva; + } *shadow_entries; struct symt_compiland* compiland; }; @@ -258,6 +267,13 @@ static enum pdb_result pdb_reader_get_segment_address(struct pdb_reader *pdb, un return R_PDB_SUCCESS; } +static enum pdb_result pdb_reader_get_rva_from_segment_offset(struct pdb_reader *pdb, unsigned segment, unsigned offset, unsigned *rva) +{ + if (!segment || segment > pdb->num_sections) return R_PDB_INVALID_PDB_FILE; + *rva = pdb->sections[segment - 1].VirtualAddress + offset; + return R_PDB_SUCCESS; +} + static enum pdb_result pdb_reader_get_segment_offset_from_address(struct pdb_reader *pdb, DWORD64 address, unsigned *segment, unsigned *offset) { unsigned i; @@ -895,7 +911,8 @@ static enum pdb_result pdb_reader_contrib_range_cmp(unsigned idx, int *cmp, void return R_PDB_SUCCESS; } -static enum pdb_result pdb_reader_lookup_compiland_by_segment_offset(struct pdb_reader *pdb, unsigned segment, unsigned offset, unsigned *compiland) +static enum pdb_result pdb_reader_lookup_compiland_by_segment_offset(struct pdb_reader *pdb, unsigned segment, unsigned offset, + unsigned *compiland) { enum pdb_result result; struct pdb_compiland_lookup lookup = {.pdb = pdb, .segment = segment, .offset = offset}; @@ -1985,8 +2002,9 @@ static enum pdb_result pdb_reader_init_DBI(struct pdb_reader *pdb) for (i = 0; i < pdb->num_compilands; i++) { pdb->compilands[i].stream_offset = compiland_iter.dbi_walker.offset; - pdb->compilands[i].compiland = NULL; pdb->compilands[i].compiland_stream_id = compiland_iter.dbi_cu_header.stream; + pdb->compilands[i].is_shadow_table_loaded = pdb->compilands[i].compiland_stream_id == 0xffff; + pdb->compilands[i].compiland_symbols_size = compiland_iter.dbi_cu_header.symbol_size; /* in today's PDB, lineno_size is zero and we wouldn't need to store it... * but just in case... @@ -1994,7 +2012,9 @@ static enum pdb_result pdb_reader_init_DBI(struct pdb_reader *pdb) pdb->compilands[i].compiland_linetab2_offset = compiland_iter.dbi_cu_header.symbol_size + compiland_iter.dbi_cu_header.lineno_size; pdb->compilands[i].compiland_linetab2_size = compiland_iter.dbi_cu_header.lineno2_size; - pdb->compilands[i].are_symbols_loaded = pdb->compilands[i].compiland_stream_id == 0xffff; + pdb->compilands[i].num_shadow_entries = 0; + pdb->compilands[i].shadow_entries = NULL; + pdb->compilands[i].compiland = NULL; result = pdb_reader_compiland_iterator_next(pdb, &compiland_iter); if ((result == R_PDB_SUCCESS) != (i + 1 < pdb->num_compilands)) return result ? result : R_PDB_INVALID_PDB_FILE; } @@ -4942,105 +4962,101 @@ failure: return result; } -static enum pdb_result pdb_reader_load_compiland_symbols(struct pdb_reader *pdb, struct symt_compiland *compiland, struct pdb_reader_walker *walker) +static enum pdb_result pdb_reader_push_compiland_shadow_entry(struct pdb_reader *pdb, struct pdb_compiland *compiland, + unsigned rva, unsigned size, pdbsize_t stream_offset) { enum pdb_result result; - union codeview_symbol *cv_symbol; - struct location loc; - DWORD64 address; - symref_t type_symref; + if (!(compiland->num_shadow_entries & (compiland->num_shadow_entries - 1))) + { + unsigned n = compiland->num_shadow_entries * 2; + if (!n) n = 16; + if ((result = pdb_reader_realloc(pdb, (void **)&compiland->shadow_entries, n * sizeof(compiland->shadow_entries[0])))) return result; + } + compiland->shadow_entries[compiland->num_shadow_entries].stream_offset = stream_offset; + compiland->shadow_entries[compiland->num_shadow_entries].symbol_size = size; + compiland->shadow_entries[compiland->num_shadow_entries].rva = rva; + compiland->shadow_entries[compiland->num_shadow_entries].symt = NULL; + compiland->num_shadow_entries++; + return R_PDB_SUCCESS; +} + +static enum pdb_result pdb_reader_build_compiland_shadow_table(struct pdb_reader *pdb, struct pdb_compiland *compiland) +{ + enum pdb_result result; + struct pdb_reader_walker walker; + union codeview_symbol cv_symbol; + unsigned rva; + + if ((result = pdb_reader_walker_init(pdb, compiland->compiland_stream_id, &walker))) return result; + if ((result = pdb_reader_walker_narrow(&walker, 0, compiland->compiland_symbols_size))) return result; /* skip first DWORD, always 4 AFAICT */ - walker->offset += sizeof(UINT32); + walker.offset += sizeof(UINT32); - /* - * Loop over the different types of records and whenever we - * find something we are interested in, record it and move on. - */ - while ((result = pdb_reader_alloc_and_read_full_codeview_symbol(pdb, walker, &cv_symbol)) == R_PDB_SUCCESS) + while ((result = pdb_reader_read_partial_codeview_symbol(pdb, &walker, &cv_symbol)) == R_PDB_SUCCESS) { - pdbsize_t symbol_start_offset = walker->offset - (sizeof(cv_symbol->generic.len) + cv_symbol->generic.len); - if (!cv_symbol->generic.id || cv_symbol->generic.len < 2) break; - if ((cv_symbol->generic.len + 2) & 3) WARN("unpadded len %u\n", cv_symbol->generic.len + 2); + pdbsize_t symbol_start_offset = walker.offset - sizeof(cv_symbol.generic.len); + if (!cv_symbol.generic.id || cv_symbol.generic.len < 2) break; + if ((cv_symbol.generic.len + 2) & 3) WARN("unpadded len %u\n", cv_symbol.generic.len + 2); - switch (cv_symbol->generic.id) + result = R_PDB_SUCCESS; + switch (cv_symbol.generic.id) { case S_THUNK32: - if ((result = pdb_reader_get_segment_address(pdb, cv_symbol->thunk_v3.segment, cv_symbol->thunk_v3.offset, &address))) goto failure; - symt_new_thunk(pdb->module, compiland, - cv_symbol->thunk_v3.name, cv_symbol->thunk_v3.thtype, - address, cv_symbol->thunk_v3.thunk_len); - /* FIXME: we've seen S_FRAMEPROC inside S_THUNK... - * so until it's better supported, skip until end of thunk declaration - */ - if ((result = pdb_reader_symbol_skip_if(pdb, walker, cv_symbol->thunk_v3.pend, S_END))) goto failure; - break; + if (!(result = pdb_reader_get_rva_from_segment_offset(pdb, cv_symbol.thunk_v3.segment, cv_symbol.thunk_v3.offset, &rva))) + result = pdb_reader_push_compiland_shadow_entry(pdb, compiland, rva, cv_symbol.thunk_v3.thunk_len, symbol_start_offset); + result = pdb_reader_symbol_skip_if(pdb, &walker, cv_symbol.thunk_v3.pend, S_END); + continue; /* * Global and static functions. */ case S_GPROC32: case S_LPROC32: + if (!(result = pdb_reader_get_rva_from_segment_offset(pdb, cv_symbol.proc_v3.segment, cv_symbol.proc_v3.offset, &rva))) { - struct symt_function *func; - - if ((result = pdb_reader_get_segment_address(pdb, cv_symbol->proc_v3.segment, cv_symbol->proc_v3.offset, &address))) goto failure; - if ((result = pdb_reader_symref_from_cv_typeid(pdb, cv_symbol->proc_v3.proctype, &type_symref))) goto failure; - if ((func = symt_new_function(pdb->module, symt_ptr_to_symref(&compiland->symt), - cv_symbol->proc_v3.name, - address, cv_symbol->proc_v3.proc_len, - type_symref, symbol_start_offset))) - { - loc.kind = loc_absolute; - loc.offset = cv_symbol->proc_v3.debug_start; - symt_add_function_point(pdb->module, func, SymTagFuncDebugStart, &loc, NULL); - loc.offset = cv_symbol->proc_v3.debug_end; - symt_add_function_point(pdb->module, func, SymTagFuncDebugEnd, &loc, NULL); - - if ((result = pdb_reader_load_compiland_function(pdb, compiland, func, func, walker))) goto failure; - } + result = pdb_reader_push_compiland_shadow_entry(pdb, compiland, rva, cv_symbol.proc_v3.proc_len, symbol_start_offset); } + result = pdb_reader_symbol_skip_if(pdb, &walker, cv_symbol.proc_v3.pend, S_END); + continue; break; case S_COMPILE2: - TRACE("S-Compile-V3 machine:%x language:%x %s\n", - cv_symbol->compile2_v3.machine, cv_symbol->compile2_v3.flags.iLanguage, debugstr_a(cv_symbol->compile2_v3.name)); + /* FIXME not tracing name - out of partial symbol */ + TRACE("S-Compile-V3 machine:%x language:%x\n", + cv_symbol.compile2_v3.machine, cv_symbol.compile2_v3.flags.iLanguage); break; case S_COMPILE3: - TRACE("S-Compile3-V3 machine:%x language:%x %s\n", - cv_symbol->compile3_v3.machine, cv_symbol->compile3_v3.flags.iLanguage, debugstr_a(cv_symbol->compile3_v3.name)); + /* FIXME not tracing name - out of partial symbol */ + TRACE("S-Compile3-V3 machine:%x language:%x\n", + cv_symbol.compile3_v3.machine, cv_symbol.compile3_v3.flags.iLanguage); break; case S_ENVBLOCK: break; case S_OBJNAME: - TRACE("S-ObjName-V3 %s\n", debugstr_a(cv_symbol->objname_v3.name)); + /* FIXME not tracing name - out of partial symbol */ + TRACE("S-ObjName-V3\n"); break; case S_LABEL32: - if ((result = pdb_reader_get_segment_address(pdb, cv_symbol->label_v3.segment, cv_symbol->label_v3.offset, &address))) goto failure; - symt_new_label(pdb->module, compiland, cv_symbol->label_v3.name, address); + if (!(result = pdb_reader_get_rva_from_segment_offset(pdb, cv_symbol.label_v3.segment, cv_symbol.label_v3.offset, &rva))) + result = pdb_reader_push_compiland_shadow_entry(pdb, compiland, rva, 1, symbol_start_offset); break; case S_SSEARCH: TRACE("Start search: seg=0x%x at offset 0x%08x\n", - cv_symbol->ssearch_v1.segment, cv_symbol->ssearch_v1.offset); + cv_symbol.ssearch_v1.segment, cv_symbol.ssearch_v1.offset); break; case S_ALIGN: TRACE("S-Align V1\n"); break; - case S_HEAPALLOCSITE: /* FIXME likely function only */ - TRACE("S-heap site V3: offset=0x%08x at sect_idx 0x%04x, inst_len 0x%08x, index 0x%08x\n", - cv_symbol->heap_alloc_site_v3.offset, cv_symbol->heap_alloc_site_v3.sect_idx, - cv_symbol->heap_alloc_site_v3.inst_len, cv_symbol->heap_alloc_site_v3.index); - break; - case S_GMANPROC: case S_LMANPROC: - if ((result = pdb_reader_symbol_skip_if(pdb, walker, cv_symbol->managed_proc_v3.pend, S_END))) goto failure; + result = pdb_reader_symbol_skip_if(pdb, &walker, cv_symbol.managed_proc_v3.pend, S_END); break; /* the symbols we can safely ignore for now */ @@ -5071,7 +5087,7 @@ static enum pdb_result pdb_reader_load_compiland_symbols(struct pdb_reader *pdb, case S_UNAMESPACE: case S_INLINEES: case S_POGODATA: - TRACE("Unsupported symbol id %x\n", cv_symbol->generic.id); + TRACE("Unsupported symbol id %x\n", cv_symbol.generic.id); break; /* Symbols only expected inside a function, thunk... */ @@ -5086,6 +5102,7 @@ static enum pdb_result pdb_reader_load_compiland_symbols(struct pdb_reader *pdb, case S_INLINESITE_END: case S_FRAMEPROC: case S_SEPCODE: + case S_HEAPALLOCSITE: /* symbols only expected in compiland symbols' stream */ case S_PUB32: case S_PROCREF: @@ -5094,25 +5111,27 @@ static enum pdb_result pdb_reader_load_compiland_symbols(struct pdb_reader *pdb, case S_GDATA32: case S_UDT: default: - PDB_REPORT_UNEXPECTED("(compiland stream - top level) symbol id", cv_symbol->generic.id); + PDB_REPORT_UNEXPECTED("(compiland stream - top level) symbol id", cv_symbol.generic.id); break; } - pdb_reader_free(pdb, cv_symbol); + if (result) return result; + walker.offset += cv_symbol.generic.len; } return R_PDB_SUCCESS; +} -failure: - pdb_reader_free(pdb, cv_symbol); - return result; +static int compiland_shadow_entry_stream_offset_cmp(const void *key, const void *entry) +{ + if (*(unsigned*)key < ((struct pdb_compiland_shadow_entry *)entry)->stream_offset) return -1; + if (*(unsigned*)key > ((struct pdb_compiland_shadow_entry *)entry)->stream_offset) return +1; + return 0; } -static enum pdb_result pdb_reader_ensure_symbols_loaded_from_compiland(struct pdb_reader *pdb, unsigned compiland_index) +static enum pdb_result pdb_reader_ensure_compiland_shadow_table(struct pdb_reader *pdb, struct pdb_compiland *compiland) { enum pdb_result result; - struct pdb_compiland *compiland = &pdb->compilands[compiland_index]; - if (compiland_index >= pdb->num_compilands) return R_PDB_INVALID_ARGUMENT; - if (!compiland->are_symbols_loaded) + if (!compiland->is_shadow_table_loaded) { struct pdb_reader_walker walker; struct symref_code code; @@ -5129,33 +5148,173 @@ static enum pdb_result pdb_reader_ensure_symbols_loaded_from_compiland(struct pd if ((result = pdb_reader_walker_init(pdb, compiland->compiland_stream_id, &walker))) return result; if ((result = pdb_reader_walker_narrow(&walker, 0, compiland->compiland_symbols_size))) return result; - if ((result = pdb_reader_load_compiland_symbols(pdb, (struct symt_compiland *)compiland->compiland, &walker))) return result; - compiland->are_symbols_loaded = TRUE; + if ((result = pdb_reader_build_compiland_shadow_table(pdb, compiland))) return result; + compiland->is_shadow_table_loaded = TRUE; } return R_PDB_SUCCESS; } +static enum pdb_result pdb_reader_compiland_lookup_shadow_entry(struct pdb_reader *pdb, struct pdb_compiland *compiland, + pdbsize_t stream_offset, struct pdb_compiland_shadow_entry **entry) +{ + enum pdb_result result; + struct pdb_compiland_shadow_entry *found; + + if ((result = pdb_reader_ensure_compiland_shadow_table(pdb, compiland))) return result; + + found = bsearch(&stream_offset, compiland->shadow_entries, compiland->num_shadow_entries, sizeof(compiland->shadow_entries[0]), + compiland_shadow_entry_stream_offset_cmp); + if (!found || found->stream_offset != stream_offset) return R_PDB_NOT_FOUND; + *entry = found; + return R_PDB_SUCCESS; +} + +static enum pdb_result pdb_reader_ensure_compiland_symbol_present(struct pdb_reader *pdb, unsigned compiland_index, pdbsize_t stream_offset, symref_t *symref) +{ + enum pdb_result result; + struct pdb_compiland *compiland; + struct pdb_compiland_shadow_entry *found; + struct pdb_reader_walker walker; + union codeview_symbol *cv_symbol; + struct location loc; + DWORD64 address; + symref_t type_symref; + + if (compiland_index > pdb->num_compilands) return R_PDB_INVALID_PDB_FILE; + compiland = &pdb->compilands[compiland_index]; + + if ((result = pdb_reader_compiland_lookup_shadow_entry(pdb, compiland, stream_offset, &found))) return result; + if (found->symt) + { + *symref = symt_ptr_to_symref(found->symt); + return R_PDB_SUCCESS; + } + + if ((result = pdb_reader_walker_init(pdb, compiland->compiland_stream_id, &walker))) return result; + /* skip first DWORD, always 4 AFAICT */ + walker.offset = stream_offset; + + /* Note: we should narrow walker from info in cv_symbol entry (=> offset to S_END) */ + if ((result = pdb_reader_alloc_and_read_full_codeview_symbol(pdb, &walker, &cv_symbol)) == R_PDB_SUCCESS) + { + switch (cv_symbol->generic.id) + { + case S_THUNK32: + if (!(result = pdb_reader_get_segment_address(pdb, cv_symbol->thunk_v3.segment, cv_symbol->thunk_v3.offset, &address))) + found->symt = &symt_new_thunk(pdb->module, compiland->compiland, + cv_symbol->thunk_v3.name, cv_symbol->thunk_v3.thtype, + address, cv_symbol->thunk_v3.thunk_len)->symt; + break; + + /* + * Global and static functions. + */ + case S_GPROC32: + case S_LPROC32: + { + struct symt_function *func = NULL; + if (!(result = pdb_reader_get_segment_address(pdb, cv_symbol->proc_v3.segment, cv_symbol->proc_v3.offset, &address)) && + !(result = pdb_reader_symref_from_cv_typeid(pdb, cv_symbol->proc_v3.proctype, &type_symref)) && + (func = symt_new_function(pdb->module, symt_ptr_to_symref(&compiland->compiland->symt), + cv_symbol->proc_v3.name, + address, cv_symbol->proc_v3.proc_len, + type_symref, stream_offset))) + { + loc.kind = loc_absolute; + loc.offset = cv_symbol->proc_v3.debug_start; + symt_add_function_point(pdb->module, func, SymTagFuncDebugStart, &loc, NULL); + loc.offset = cv_symbol->proc_v3.debug_end; + symt_add_function_point(pdb->module, func, SymTagFuncDebugEnd, &loc, NULL); + + result = pdb_reader_load_compiland_function(pdb, compiland->compiland, func, func, &walker); + } + if (result == R_PDB_SUCCESS) + { + if (func) found->symt = &func->symt; + else result = R_PDB_OUT_OF_MEMORY; + } + } + break; + + case S_LABEL32: + if (!(result = pdb_reader_get_segment_address(pdb, cv_symbol->label_v3.segment, cv_symbol->label_v3.offset, &address))) + found->symt = &symt_new_label(pdb->module, compiland->compiland, cv_symbol->label_v3.name, address)->symt; + break; + + default: + /* list of handled symbols should match the ones which push entries in pdb_reader_build_compiland_shadow_table */ + break; + } + pdb_reader_free(pdb, cv_symbol); + } + if (result == R_PDB_SUCCESS) + *symref = symt_ptr_to_symref(found->symt); + return result; +} + static enum pdb_result pdb_reader_lookup_top_symbol_by_segment_offset(struct pdb_reader *pdb, unsigned segment, unsigned offset, symref_t *symref) { enum pdb_result result; unsigned compiland_index; - DWORD64 in_address; - struct symt_ht *symbol; + struct pdb_global key, *found; - if ((result = pdb_reader_get_segment_address(pdb, segment, offset, &in_address))) return result; - result = pdb_reader_lookup_compiland_by_segment_offset(pdb, segment, offset, &compiland_index); - if (result == R_PDB_SUCCESS) + /* FIXME: that's a bit complex today + * - we first search globals from DBI (but procedures are not there yet), + * - if not found, lookup for symbol in its compiland (by contrib) + * - in last resort, search in old symt_ht table (eg the function static variable are here) + * When all symbols are migrated to symref, the latest point will not be needed + */ + if ((result = pdb_reader_get_rva_from_segment_offset(pdb, segment, offset, &key.rva))) return result; + found = bsearch(&key, pdb->globals, pdb->num_globals, sizeof(*pdb->globals), &pdb_global_cmp); + if (found && found->rva == key.rva) + { + /* Note: we can have several names for the same address. + * For now, we return one of the entries, no clear way of choosing one or another + */ + if (found > pdb->globals && (found - 1)->rva == key.rva) + WARN("Duplicate found before\n"); + if (found + 1 < pdb->globals + pdb->num_globals && (found + 1)->rva == key.rva) + WARN("Duplicate found after\n"); + *symref = found->symref; + return R_PDB_SUCCESS; + } + if (!(result = pdb_reader_lookup_compiland_by_segment_offset(pdb, segment, offset, &compiland_index))) { - if ((result = pdb_reader_ensure_symbols_loaded_from_compiland(pdb, compiland_index))) + struct pdb_compiland *compiland = &pdb->compilands[compiland_index]; + struct pdb_compiland_shadow_entry *best = NULL; + unsigned i; + + /* FIXME the entries are stored in ascending order wrt. stream_offset + * we use a linear search for now... + */ + if ((result = pdb_reader_ensure_compiland_shadow_table(pdb, compiland))) return result; + for (i = 0; i < compiland->num_shadow_entries; i++) + { + if (compiland->shadow_entries[i].rva <= key.rva && + key.rva < compiland->shadow_entries[i].rva + compiland->shadow_entries[i].symbol_size) + { + best = &compiland->shadow_entries[i]; + break; + } + } + if (best) + { + result = pdb_reader_ensure_compiland_symbol_present(pdb, compiland_index, best->stream_offset, symref); return result; + } } - /* don't fail if not found as some symbols are only present in DBI, but not in compiland */ - else if (result != R_PDB_NOT_FOUND) return result; - /* fallback to ptr symbols lookup (as ptr should be loaded by now) */ - symbol = symt_find_symbol_at(pdb->module, in_address); - if (!symbol) return R_PDB_NOT_FOUND; - *symref = symt_ptr_to_symref(&symbol->symt); - return R_PDB_SUCCESS; + /* we can end up here when eg looking up for static variables inside a function... + * FIXME: this will only work if the symt for the function has been loaded + */ + { + struct symt_ht *symt = symt_find_symbol_at(pdb->module, pdb->module->module.BaseOfImage + key.rva); + if (symt) + { + *symref = symt_ptr_to_symref(&symt->symt); + result = R_PDB_SUCCESS; + } + } + return R_PDB_NOT_FOUND; } static enum method_result pdb_method_lookup_symbol_by_address(struct module_format *modfmt, DWORD_PTR address, symref_t *symref) @@ -5163,7 +5322,6 @@ static enum method_result pdb_method_lookup_symbol_by_address(struct module_form enum pdb_result result; struct pdb_reader *pdb; unsigned segment, offset; - struct pdb_global key, *found; pdb = pdb_get_current_reader(modfmt); if ((result = pdb_reader_get_segment_offset_from_address(pdb, address, &segment, &offset))) @@ -5173,21 +5331,8 @@ static enum method_result pdb_method_lookup_symbol_by_address(struct module_form return MR_SUCCESS; return MR_FAILURE; } - key.rva = address - pdb->module->module.BaseOfImage; - found = bsearch(&key, pdb->globals, pdb->num_globals, sizeof(*pdb->globals), &pdb_global_cmp); - if (found && found->rva == key.rva) - { - /* Note: we can have several names for the same address. - * For now, we return one of the entries, no clear way of choosing one or another - */ - if (found > pdb->globals && (found - 1)->rva == key.rva) - WARN("Duplicate found before\n"); - if (found + 1 < pdb->globals + pdb->num_globals && (found + 1)->rva == key.rva) - WARN("Duplicate found after\n"); - *symref = found->symref; - return MR_SUCCESS; - } - return pdb_method_result(pdb_reader_lookup_top_symbol_by_segment_offset(pdb, segment, offset, symref)); + result = pdb_reader_lookup_top_symbol_by_segment_offset(pdb, segment, offset, symref); + return pdb_method_result(result); } static enum pdb_result pdb_reader_dereference_procedure(struct pdb_reader *pdb, unsigned compiland_id, pdbsize_t stream_offset, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10087
From: Eric Pouech <epouech@codeweavers.com> Signed-off-by: Eric Pouech <epouech@codeweavers.com> --- dlls/dbghelp/pdb.c | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/dlls/dbghelp/pdb.c b/dlls/dbghelp/pdb.c index ec19306df9f..0870244265d 100644 --- a/dlls/dbghelp/pdb.c +++ b/dlls/dbghelp/pdb.c @@ -509,6 +509,7 @@ static enum pdb_result pdb_reader_internal_binary_search(size_t num_elt, enum pdb_result (*cmp)(unsigned idx, int *cmp_ressult, void *user), size_t *found, void *user); static enum pdb_result pdb_reader_symref_from_cv_typeid(struct pdb_reader *pdb, cv_typ_t cv_typeid, symref_t *symref); +static enum pdb_result pdb_reader_ensure_compiland_symbol_present(struct pdb_reader *pdb, unsigned compiland_index, pdbsize_t stream_offset, symref_t *symref); static enum pdb_result pdb_reader_init(struct pdb_reader *pdb, struct module *module, HANDLE file, const IMAGE_SECTION_HEADER *sections, unsigned num_sections) @@ -3835,6 +3836,26 @@ static enum pdb_result pdb_reader_top_fill_in(struct pdb_reader *pdb, struct pdb } } } + /* now push compiland specific bits */ + if (!result && compiland) + { + int i; + symref_t symref; + for (i = 0; !result && i < compiland->num_shadow_entries; i++) + { + /* FIXME for now this requires loading all corresponding symt... */ + if (!(result = pdb_reader_ensure_compiland_symbol_present(pdb, compiland - pdb->compilands, + compiland->shadow_entries[i].stream_offset, &symref)) && + ids) + { + if (*count >= last) + result = R_PDB_BUFFER_TOO_SMALL; + if (*count >= first) + ids[*count] = symt_symref_to_index(pdb->module, symref); + } + (*count)++; + } + } (void)pdb_reader_dispose_whole_stream(pdb, &whole); return result; } @@ -5178,11 +5199,14 @@ static enum pdb_result pdb_reader_ensure_compiland_symbol_present(struct pdb_rea union codeview_symbol *cv_symbol; struct location loc; DWORD64 address; - symref_t type_symref; + symref_t type_symref, compiland_symref; + struct symref_code code; if (compiland_index > pdb->num_compilands) return R_PDB_INVALID_PDB_FILE; compiland = &pdb->compilands[compiland_index]; + if ((result = pdb_reader_encode_symref(pdb, symref_code_init_from_compiland(&code, compiland_index), &compiland_symref))) + return result; if ((result = pdb_reader_compiland_lookup_shadow_entry(pdb, compiland, stream_offset, &found))) return result; if (found->symt) { @@ -5215,7 +5239,7 @@ static enum pdb_result pdb_reader_ensure_compiland_symbol_present(struct pdb_rea struct symt_function *func = NULL; if (!(result = pdb_reader_get_segment_address(pdb, cv_symbol->proc_v3.segment, cv_symbol->proc_v3.offset, &address)) && !(result = pdb_reader_symref_from_cv_typeid(pdb, cv_symbol->proc_v3.proctype, &type_symref)) && - (func = symt_new_function(pdb->module, symt_ptr_to_symref(&compiland->compiland->symt), + (func = symt_new_function(pdb->module, compiland_symref, cv_symbol->proc_v3.name, address, cv_symbol->proc_v3.proc_len, type_symref, stream_offset))) @@ -5423,6 +5447,8 @@ static enum method_result pdb_method_enumerate_symbols(struct module_format *mod unsigned segment, offset; char *symbol_name; symref_t symref; + /* default is to keep enumerating with this module's symt... as function static variables are still only stored as symt */ + enum method_result mr = MR_NOT_FOUND; pdb = pdb_get_current_reader(modfmt); @@ -5460,13 +5486,15 @@ static enum method_result pdb_method_enumerate_symbols(struct module_format *mod if (pdb_reader_extract_name_out_of_codeview_symbol(iter.full_cv_symbol, &symbol_name) == R_PDB_SUCCESS) { if (!cb(symref, symbol_name, user)) + { + mr = MR_SUCCESS; break; + } } } pdb_reader_dispose_DBI_hash_iterator(pdb, &iter); - return MR_SUCCESS; } - return MR_FAILURE; + return mr; } static void pdb_module_remove(struct module_format* modfmt) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10087
From: Eric Pouech <epouech@codeweavers.com> This shall prevent to use stale DBI entries from incremental linking. Signed-off-by: Eric Pouech <epouech@codeweavers.com> --- dlls/dbghelp/pdb.c | 95 +++++++++++++++++++++++++++------------------- 1 file changed, 57 insertions(+), 38 deletions(-) diff --git a/dlls/dbghelp/pdb.c b/dlls/dbghelp/pdb.c index 0870244265d..0f8758fa6d8 100644 --- a/dlls/dbghelp/pdb.c +++ b/dlls/dbghelp/pdb.c @@ -1976,16 +1976,27 @@ static int pdb_global_cmp(const void *p1, const void *p2) return 0; } +static int my_action_global_cmp(const void *p1, const void *p2) +{ + pdbsize_t o1 = ((const struct pdb_action_entry *)p1)->stream_offset; + pdbsize_t o2 = ((const struct pdb_action_entry *)p2)->stream_offset; + + if (o1 < o2) return -1; + if (o1 > o2) return +1; + return 0; +} + static enum pdb_result pdb_reader_init_DBI(struct pdb_reader *pdb) { enum pdb_result result; struct pdb_reader_compiland_iterator compiland_iter; + struct pdb_reader_whole_stream whole; struct pdb_reader_walker walker; - union codeview_symbol cv_symbol; + const union codeview_symbol *cv_symbol; struct symref_code code; + unsigned hash, i, rva; symref_t top_symref; symref_t symref; - unsigned i; if ((result = pdb_reader_read_DBI_header(pdb, &pdb->dbi_header, &walker))) return result; if ((result = pdb_reader_read_TPI_header(pdb))) return result; @@ -2026,39 +2037,54 @@ static enum pdb_result pdb_reader_init_DBI(struct pdb_reader *pdb) pdb->num_globals = 0; pdb->globals = NULL; - while (!(result = pdb_reader_read_partial_codeview_symbol(pdb, &walker, &cv_symbol))) + + pdb_reader_alloc_and_load_whole_stream(pdb, pdb->dbi_header.gsym_stream, &whole); + for (hash = 0; hash < DBI_MAX_HASH; hash++) { - DWORD64 addr; - switch (cv_symbol.generic.id) + struct pdb_dbi_hash_bucket *bucket = &pdb->dbi_symbols_hash_buckets[hash]; + for (i = 0; i < bucket->num_entries; i++) { - case S_GDATA32: - case S_LDATA32: - case S_GTHREAD32: - case S_LTHREAD32: - case S_UDT: - if ((result = pdb_reader_push_action(pdb, action_type_globals, walker.offset - sizeof(cv_symbol.generic.len), - cv_symbol.generic.len + sizeof(cv_symbol.generic.len), - top_symref, &symref))) return result; - if (cv_symbol.generic.id == S_GDATA32 || cv_symbol.generic.id == S_LDATA32) + if ((result = pdb_reader_whole_stream_access_codeview_symbol(pdb, &whole, bucket->entries[i].dbi_stream_offset, &cv_symbol))) return result; + switch (cv_symbol->generic.id) { - if (!pdb_reader_get_segment_address(pdb, cv_symbol.data_v3.segment, cv_symbol.data_v3.offset, &addr)) + case S_GDATA32: + case S_LDATA32: + case S_GTHREAD32: + case S_LTHREAD32: + case S_UDT: + if ((result = pdb_reader_push_action(pdb, action_type_globals, bucket->entries[i].dbi_stream_offset, + cv_symbol->generic.len + 2, top_symref, &symref))) return result; + break; + } + } + } + + /* resort action in ascending order wrt. stream_offsets */ + pdb->num_action_globals = pdb->num_action_entries; + qsort(pdb->action_store, pdb->num_action_globals, sizeof(pdb->action_store[0]), &my_action_global_cmp); + + /* now build the global rva map */ + for (int i = 0; i < pdb->num_action_globals; i++) + { + if ((result = pdb_reader_whole_stream_access_codeview_symbol(pdb, &whole, pdb->action_store[i].stream_offset, &cv_symbol))) return result; + if (cv_symbol->generic.id == S_GDATA32 || cv_symbol->generic.id == S_LDATA32) + { + if (!pdb_reader_get_rva_from_segment_offset(pdb, cv_symbol->data_v3.segment, cv_symbol->data_v3.offset, &rva)) + { + if (!(pdb->num_globals & (pdb->num_globals - 1))) { - if (!(pdb->num_globals & (pdb->num_globals - 1))) - { - unsigned sz = pdb->num_globals ? pdb->num_globals * 2 : 1; - if ((result = pdb_reader_realloc(pdb, (void **)&pdb->globals, sz * sizeof(pdb->globals[0])))) - return result; - } - pdb->globals[pdb->num_globals].rva = addr - pdb->module->module.BaseOfImage; - pdb->globals[pdb->num_globals].symref = symref; - pdb->num_globals++; + unsigned sz = pdb->num_globals ? pdb->num_globals * 2 : 1; + if ((result = pdb_reader_realloc(pdb, (void **)&pdb->globals, sz * sizeof(pdb->globals[0])))) + return result; } + pdb->globals[pdb->num_globals].rva = rva; + pdb_reader_encode_symref(pdb, symref_code_init_from_action(&code, i), &pdb->globals[pdb->num_globals].symref); + pdb->num_globals++; } - break; } - walker.offset += cv_symbol.generic.len; } - pdb->num_action_globals = pdb->num_action_entries; + pdb_reader_dispose_whole_stream(pdb, &whole); + /* Note: rva is not unique */ qsort(pdb->globals, pdb->num_globals, sizeof(*pdb->globals), &pdb_global_cmp); if ((result = pdb_reader_init_DBI_substreams(pdb))) return result; @@ -2675,22 +2701,15 @@ static enum pdb_result pdb_reader_read_codeview_type_by_name(struct pdb_reader * return R_PDB_NOT_FOUND; } -static int my_action_global_cmp(const void *p1, const void *p2) -{ - pdbsize_t o1 = *(pdbsize_t*)p1; - pdbsize_t o2 = ((const struct pdb_action_entry *)p2)->stream_offset; - - if (o1 < o2) return -1; - if (o1 > o2) return +1; - return 0; -} - static enum pdb_result pdb_reader_DBI_globals_symref(struct pdb_reader *pdb, unsigned globals_offset, symref_t *symref) { struct symref_code code; struct pdb_action_entry *entry; + struct pdb_action_entry key; + + key.stream_offset = globals_offset; - entry = bsearch(&globals_offset, pdb->action_store, pdb->num_action_globals, sizeof(pdb->action_store[0]), + entry = bsearch(&key, pdb->action_store, pdb->num_action_globals, sizeof(pdb->action_store[0]), &my_action_global_cmp); if (entry) return pdb_reader_encode_symref(pdb, symref_code_init_from_action(&code, entry - pdb->action_store), symref); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10087
From: Eric Pouech <epouech@codeweavers.com> Signed-off-by: Eric Pouech <epouech@codeweavers.com> --- dlls/dbghelp/pdb.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/dlls/dbghelp/pdb.c b/dlls/dbghelp/pdb.c index 0f8758fa6d8..ac7863caded 100644 --- a/dlls/dbghelp/pdb.c +++ b/dlls/dbghelp/pdb.c @@ -5309,17 +5309,22 @@ static enum pdb_result pdb_reader_lookup_top_symbol_by_segment_offset(struct pdb */ if ((result = pdb_reader_get_rva_from_segment_offset(pdb, segment, offset, &key.rva))) return result; found = bsearch(&key, pdb->globals, pdb->num_globals, sizeof(*pdb->globals), &pdb_global_cmp); - if (found && found->rva == key.rva) + if (found && found->rva <= key.rva) { - /* Note: we can have several names for the same address. - * For now, we return one of the entries, no clear way of choosing one or another - */ - if (found > pdb->globals && (found - 1)->rva == key.rva) - WARN("Duplicate found before\n"); - if (found + 1 < pdb->globals + pdb->num_globals && (found + 1)->rva == key.rva) - WARN("Duplicate found after\n"); - *symref = found->symref; - return R_PDB_SUCCESS; + DWORD64 len; + if (found->rva == key.rva || + (!pdb_reader_request_symref_t(pdb, found->symref, TI_GET_LENGTH, &len) && key.rva < found->rva + len)) + { + /* Note: we can have several names for the same address. + * For now, we return one of the entries, no clear way of choosing one or another + */ + if (found > pdb->globals && (found - 1)->rva == key.rva) + WARN("Duplicate found before\n"); + if (found + 1 < pdb->globals + pdb->num_globals && (found + 1)->rva == key.rva) + WARN("Duplicate found after\n"); + *symref = found->symref; + return R_PDB_SUCCESS; + } } if (!(result = pdb_reader_lookup_compiland_by_segment_offset(pdb, segment, offset, &compiland_index))) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10087
participants (2)
-
Eric Pouech -
eric pouech (＠epo)