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

[Pedro Alves <palves@redhat.com>]

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