[PATCH 0/5] MR950: Improvements in dbghelp support for global/static variables.
This series includes several improvements in DbgHelp for supporting (global / file static / function static) variables. In more details: - correct distinction and hierarchy attributes for the three kinds (Dwarf and PDB) - better filtering of PDB information + dropping invalid variable definitions from incremental linker (hence preventing their use instead of the valid ones) + keeping more wanted variables in case of collision (by address or by name) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/950
From: Eric Pouech <eric.pouech(a)gmail.com> These should be stored in function's children vector. Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/dbghelp/msc.c | 33 +++++++++++++++++++++------------ dlls/dbghelp/symbol.c | 4 +++- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/dlls/dbghelp/msc.c b/dlls/dbghelp/msc.c index 8714e756462..c1cfc4f4ca1 100644 --- a/dlls/dbghelp/msc.c +++ b/dlls/dbghelp/msc.c @@ -1764,6 +1764,8 @@ static const union codeview_symbol* get_next_sym(const union codeview_symbol* sy static inline void codeview_add_variable(const struct msc_debug_info* msc_dbg, struct symt_compiland* compiland, + struct symt_function* func, + struct symt_block* block, const char* name, unsigned segment, unsigned offset, unsigned symtype, BOOL is_local, BOOL in_tls, BOOL force) @@ -1775,6 +1777,13 @@ static inline void codeview_add_variable(const struct msc_debug_info* msc_dbg, loc.kind = in_tls ? loc_tlsrel : loc_absolute; loc.reg = 0; loc.offset = in_tls ? offset : codeview_get_address(msc_dbg, segment, offset); + if (func) + { + if (!is_local || in_tls) WARN("Unsupported construct\n"); + symt_add_func_local(msc_dbg->module, func, DataIsStaticLocal, &loc, block, + codeview_get_type(symtype, FALSE), name); + return; + } if (force || in_tls || !symt_find_symbol_at(msc_dbg->module, loc.offset)) { symt_new_global_variable(msc_dbg->module, compiland, @@ -2232,21 +2241,21 @@ static BOOL codeview_snarf(const struct msc_debug_info* msc_dbg, case S_GDATA32_16t: case S_LDATA32_16t: if (do_globals) - codeview_add_variable(msc_dbg, compiland, terminate_string(&sym->data_v1.p_name), + codeview_add_variable(msc_dbg, compiland, curr_func, block, terminate_string(&sym->data_v1.p_name), sym->data_v1.segment, sym->data_v1.offset, sym->data_v1.symtype, sym->generic.id == S_LDATA32_16t, FALSE, TRUE); break; case S_GDATA32_ST: case S_LDATA32_ST: if (do_globals) - codeview_add_variable(msc_dbg, compiland, terminate_string(&sym->data_v2.p_name), + codeview_add_variable(msc_dbg, compiland, curr_func, block, terminate_string(&sym->data_v2.p_name), sym->data_v2.segment, sym->data_v2.offset, sym->data_v2.symtype, sym->generic.id == S_LDATA32_ST, FALSE, TRUE); break; case S_GDATA32: case S_LDATA32: if (do_globals) - codeview_add_variable(msc_dbg, compiland, sym->data_v3.name, + codeview_add_variable(msc_dbg, compiland, curr_func, block, sym->data_v3.name, sym->data_v3.segment, sym->data_v3.offset, sym->data_v3.symtype, sym->generic.id == S_LDATA32, FALSE, TRUE); break; @@ -2255,21 +2264,21 @@ static BOOL codeview_snarf(const struct msc_debug_info* msc_dbg, case S_GTHREAD32_16t: case S_LTHREAD32_16t: if (do_globals) - codeview_add_variable(msc_dbg, compiland, terminate_string(&sym->thread_v1.p_name), + codeview_add_variable(msc_dbg, compiland, curr_func, block, terminate_string(&sym->thread_v1.p_name), sym->thread_v1.segment, sym->thread_v1.offset, sym->thread_v1.symtype, sym->generic.id == S_LTHREAD32_16t, TRUE, TRUE); break; case S_GTHREAD32_ST: case S_LTHREAD32_ST: if (do_globals) - codeview_add_variable(msc_dbg, compiland, terminate_string(&sym->thread_v2.p_name), + codeview_add_variable(msc_dbg, compiland, curr_func, block, terminate_string(&sym->thread_v2.p_name), sym->thread_v2.segment, sym->thread_v2.offset, sym->thread_v2.symtype, sym->generic.id == S_LTHREAD32_ST, TRUE, TRUE); break; case S_GTHREAD32: case S_LTHREAD32: if (do_globals) - codeview_add_variable(msc_dbg, compiland, sym->thread_v3.name, + codeview_add_variable(msc_dbg, compiland, curr_func, block, sym->thread_v3.name, sym->thread_v3.segment, sym->thread_v3.offset, sym->thread_v3.symtype, sym->generic.id == S_LTHREAD32, TRUE, TRUE); break; @@ -2881,19 +2890,19 @@ static BOOL codeview_snarf_public(const struct msc_debug_info* msc_dbg, const BY */ case S_GDATA32_16t: case S_LDATA32_16t: - codeview_add_variable(msc_dbg, compiland, terminate_string(&sym->data_v1.p_name), + codeview_add_variable(msc_dbg, compiland, NULL, NULL, terminate_string(&sym->data_v1.p_name), sym->data_v1.segment, sym->data_v1.offset, sym->data_v1.symtype, sym->generic.id == S_LDATA32_16t, FALSE, FALSE); break; case S_GDATA32_ST: case S_LDATA32_ST: - codeview_add_variable(msc_dbg, compiland, terminate_string(&sym->data_v2.p_name), + codeview_add_variable(msc_dbg, compiland, NULL, NULL, terminate_string(&sym->data_v2.p_name), sym->data_v2.segment, sym->data_v2.offset, sym->data_v2.symtype, sym->generic.id == S_LDATA32_ST, FALSE, FALSE); break; case S_GDATA32: case S_LDATA32: - codeview_add_variable(msc_dbg, compiland, sym->data_v3.name, + codeview_add_variable(msc_dbg, compiland, NULL, NULL, sym->data_v3.name, sym->data_v3.segment, sym->data_v3.offset, sym->data_v3.symtype, sym->generic.id == S_LDATA32, FALSE, FALSE); break; @@ -2901,19 +2910,19 @@ static BOOL codeview_snarf_public(const struct msc_debug_info* msc_dbg, const BY /* variables with thread storage */ case S_GTHREAD32_16t: case S_LTHREAD32_16t: - codeview_add_variable(msc_dbg, compiland, terminate_string(&sym->thread_v1.p_name), + codeview_add_variable(msc_dbg, compiland, NULL, NULL, terminate_string(&sym->thread_v1.p_name), sym->thread_v1.segment, sym->thread_v1.offset, sym->thread_v1.symtype, sym->generic.id == S_LTHREAD32_16t, TRUE, FALSE); break; case S_GTHREAD32_ST: case S_LTHREAD32_ST: - codeview_add_variable(msc_dbg, compiland, terminate_string(&sym->thread_v2.p_name), + codeview_add_variable(msc_dbg, compiland, NULL, NULL, terminate_string(&sym->thread_v2.p_name), sym->thread_v2.segment, sym->thread_v2.offset, sym->thread_v2.symtype, sym->generic.id == S_LTHREAD32_ST, TRUE, FALSE); break; case S_GTHREAD32: case S_LTHREAD32: - codeview_add_variable(msc_dbg, compiland, sym->thread_v3.name, + codeview_add_variable(msc_dbg, compiland, NULL, NULL, sym->thread_v3.name, sym->thread_v3.segment, sym->thread_v3.offset, sym->thread_v3.symtype, sym->generic.id == S_LTHREAD32, TRUE, FALSE); break; diff --git a/dlls/dbghelp/symbol.c b/dlls/dbghelp/symbol.c index 7b0324f5013..4a5a971082e 100644 --- a/dlls/dbghelp/symbol.c +++ b/dlls/dbghelp/symbol.c @@ -449,6 +449,7 @@ void symt_add_func_line(struct module* module, struct symt_function* func, * * Adds a new local/parameter to a given function: * In any cases, dt tells whether it's a local variable or a parameter + * or a static variable inside the function. * If regno it's not 0: * - then variable is stored in a register * - otherwise, value is referenced by register + offset @@ -470,7 +471,7 @@ struct symt_data* symt_add_func_local(struct module* module, name, type); assert(symt_check_tag(&func->symt, SymTagFunction) || symt_check_tag(&func->symt, SymTagInlineSite)); - assert(dt == DataIsParam || dt == DataIsLocal); + assert(dt == DataIsParam || dt == DataIsLocal || dt == DataIsStaticLocal); locsym = pool_alloc(&module->pool, sizeof(*locsym)); locsym->symt.tag = SymTagData; @@ -804,6 +805,7 @@ static void symt_fill_sym_info(struct module_pair* pair, break; case DataIsGlobal: case DataIsFileStatic: + case DataIsStaticLocal: switch (data->u.var.kind) { case loc_tlsrel: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/950
From: Eric Pouech <eric.pouech(a)gmail.com> Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/dbghelp/dwarf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dlls/dbghelp/dwarf.c b/dlls/dbghelp/dwarf.c index 5c0f34d4bf5..13931c45d8b 100644 --- a/dlls/dbghelp/dwarf.c +++ b/dlls/dbghelp/dwarf.c @@ -1997,7 +1997,8 @@ static void dwarf2_parse_variable(dwarf2_subprogram_t* subpgm, if (!dwarf2_find_attribute(di, DW_AT_external, &ext)) ext.u.uvalue = 0; loc.offset += subpgm->ctx->module_ctx->load_offset; - symt_new_global_variable(subpgm->ctx->module_ctx->module, subpgm->ctx->compiland, + symt_new_global_variable(subpgm->ctx->module_ctx->module, + ext.u.uvalue ? NULL : subpgm->ctx->compiland, dwarf2_get_cpp_name(di, name.u.string), !ext.u.uvalue, loc, 0, param_type); break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/950
From: Eric Pouech <eric.pouech(a)gmail.com> Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/dbghelp/dwarf.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/dlls/dbghelp/dwarf.c b/dlls/dbghelp/dwarf.c index 13931c45d8b..71163ced733 100644 --- a/dlls/dbghelp/dwarf.c +++ b/dlls/dbghelp/dwarf.c @@ -1993,14 +1993,23 @@ static void dwarf2_parse_variable(dwarf2_subprogram_t* subpgm, break; case loc_absolute: /* it's a global variable */ - /* FIXME: we don't handle its scope yet */ if (!dwarf2_find_attribute(di, DW_AT_external, &ext)) ext.u.uvalue = 0; loc.offset += subpgm->ctx->module_ctx->load_offset; - symt_new_global_variable(subpgm->ctx->module_ctx->module, - ext.u.uvalue ? NULL : subpgm->ctx->compiland, - dwarf2_get_cpp_name(di, name.u.string), !ext.u.uvalue, - loc, 0, param_type); + if (subpgm->top_func) + { + if (ext.u.uvalue) WARN("unexpected global inside a functionn"); + symt_add_func_local(subpgm->ctx->module_ctx->module, subpgm->current_func, + DataIsStaticLocal, &loc, subpgm->current_block, + param_type, dwarf2_get_cpp_name(di, name.u.string)); + } + else + { + symt_new_global_variable(subpgm->ctx->module_ctx->module, + ext.u.uvalue ? NULL : subpgm->ctx->compiland, + dwarf2_get_cpp_name(di, name.u.string), !ext.u.uvalue, + loc, 0, param_type); + } break; default: subpgm->non_computed_variable = TRUE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/950
From: Eric Pouech <eric.pouech(a)gmail.com> Only load records that are listed in global hash file when handling the global symbol stream. Do the same thing for the public symbols. When using MS linker in incremental mode: - old variable definitions are kept in the (DBI) global symbol stream along side the new definition - but only the latest (valid) definition is referenced from the hash table Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/dbghelp/msc.c | 338 ++++++++++++++++++++++++++------------------- 1 file changed, 194 insertions(+), 144 deletions(-) diff --git a/dlls/dbghelp/msc.c b/dlls/dbghelp/msc.c index c1cfc4f4ca1..aa5b0613a10 100644 --- a/dlls/dbghelp/msc.c +++ b/dlls/dbghelp/msc.c @@ -2209,8 +2209,7 @@ static struct symt_inlinesite* codeview_create_inline_site(const struct msc_debu static BOOL codeview_snarf(const struct msc_debug_info* msc_dbg, const BYTE* root, unsigned offset, unsigned size, - const struct cv_module_snarf* cvmod, - BOOL do_globals) + const struct cv_module_snarf* cvmod) { struct symt_function* top_func = NULL; struct symt_function* curr_func = NULL; @@ -2240,47 +2239,41 @@ static BOOL codeview_snarf(const struct msc_debug_info* msc_dbg, */ case S_GDATA32_16t: case S_LDATA32_16t: - if (do_globals) - codeview_add_variable(msc_dbg, compiland, curr_func, block, terminate_string(&sym->data_v1.p_name), - sym->data_v1.segment, sym->data_v1.offset, sym->data_v1.symtype, - sym->generic.id == S_LDATA32_16t, FALSE, TRUE); + codeview_add_variable(msc_dbg, compiland, curr_func, block, terminate_string(&sym->data_v1.p_name), + sym->data_v1.segment, sym->data_v1.offset, sym->data_v1.symtype, + sym->generic.id == S_LDATA32_16t, FALSE, TRUE); break; case S_GDATA32_ST: case S_LDATA32_ST: - if (do_globals) - codeview_add_variable(msc_dbg, compiland, curr_func, block, terminate_string(&sym->data_v2.p_name), - sym->data_v2.segment, sym->data_v2.offset, sym->data_v2.symtype, - sym->generic.id == S_LDATA32_ST, FALSE, TRUE); + codeview_add_variable(msc_dbg, compiland, curr_func, block, terminate_string(&sym->data_v2.p_name), + sym->data_v2.segment, sym->data_v2.offset, sym->data_v2.symtype, + sym->generic.id == S_LDATA32_ST, FALSE, TRUE); break; case S_GDATA32: case S_LDATA32: - if (do_globals) - codeview_add_variable(msc_dbg, compiland, curr_func, block, sym->data_v3.name, - sym->data_v3.segment, sym->data_v3.offset, sym->data_v3.symtype, - sym->generic.id == S_LDATA32, FALSE, TRUE); + codeview_add_variable(msc_dbg, compiland, curr_func, block, sym->data_v3.name, + sym->data_v3.segment, sym->data_v3.offset, sym->data_v3.symtype, + sym->generic.id == S_LDATA32, FALSE, TRUE); break; /* variables with thread storage */ case S_GTHREAD32_16t: case S_LTHREAD32_16t: - if (do_globals) - codeview_add_variable(msc_dbg, compiland, curr_func, block, terminate_string(&sym->thread_v1.p_name), - sym->thread_v1.segment, sym->thread_v1.offset, sym->thread_v1.symtype, - sym->generic.id == S_LTHREAD32_16t, TRUE, TRUE); + codeview_add_variable(msc_dbg, compiland, curr_func, block, terminate_string(&sym->thread_v1.p_name), + sym->thread_v1.segment, sym->thread_v1.offset, sym->thread_v1.symtype, + sym->generic.id == S_LTHREAD32_16t, TRUE, TRUE); break; case S_GTHREAD32_ST: case S_LTHREAD32_ST: - if (do_globals) - codeview_add_variable(msc_dbg, compiland, curr_func, block, terminate_string(&sym->thread_v2.p_name), - sym->thread_v2.segment, sym->thread_v2.offset, sym->thread_v2.symtype, - sym->generic.id == S_LTHREAD32_ST, TRUE, TRUE); + codeview_add_variable(msc_dbg, compiland, curr_func, block, terminate_string(&sym->thread_v2.p_name), + sym->thread_v2.segment, sym->thread_v2.offset, sym->thread_v2.symtype, + sym->generic.id == S_LTHREAD32_ST, TRUE, TRUE); break; case S_GTHREAD32: case S_LTHREAD32: - if (do_globals) - codeview_add_variable(msc_dbg, compiland, curr_func, block, sym->thread_v3.name, - sym->thread_v3.segment, sym->thread_v3.offset, sym->thread_v3.symtype, - sym->generic.id == S_LTHREAD32, TRUE, TRUE); + codeview_add_variable(msc_dbg, compiland, curr_func, block, sym->thread_v3.name, + sym->thread_v3.segment, sym->thread_v3.offset, sym->thread_v3.symtype, + sym->generic.id == S_LTHREAD32, TRUE, TRUE); break; /* Public symbols */ @@ -2823,124 +2816,156 @@ static void pdb_location_compute(struct process* pcs, loc->reg = loc_err_internal; } -static BOOL codeview_snarf_public(const struct msc_debug_info* msc_dbg, const BYTE* root, - int offset, int size) +static void* pdb_read_file(const struct pdb_file_info* pdb_file, DWORD file_nr); +static unsigned pdb_get_file_size(const struct pdb_file_info* pdb_file, DWORD file_nr); +static BOOL codeview_snarf_sym_hashtable(const struct msc_debug_info* msc_dbg, const BYTE* symroot, DWORD symsize, + const BYTE* hashroot, DWORD hashsize, + BOOL (*feed)(const struct msc_debug_info* msc_dbg, const union codeview_symbol*)) { - int i, length; - struct symt_compiland* compiland = NULL; + const DBI_HASH_HEADER* hash_hdr = (const DBI_HASH_HEADER*)hashroot; + unsigned num_hash_records, i; + const DBI_HASH_RECORD* hr; + + if (hashsize < sizeof(DBI_HASH_HEADER) || + hash_hdr->signature != 0xFFFFFFFF || + hash_hdr->version != 0xeffe0000 + 19990810 || + (hash_hdr->size_hash_records % sizeof(DBI_HASH_RECORD)) != 0 || + sizeof(DBI_HASH_HEADER) + hash_hdr->size_hash_records + DBI_BITMAP_HASH_SIZE > hashsize || + (hashsize - (sizeof(DBI_HASH_HEADER) + hash_hdr->size_hash_records + DBI_BITMAP_HASH_SIZE)) % sizeof(unsigned)) + { + FIXME("Incorrect hash structure\n"); + return FALSE; + } - /* - * Loop over the different types of records and whenever we - * find something we are interested in, record it and move on. + hr = (DBI_HASH_RECORD*)(hash_hdr + 1); + num_hash_records = hash_hdr->size_hash_records / sizeof(DBI_HASH_RECORD); + + /* Only iterate over the records listed in the hash table. + * We assume that records present in stream, but not listed in hash table, are + * invalid (and thus not loaded). */ - for (i = offset; i < size; i += length) + for (i = 0; i < num_hash_records; i++) { - const union codeview_symbol* sym = (const union codeview_symbol*)(root + i); - length = sym->generic.len + 2; - if (i + length > size) break; - if (!sym->generic.id || length < 4) break; - if (length & 3) FIXME("unpadded len %u\n", length); - - switch (sym->generic.id) + if (hr[i].offset && hr[i].offset < symsize) { - case S_PUB32_16t: - if (!(dbghelp_options & SYMOPT_NO_PUBLICS)) - { - symt_new_public(msc_dbg->module, compiland, - terminate_string(&sym->public_v1.p_name), - sym->public_v1.pubsymflags == SYMTYPE_FUNCTION, - codeview_get_address(msc_dbg, sym->public_v1.segment, sym->public_v1.offset), 1); - } - break; - case S_PUB32_ST: - if (!(dbghelp_options & SYMOPT_NO_PUBLICS)) - { - symt_new_public(msc_dbg->module, compiland, - terminate_string(&sym->public_v2.p_name), - sym->public_v2.pubsymflags == SYMTYPE_FUNCTION, - codeview_get_address(msc_dbg, sym->public_v2.segment, sym->public_v2.offset), 1); - } - break; + const union codeview_symbol* sym = (const union codeview_symbol*)(symroot + hr[i].offset - 1); + (*feed)(msc_dbg, sym); + } + } + return TRUE; +} - case S_PUB32: - if (!(dbghelp_options & SYMOPT_NO_PUBLICS)) - { - symt_new_public(msc_dbg->module, compiland, - sym->public_v3.name, - sym->public_v3.pubsymflags == SYMTYPE_FUNCTION, - codeview_get_address(msc_dbg, sym->public_v3.segment, sym->public_v3.offset), 1); - } - break; - case S_PROCREF: - case S_LPROCREF: /* using a data_v3 isn't what we'd expect */ -#if 0 - /* FIXME: this is plain wrong (from a simple test) */ - if (!(dbghelp_options & SYMOPT_NO_PUBLICS)) - { - symt_new_public(msc_dbg->module, compiland, - sym->data_v3.name, - codeview_get_address(msc_dbg, sym->data_v3.segment, sym->data_v3.offset), 1); - } -#endif - break; - /* - * Global and local data symbols. We don't associate these - * with any given source file. - */ - case S_GDATA32_16t: - case S_LDATA32_16t: - codeview_add_variable(msc_dbg, compiland, NULL, NULL, terminate_string(&sym->data_v1.p_name), - sym->data_v1.segment, sym->data_v1.offset, sym->data_v1.symtype, - sym->generic.id == S_LDATA32_16t, FALSE, FALSE); - break; - case S_GDATA32_ST: - case S_LDATA32_ST: - codeview_add_variable(msc_dbg, compiland, NULL, NULL, terminate_string(&sym->data_v2.p_name), - sym->data_v2.segment, sym->data_v2.offset, sym->data_v2.symtype, - sym->generic.id == S_LDATA32_ST, FALSE, FALSE); - break; - case S_GDATA32: - case S_LDATA32: - codeview_add_variable(msc_dbg, compiland, NULL, NULL, sym->data_v3.name, - sym->data_v3.segment, sym->data_v3.offset, sym->data_v3.symtype, - sym->generic.id == S_LDATA32, FALSE, FALSE); - break; +static BOOL pdb_global_feed_types(const struct msc_debug_info* msc_dbg, const union codeview_symbol* sym) +{ + struct symt* symt; + switch (sym->generic.id) + { + case S_UDT_16t: + if (sym->udt_v1.type) + { + if ((symt = codeview_get_type(sym->udt_v1.type, FALSE))) + symt_new_typedef(msc_dbg->module, symt, + terminate_string(&sym->udt_v1.p_name)); + else + FIXME("S-Udt %s: couldn't find type 0x%x\n", + terminate_string(&sym->udt_v1.p_name), sym->udt_v1.type); + } + break; + case S_UDT_ST: + if (sym->udt_v2.type) + { + if ((symt = codeview_get_type(sym->udt_v2.type, FALSE))) + symt_new_typedef(msc_dbg->module, symt, + terminate_string(&sym->udt_v2.p_name)); + else + FIXME("S-Udt %s: couldn't find type 0x%x\n", + terminate_string(&sym->udt_v2.p_name), sym->udt_v2.type); + } + break; + case S_UDT: + if (sym->udt_v3.type) + { + if ((symt = codeview_get_type(sym->udt_v3.type, FALSE))) + symt_new_typedef(msc_dbg->module, symt, sym->udt_v3.name); + else + FIXME("S-Udt %s: couldn't find type 0x%x\n", + sym->udt_v3.name, sym->udt_v3.type); + } + break; + default: return FALSE; + } + return TRUE; +} - /* variables with thread storage */ - case S_GTHREAD32_16t: - case S_LTHREAD32_16t: - codeview_add_variable(msc_dbg, compiland, NULL, NULL, terminate_string(&sym->thread_v1.p_name), - sym->thread_v1.segment, sym->thread_v1.offset, sym->thread_v1.symtype, - sym->generic.id == S_LTHREAD32_16t, TRUE, FALSE); - break; - case S_GTHREAD32_ST: - case S_LTHREAD32_ST: - codeview_add_variable(msc_dbg, compiland, NULL, NULL, terminate_string(&sym->thread_v2.p_name), - sym->thread_v2.segment, sym->thread_v2.offset, sym->thread_v2.symtype, - sym->generic.id == S_LTHREAD32_ST, TRUE, FALSE); - break; - case S_GTHREAD32: - case S_LTHREAD32: - codeview_add_variable(msc_dbg, compiland, NULL, NULL, sym->thread_v3.name, - sym->thread_v3.segment, sym->thread_v3.offset, sym->thread_v3.symtype, - sym->generic.id == S_LTHREAD32, TRUE, FALSE); - break; +static BOOL pdb_global_feed_variables(const struct msc_debug_info* msc_dbg, const union codeview_symbol* sym) +{ + /* The only interest here is to add global variables that haven't been seen + * in module (=compilation unit) stream. + * So we don't care about 'local' symbols since we cannot tell their compiland. + */ + switch (sym->generic.id) + { + case S_GDATA32_16t: + codeview_add_variable(msc_dbg, NULL, NULL, NULL, terminate_string(&sym->data_v1.p_name), + sym->data_v1.segment, sym->data_v1.offset, sym->data_v1.symtype, + FALSE, FALSE, FALSE); + break; + case S_GDATA32_ST: + codeview_add_variable(msc_dbg, NULL, NULL, NULL, terminate_string(&sym->data_v2.p_name), + sym->data_v2.segment, sym->data_v2.offset, sym->data_v2.symtype, + FALSE, FALSE, FALSE); + break; + case S_GDATA32: + codeview_add_variable(msc_dbg, NULL, NULL, NULL, sym->data_v3.name, + sym->data_v3.segment, sym->data_v3.offset, sym->data_v3.symtype, + FALSE, FALSE, FALSE); + break; + /* variables with thread storage */ + case S_GTHREAD32_16t: + codeview_add_variable(msc_dbg, NULL, NULL, NULL, terminate_string(&sym->thread_v1.p_name), + sym->thread_v1.segment, sym->thread_v1.offset, sym->thread_v1.symtype, + FALSE, TRUE, FALSE); + break; + case S_GTHREAD32_ST: + codeview_add_variable(msc_dbg, NULL, NULL, NULL, terminate_string(&sym->thread_v2.p_name), + sym->thread_v2.segment, sym->thread_v2.offset, sym->thread_v2.symtype, + FALSE, TRUE, FALSE); + break; + case S_GTHREAD32: + codeview_add_variable(msc_dbg, NULL, NULL, NULL, sym->thread_v3.name, + sym->thread_v3.segment, sym->thread_v3.offset, sym->thread_v3.symtype, + FALSE, TRUE, FALSE); + break; + default: return FALSE; + } + return TRUE; +} - /* - * These are special, in that they are always followed by an - * additional length-prefixed string which is *not* included - * into the symbol length count. We need to skip it. - */ - case S_PROCREF_ST: - case S_DATAREF_ST: - case S_LPROCREF_ST: - length += (((const char*)sym)[length] + 1 + 3) & ~3; - break; - } - msc_dbg->module->sortlist_valid = TRUE; +static BOOL pdb_global_feed_public(const struct msc_debug_info* msc_dbg, const union codeview_symbol* sym) +{ + switch (sym->generic.id) + { + case S_PUB32_16t: + symt_new_public(msc_dbg->module, NULL, + terminate_string(&sym->public_v1.p_name), + sym->public_v1.pubsymflags == SYMTYPE_FUNCTION, + codeview_get_address(msc_dbg, sym->public_v1.segment, sym->public_v1.offset), 1); + break; + case S_PUB32_ST: + symt_new_public(msc_dbg->module, NULL, + terminate_string(&sym->public_v2.p_name), + sym->public_v2.pubsymflags == SYMTYPE_FUNCTION, + codeview_get_address(msc_dbg, sym->public_v2.segment, sym->public_v2.offset), 1); + break; + case S_PUB32: + symt_new_public(msc_dbg->module, NULL, + sym->public_v3.name, + sym->public_v3.pubsymflags == SYMTYPE_FUNCTION, + codeview_get_address(msc_dbg, sym->public_v3.segment, sym->public_v3.offset), 1); + break; + default: return FALSE; } - msc_dbg->module->sortlist_valid = FALSE; return TRUE; } @@ -3681,12 +3706,23 @@ static BOOL pdb_process_internal(const struct process* pcs, ipi_image = pdb_read_file(pdb_file, 4); ipi_ok = pdb_init_type_parse(msc_dbg, pdb_file, &ipi_ctp, ipi_image); - /* Read global symbol table */ + /* Read global types first, so that lookup by name in module (=compilation unit) + * streams' loading can succeed them. + */ globalimage = pdb_read_file(pdb_file, symbols.gsym_file); if (globalimage) { - codeview_snarf(msc_dbg, globalimage, 0, pdb_get_file_size(pdb_file, symbols.gsym_file), - NULL, FALSE); + const BYTE* data; + unsigned global_size = pdb_get_file_size(pdb_file, symbols.gsym_file); + + data = pdb_read_file(pdb_file, symbols.global_hash_file); + if (data) + { + codeview_snarf_sym_hashtable(msc_dbg, globalimage, global_size, + data, pdb_get_file_size(pdb_file, symbols.global_hash_file), + pdb_global_feed_types); + pdb_free((void*)data); + } } /* Read per-module symbols' tables */ @@ -3705,8 +3741,7 @@ static BOOL pdb_process_internal(const struct process* pcs, { struct cv_module_snarf cvmod = {ipi_ok ? &ipi_ctp : NULL, (const void*)(modimage + sfile.symbol_size), sfile.lineno2_size, files_image}; - codeview_snarf(msc_dbg, modimage, sizeof(DWORD), sfile.symbol_size, - &cvmod, TRUE); + codeview_snarf(msc_dbg, modimage, sizeof(DWORD), sfile.symbol_size, &cvmod); if (sfile.lineno_size && sfile.lineno2_size) FIXME("Both line info present... only supporting second\n"); @@ -3722,11 +3757,27 @@ static BOOL pdb_process_internal(const struct process* pcs, file_name += strlen(file_name) + 1; file = (BYTE*)((DWORD_PTR)(file_name + strlen(file_name) + 1 + 3) & ~3); } - /* finish the remaining public and global information */ + /* Load the global variables and constants (if not yet loaded) and public information */ if (globalimage) { - codeview_snarf_public(msc_dbg, globalimage, 0, - pdb_get_file_size(pdb_file, symbols.gsym_file)); + const BYTE* data; + unsigned global_size = pdb_get_file_size(pdb_file, symbols.gsym_file); + + data = pdb_read_file(pdb_file, symbols.global_hash_file); + if (data) + { + codeview_snarf_sym_hashtable(msc_dbg, globalimage, global_size, + data, pdb_get_file_size(pdb_file, symbols.global_hash_file), + pdb_global_feed_variables); + pdb_free((void*)data); + } + if (!(dbghelp_options & SYMOPT_NO_PUBLICS) && (data = pdb_read_file(pdb_file, symbols.public_file))) + { + const DBI_PUBLIC_HEADER* pubhdr = (const DBI_PUBLIC_HEADER*)data; + codeview_snarf_sym_hashtable(msc_dbg, globalimage, pdb_get_file_size(pdb_file, symbols.gsym_file), + (const BYTE*)(pubhdr + 1), pubhdr->hash_size, pdb_global_feed_public); + pdb_free((void*)data); + } pdb_free(globalimage); } HeapFree(GetProcessHeap(), 0, (DWORD*)ipi_ctp.offset); @@ -4195,8 +4246,7 @@ static BOOL codeview_process_info(const struct process* pcs, if (ent->SubSection == sstAlignSym) { - codeview_snarf(msc_dbg, msc_dbg->root + ent->lfo, sizeof(DWORD), ent->cb, - NULL, TRUE); + codeview_snarf(msc_dbg, msc_dbg->root + ent->lfo, sizeof(DWORD), ent->cb, NULL); /* * Check the next and previous entry. If either is a -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/950
From: Eric Pouech <eric.pouech(a)gmail.com> We have dups in global / file static variables definition: - between compiland stream and global (DBI) stream (we need to de-dup these) - still adding variables only present global DBI stream - keeping in mind, we need to keep: + two variables of different names at same address (aliasing) + variables of same name at different addresses (MS linker generate those) Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/dbghelp/msc.c | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/dlls/dbghelp/msc.c b/dlls/dbghelp/msc.c index aa5b0613a10..a5b688bae17 100644 --- a/dlls/dbghelp/msc.c +++ b/dlls/dbghelp/msc.c @@ -1768,7 +1768,7 @@ static inline void codeview_add_variable(const struct msc_debug_info* msc_dbg, struct symt_block* block, const char* name, unsigned segment, unsigned offset, - unsigned symtype, BOOL is_local, BOOL in_tls, BOOL force) + unsigned symtype, BOOL is_local, BOOL in_tls, BOOL dontcheck) { if (name && *name) { @@ -1784,13 +1784,38 @@ static inline void codeview_add_variable(const struct msc_debug_info* msc_dbg, codeview_get_type(symtype, FALSE), name); return; } - if (force || in_tls || !symt_find_symbol_at(msc_dbg->module, loc.offset)) + if (!dontcheck && !in_tls) { - symt_new_global_variable(msc_dbg->module, compiland, - name, is_local, loc, 0, - codeview_get_type(symtype, FALSE)); + /* Check that we don't add twice the same variable */ + struct hash_table_iter hti; + void* ptr; + struct symt_ht* sym; + + hash_table_iter_init(&msc_dbg->module->ht_symbols, &hti, name); + while ((ptr = hash_table_iter_up(&hti))) + { + sym = CONTAINING_RECORD(ptr, struct symt_ht, hash_elt); + if (symt_check_tag(&sym->symt, SymTagData) && !strcmp(sym->hash_elt.name, name)) + { + struct symt_data* symdata = (struct symt_data*)&sym->symt; + if (symdata->kind == (is_local ? DataIsFileStatic : DataIsGlobal) && + symdata->u.var.kind == loc.kind && + symdata->u.var.offset == loc.offset && + symdata->container == &compiland->symt) + { + /* We don't compare types yet... Unfortunately, they are not + * always the same typeid... it'd require full type equivalence + * (eg: we've seen 'int* foo' <> 'int[4] foo') + */ + return; + } + } + } } - } + if (is_local ^ (compiland != NULL)) FIXME("Unsupported construct\n"); + symt_new_global_variable(msc_dbg->module, compiland, name, is_local, loc, 0, + codeview_get_type(symtype, FALSE)); + } } struct cv_local_info -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/950
participants (2)
-
Eric Pouech -
eric pouech (＠epo)