Message ID | 20200110115728.13940-1-shahab.vahedi@gmail.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 116942 invoked by alias); 10 Jan 2020 11:57:52 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 116578 invoked by uid 89); 10 Jan 2020 11:57:52 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=9765, HContent-Transfer-Encoding:8bit X-HELO: mail-lj1-f194.google.com Received: from mail-lj1-f194.google.com (HELO mail-lj1-f194.google.com) (209.85.208.194) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 10 Jan 2020 11:57:51 +0000 Received: by mail-lj1-f194.google.com with SMTP id u1so1855232ljk.7 for <gdb-patches@sourceware.org>; Fri, 10 Jan 2020 03:57:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=AfErpOj+v7hd7U+n3YwpFNuqeVg7mYJUy02A8p1rIFc=; b=e7SORPKWXoUFqLhuFaNvYgWrWZtOaPJ9+QotT1u7Dc9nTTPmWEzwEEYgZynuIhX6Nj RWhE22CJzOeNFb0oI53BYoK0dL9Pj2rCR10A2tlMvsB7Xd372sDKCBaeuNnkpoPNnjjl ipHWF0IsmnlXrs2RJw30BH//tV+nLlFXTjfuRIbEtoC18DRgkBGvL3fO0zMWpr+d+YOI r96utROCNxmpB0sfsRShoh3FJxJxnmLCZNYLw7diaSdpiECilDT18RTfx8ZztGFKOD2C A4dFI2pR9O0nvE/xrCH90zaajoQaoxAGGsIlc1yPOkV8kZmieEV5d5A3c0cnggAIJjte pcqg== Return-Path: <shahab.vahedi@gmail.com> Received: from archie.internal.synopsys.com ([2a03:1b20:6:f011::2d]) by smtp.gmail.com with ESMTPSA id n23sm916672lfa.41.2020.01.10.03.57.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Jan 2020 03:57:47 -0800 (PST) From: Shahab Vahedi <shahab.vahedi@gmail.com> To: gdb-patches@sourceware.org Cc: Shahab Vahedi <shahab@synopsys.com>, Pedro Alves <palves@redhat.com>, Andrew Burgess <andrew.burgess@embecosm.com>, Tom Tromey <tom@tromey.com>, Claudiu Zissulescu <claziss@synopsys.com>, Francois Bedard <fbedard@synopsys.com> Subject: [PATCH v2][PR tui/9765] Fix segfault in asm TUI when reaching end of file Date: Fri, 10 Jan 2020 12:57:28 +0100 Message-Id: <20200110115728.13940-1-shahab.vahedi@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit |
Commit Message
Shahab Vahedi
Jan. 10, 2020, 11:57 a.m. UTC
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(-)
Comments
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 >
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. */ + } asm_lines[pos + i].insn = std::move (gdb_dis_out.string ());