[PATCH 0/4] MR2664: Several fixes to break and watch command in winedbg.
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- programs/winedbg/break.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/programs/winedbg/break.c b/programs/winedbg/break.c index 6cff37eb886..88c8eef20ab 100644 --- a/programs/winedbg/break.c +++ b/programs/winedbg/break.c @@ -340,8 +340,17 @@ void break_add_break_from_lineno(const char *filename, int lineno, BOOL swbp) if (bkln.addr.Offset) break_add_break(&bkln.addr, TRUE, swbp); else - dbg_printf("Unknown line number\n" - "(either out of file, or no code at given line number)\n"); + { + /* winedbg's lexer can't tell in 'break foo : 3' whether foo shall be interpreted + * as a debuggee symbol or as a debuggee source file. + * Try now symbol since source file failed. + */ + if (filename) + break_add_break_from_id(filename, lineno, swbp); + else + dbg_printf("Unknown line number\n" + "(either out of file, or no code at given line number)\n"); + } } /*********************************************************************** -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2664
From: Eric Pouech <epouech(a)codeweavers.com> Currently, dbghelp returns the source file either: - in DOS format when native module option isn't enabled - as stored in debug info format otherwise This used to work for PE modules inside ELF shared libraries but is broken since evolution to REAL modules. This generates several issues: - winedbg does not always set the native module option when calling dbghelp for source file related functions, leading to heterogenous output to user - some dbghelp function rely on matching source paths, hence leading to errors in winedbg when mixing the two formats for the same source file. Introduce a new Wine only dbghelp option to return the source paths as they are stored inside debug information format, and activate it unconditionaly inside winedbg. This fixes some failure cases of command 'break <NN>' in winedbg. Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/dbghelp/dbghelp.c | 7 +++++++ dlls/dbghelp/dbghelp_private.h | 1 + dlls/dbghelp/symbol.c | 2 +- include/dbghelp.h | 4 ++++ programs/winedbg/winedbg.c | 2 ++ 5 files changed, 15 insertions(+), 1 deletion(-) diff --git a/dlls/dbghelp/dbghelp.c b/dlls/dbghelp/dbghelp.c index 3341bea442e..d3003fc44d2 100644 --- a/dlls/dbghelp/dbghelp.c +++ b/dlls/dbghelp/dbghelp.c @@ -68,6 +68,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(dbghelp); unsigned dbghelp_options = SYMOPT_UNDNAME; BOOL dbghelp_opt_native = FALSE; BOOL dbghelp_opt_real_path = FALSE; +BOOL dbghelp_opt_source_actual_path = FALSE; SYSTEM_INFO sysinfo; static struct process* process_first /* = NULL */; @@ -619,6 +620,10 @@ BOOL WINAPI SymSetExtendedOption(IMAGEHLP_EXTENDED_OPTIONS option, BOOL value) old = dbghelp_opt_real_path; dbghelp_opt_real_path = value; break; + case SYMOPT_EX_WINE_SOURCE_ACTUAL_PATH: + old = dbghelp_opt_source_actual_path; + dbghelp_opt_source_actual_path = value; + break; default: FIXME("Unsupported option %d with value %d\n", option, value); } @@ -638,6 +643,8 @@ BOOL WINAPI SymGetExtendedOption(IMAGEHLP_EXTENDED_OPTIONS option) return dbghelp_opt_native; case SYMOPT_EX_WINE_MODULE_REAL_PATH: return dbghelp_opt_real_path; + case SYMOPT_EX_WINE_SOURCE_ACTUAL_PATH: + return dbghelp_opt_source_actual_path; default: FIXME("Unsupported option %d\n", option); } diff --git a/dlls/dbghelp/dbghelp_private.h b/dlls/dbghelp/dbghelp_private.h index 9a9773269af..7ba10447771 100644 --- a/dlls/dbghelp/dbghelp_private.h +++ b/dlls/dbghelp/dbghelp_private.h @@ -112,6 +112,7 @@ void* hash_table_iter_up(struct hash_table_iter* hti) DECLSPEC_HIDDEN; extern unsigned dbghelp_options DECLSPEC_HIDDEN; extern BOOL dbghelp_opt_native DECLSPEC_HIDDEN; extern BOOL dbghelp_opt_real_path DECLSPEC_HIDDEN; +extern BOOL dbghelp_opt_source_actual_path DECLSPEC_HIDDEN; extern SYSTEM_INFO sysinfo DECLSPEC_HIDDEN; /* FIXME: this could be optimized later on by using relative offsets and smaller integral sizes */ diff --git a/dlls/dbghelp/symbol.c b/dlls/dbghelp/symbol.c index e3352761c53..5579d79bf48 100644 --- a/dlls/dbghelp/symbol.c +++ b/dlls/dbghelp/symbol.c @@ -1882,7 +1882,7 @@ static BOOL get_line_from_function(struct module_pair* pair, struct symt_functio if (found_dli) { BOOL ret; - if (dbghelp_opt_native) + if (dbghelp_opt_source_actual_path) { /* Return native file paths when using winedbg */ ret = internal_line_set_nameA(pair->pcs, intl, (char*)source_get(pair->effective, dli->u.source_file), FALSE); diff --git a/include/dbghelp.h b/include/dbghelp.h index fda8fefa35f..84fa7e10595 100644 --- a/include/dbghelp.h +++ b/include/dbghelp.h @@ -1098,8 +1098,12 @@ typedef enum SYMOPT_EX_MAX, #ifdef __WINESRC__ + /* Include ELF/Mach-O modules in module operations */ SYMOPT_EX_WINE_NATIVE_MODULES = 1000, + /* Return the Unix actual path of loaded module */ SYMOPT_EX_WINE_MODULE_REAL_PATH, + /* Return the raw source file path from debug info (not always mapped to DOS) */ + SYMOPT_EX_WINE_SOURCE_ACTUAL_PATH, #endif } IMAGEHLP_EXTENDED_OPTIONS; diff --git a/programs/winedbg/winedbg.c b/programs/winedbg/winedbg.c index 9815ed4a046..e7ba6afa2e4 100644 --- a/programs/winedbg/winedbg.c +++ b/programs/winedbg/winedbg.c @@ -722,6 +722,8 @@ int main(int argc, char** argv) SYMOPT_LOAD_LINES | SYMOPT_DEFERRED_LOADS | SYMOPT_AUTO_PUBLICS | SYMOPT_INCLUDE_32BIT_MODULES); + SymSetExtendedOption(SYMOPT_EX_WINE_SOURCE_ACTUAL_PATH, TRUE); + if (argc && !strcmp(argv[0], "--auto")) { switch (dbg_active_auto(argc, argv)) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2664
From: Eric Pouech <eric.pouech(a)gmail.com> This also correctly initialize some lvalue. Context of the bug has changed, but the underlying issue remained the same. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=39495 Signed-off-by: Eric Pouech <eric.pouech(a)codeweavers.com> --- programs/winedbg/break.c | 39 +------------------------------------ programs/winedbg/dbg.y | 6 ++---- programs/winedbg/debugger.h | 3 +-- 3 files changed, 4 insertions(+), 44 deletions(-) diff --git a/programs/winedbg/break.c b/programs/winedbg/break.c index 88c8eef20ab..c9abb737487 100644 --- a/programs/winedbg/break.c +++ b/programs/winedbg/break.c @@ -395,7 +395,7 @@ void break_check_delayed_bp(void) * * Add a watchpoint. */ -static void break_add_watch(const struct dbg_lvalue* lvalue, BOOL is_write) +void break_add_watch(const struct dbg_lvalue* lvalue, BOOL is_write) { int num; DWORD64 l = 4; @@ -436,43 +436,6 @@ static void break_add_watch(const struct dbg_lvalue* lvalue, BOOL is_write) dbg_printf("\n"); } -/****************************************************************** - * break_add_watch_from_lvalue - * - * Adds a watch point from an address (stored in a lvalue) - */ -void break_add_watch_from_lvalue(const struct dbg_lvalue* lvalue,BOOL is_write) -{ - struct dbg_lvalue lval; - - types_extract_as_address(lvalue, &lval.addr); - lval.type.id = dbg_itype_none; - - break_add_watch(&lval, is_write); -} - -/*********************************************************************** - * break_add_watch_from_id - * - * Add a watchpoint from a symbol name - */ -void break_add_watch_from_id(const char *name, BOOL is_write) -{ - struct dbg_lvalue lvalue; - - switch (symbol_get_lvalue(name, -1, &lvalue, TRUE)) - { - case sglv_found: - break_add_watch(&lvalue, is_write); - break; - case sglv_unknown: - dbg_printf("Unable to add watchpoint\n"); - break; - case sglv_aborted: /* user aborted symbol lookup */ - break; - } -} - /*********************************************************************** * break_delete_xpoint * diff --git a/programs/winedbg/dbg.y b/programs/winedbg/dbg.y index f46e665e6b9..a7336c34206 100644 --- a/programs/winedbg/dbg.y +++ b/programs/winedbg/dbg.y @@ -248,10 +248,8 @@ break_command: ; watch_command: - tWATCH '*' expr_lvalue { break_add_watch_from_lvalue(&$3, TRUE); } - | tWATCH identifier { break_add_watch_from_id($2, TRUE); } - | tRWATCH '*' expr_lvalue { break_add_watch_from_lvalue(&$3, FALSE); } - | tRWATCH identifier { break_add_watch_from_id($2, FALSE); } + tWATCH expr_lvalue { break_add_watch(&$2, TRUE); } + | tRWATCH expr_lvalue { break_add_watch(&$2, FALSE); } ; diff --git a/programs/winedbg/debugger.h b/programs/winedbg/debugger.h index c3a0be02984..234b34aa083 100644 --- a/programs/winedbg/debugger.h +++ b/programs/winedbg/debugger.h @@ -331,8 +331,7 @@ extern BOOL break_add_break(const ADDRESS64* addr, BOOL verbose, BOO extern BOOL break_add_break_from_lvalue(const struct dbg_lvalue* value, BOOL swbp); extern void break_add_break_from_id(const char* name, int lineno, BOOL swbp); extern void break_add_break_from_lineno(const char *filename, int lineno, BOOL swbp); -extern void break_add_watch_from_lvalue(const struct dbg_lvalue* lvalue, BOOL is_write); -extern void break_add_watch_from_id(const char* name, BOOL is_write); +extern void break_add_watch(const struct dbg_lvalue* value, BOOL is_write); extern void break_check_delayed_bp(void); extern void break_delete_xpoint(int num); extern void break_delete_xpoints_from_module(DWORD64 base); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2664
From: Eric Pouech <eric.pouech(a)gmail.com> - support 8 byte targets - default to CPU pointer size if size isn't present nor supported. - detect unaligned requests Signed-off-by: Eric Pouech <eric.pouech(a)codeweavers.com> --- programs/winedbg/break.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/programs/winedbg/break.c b/programs/winedbg/break.c index c9abb737487..8001ba1707b 100644 --- a/programs/winedbg/break.c +++ b/programs/winedbg/break.c @@ -398,7 +398,7 @@ void break_check_delayed_bp(void) void break_add_watch(const struct dbg_lvalue* lvalue, BOOL is_write) { int num; - DWORD64 l = 4; + DWORD l = ADDRSIZE; if (!lvalue->in_debuggee) { @@ -409,21 +409,30 @@ void break_add_watch(const struct dbg_lvalue* lvalue, BOOL is_write) &lvalue->addr); if (num == -1) return; + /* FIXME: this validation part should be moved to CPU backends (likely in a new method) */ if (lvalue->type.id != dbg_itype_none) { - if (types_get_info(&lvalue->type, TI_GET_LENGTH, &l)) + DWORD64 l64; + if (types_get_info(&lvalue->type, TI_GET_LENGTH, &l64)) { - switch (l) + /* Only allow size of a power of 2, smaller than CPU's pointer size. + * (Validation is done by CPU backend with insert_Xpoint method) + */ + if (!(l64 & (l64 - 1)) && l64 <= l) + l = l64; + else + dbg_printf("Unsupported length (%I64x) for watch-points, defaulting to %lu\n", l64, l); + /* most CPUs request watch address to be aligned on length */ + if (lvalue->addr.Offset & (l - 1)) { - case 4: case 2: case 1: break; - default: - dbg_printf("Unsupported length (%I64x) for watch-points, defaulting to 4\n", l); - break; + dbg_printf("Watchpoint on unaligned address is not supported\n"); + dbg_curr_process->bp[num].refcount = 0; + return; } } - else dbg_printf("Cannot get watch size, defaulting to 4\n"); + else dbg_printf("Cannot get watch size, defaulting to %lu\n", l); } - dbg_curr_process->bp[num].w.len = (DWORD)l - 1; + dbg_curr_process->bp[num].w.len = l - 1; if (!get_watched_value(num, &dbg_curr_process->bp[num].w.oldval)) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2664
pipeline failures are in user32:msg, and unrelated to this MR -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2664#note_30319
participants (2)
-
Eric Pouech -
eric pouech (＠epo)