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

  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(-)
  

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
  

On Fri, Jan 10, 2020 at 12:53:17PM +0000, Pedro Alves wrote:
> 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

I have found two different call stacks with this exception. There can be
more.

Call stack 1:
  #0  tui_disassemble (...) at gdb/tui/tui-disasm.c:126
  #1  tui_disasm_window::set_contents (...) at gdb/tui/tui-disasm.c:241
  #2  tui_source_window_base::update_source_window_as_is (...) at
        gdb/tui/tui-winsource.c:184
  #3  tui_source_window_base::update_source_window (...) at
        gdb/tui/tui-winsource.c:173
  #4  tui_update_source_windows_with_addr (...) at
        gdb/tui/tui-winsource.c:207
  #5  tui_set_layout (layout_type=DISASSEM_COMMAND) at
        gdb/tui/tui-layout.c:181
  #6  tui_layout_command (layout_name="asm", from_tty=1) at
        gdb/tui/tui-layout.c:287
  #7  do_const_cfunc (c=, args="asm", from_tty=1) at
        gdb/cli/cli-decode.c:107
  #8  cmd_func (cmd=, args="asm", from_tty=1) at
        gdb/cli/cli-decode.c:1952
  #9  execute_command (p="m", from_tty=1) at gdb/top.c:653
  #10 catch_command_errors (...) at gdb/main.c:401
  #11 captured_main_1 (context=) at gdb/main.c:1168
  #12 captured_main (data=) at gdb/main.c:1193
  #13 gdb_main (args=) at gdb/main.c:1218
  #14 main (argc=4, argv=) at gdb/gdb.c:32

call stack 2:
#0  tui_disassemble (...)
    at gdb/tui/tui-disasm.c:126
#1  tui_find_disassembly_address (...)
    at gdb/tui/tui-disasm.c:157
#2  tui_disasm_window::do_scroll_vertical (this=, num_to_scroll=32)
    at gdb/tui/tui-disasm.c:348
#3  tui_win_info::forward_scroll (this=, num_to_scroll=31)
    at gdb/tui/tui-win.c:476
#4  tui_dispatch_ctrl_char (ch=338) at gdb/tui/tui-io.c:921
#5  tui_getc (fp=<_IO_2_1_stdin_>) at gdb/tui/tui-io.c:1005
#6  rl_read_key () at readline/readline/input.c:495
#7  readline_internal_char () at readline/readline/readline.c:573
#8  rl_callback_read_char () at readline/readline/callback.c:262
#9  gdb_rl_callback_read_char_wrapper_noexcept ()
    at gdb/event-top.c:176
#10 gdb_rl_callback_read_char_wrapper (client_data=)
    at gdb/event-top.c:193
#11 stdin_event_handler (error=0, client_data=) at gdb/event-top.c:515
#12 handle_file_event (file_ptr=, ready_mask=1)
    at gdb/event-loop.c:731
#13 gdb_wait_for_event (block=1) at gdb/event-loop.c:857
#14 gdb_do_one_event () at gdb/event-loop.c:346
#15 start_event_loop () at gdb/event-loop.c:370
#16 captured_command_loop () at gdb/main.c:360
#17 captured_main (data=) at gdb/main.c:1203
#18 gdb_main (args=) at gdb/main.c:1218
#19 main (argc=4, argv=) at gdb/gdb.c:32

> 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.

For "call stack 1", other exceptions end up here:
gdb/main.c:
 catch_command_errors (...)
 {
   try
     {
       ...
     }
   catch (const gdb_exception &e)
     {
       return handle_command_errors (e);
     }
   ...
 }

"call stack 2" is doomed. Probably in do_scroll_vertical () it aborts.

> 
> 
> Thanks,
> Pedro Alves
>