Re: [PATCH v2][PR tui/9765] Fix segfault in asm TUI when reaching end of file

Pedro Alves
2020-01-10T12:53:26Z

On 1/10/20 11:57 AM, Shahab Vahedi wrote:
> From: Shahab Vahedi <shahab@synopsys.com>
>
> In TUI mode, when the assembly layout reaches the end of a binary,
> GDB wants to disassemle the addresses beyond the last valid ones.
> This results in a "MEMORY_ERROR" exception to be thrown when
> tui_disasm_window::set_contents() invokes tui_disassemble(). When
> that happens set_contents() bails out prematurely without filling
> the "content" for the valid addresses. This eventually leads to
> no assembly lines or termination of GDB when you scroll down to
> the last lines of the program.
>
> With this change, tui_disassemble() catches MEMORY_ERROR exceptions
> and ignores them, while filling the rest of "asm_lines" with the
> same address (the one just beyond the last PC address).
>
> The issue has been discussed at length in bug 25345 (and 9765).
>
> gdb/ChangeLog:
> 2020-01-10 Shahab Vahedi <shahab@synopsys.com>
>
> PR tui/25345
> * tui/tui-disasm.c (tui_disasm_window::tui_disassemble):
> Handle MEMORY_ERROR exceptions gracefully.
> ---
> The behavior of GDB after this fix is illustrated here:
> https://sourceware.org/bugzilla/attachment.cgi?id=12178
>
> gdb/tui/tui-disasm.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
> index 98c691f3387..dffcd257a0d 100644
> --- a/gdb/tui/tui-disasm.c
> +++ b/gdb/tui/tui-disasm.c
> @@ -114,7 +114,19 @@ tui_disassemble (struct gdbarch *gdbarch,
> asm_lines[pos + i].addr_size = new_size;
> }
>
> - pc = pc + gdb_print_insn (gdbarch, pc, &gdb_dis_out, NULL);
> + try
> + {
> + pc = pc + gdb_print_insn (gdbarch, pc, &gdb_dis_out, NULL);
> + }
> + catch (const gdb_exception &except)
> + {
> + /* In cases where max_lines is asking tui_disassemble() to fetch
> + too much, like when PC goes past the valid address range, a
> + MEMORY_ERROR is thrown, but it is alright. */
> + if (except.error != MEMORY_ERROR)
> + throw;
> + /* fall through: let asm_lines still to be filled. */
> + }
>

I didn't delve deep into the patch, but, I should point out one
thing -- as described in the PR, it's a problem to let exceptions
cross ncurses. Any kind of C++ exception. So which ncurses callback/entry
point in gdb were we at? We need to look into it and make sure that
no exceptions are thrown from it back into ncurses. Above, you're rethrowing
non-memory exceptions, which is what made me wonder, since it sounds like
for example a Ctrl-C at some "wrong" time may bring down GDB.
For readline, we ended up with TRY_SJLJ/CATCH_SJLJ.

> asm_lines[pos + i].insn = std::move (gdb_dis_out.string ());
>
>

Thanks,
Pedro Alves