Message ID | ed397bcd-0562-d18d-b7e8-365bbf8e7b37@redhat.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 27462 invoked by alias); 10 Jan 2020 13:37:12 -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 27440 invoked by uid 89); 10 Jan 2020 13:37:09 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=sk:readlin, wrapped, crossing, filed X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (205.139.110.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 10 Jan 2020 13:37:07 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1578663425; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=HHQ5psDPP85xQioxnZRc3GrGAnAKtddO1kjYxNh1Gbc=; b=aRdVwL59ABSTGeBzn1CAmJDqwcQbCqp8Irg/Fer1TW3o3pt9m4twzwzwVs2da+F9pKVJdB DSvQQlAOWsID+s14CdrDgykwNp5vrc0imngAstfvc+M9WgNCotgpxVYTUTTfXX6GQDpxSC HUStxLPZUOiD1Xxx9pvNYE/IjxbhFdk= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-364-b1nIY8YrOySzbHQcXyx-eA-1; Fri, 10 Jan 2020 08:37:02 -0500 Received: by mail-wm1-f71.google.com with SMTP id t4so778607wmf.2 for <gdb-patches@sourceware.org>; Fri, 10 Jan 2020 05:37:02 -0800 (PST) Return-Path: <palves@redhat.com> Received: from ?IPv6:2001:8a0:f913:f700:4c97:6d52:2cea:997b? ([2001:8a0:f913:f700:4c97:6d52:2cea:997b]) by smtp.gmail.com with ESMTPSA id x11sm2230667wmg.46.2020.01.10.05.36.59 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 10 Jan 2020 05:37:00 -0800 (PST) Subject: [PATCH] Don't let TUI exceptions escape to readline (PR tui/9765) To: Shahab Vahedi <shahab.vahedi@gmail.com>, gdb-patches@sourceware.org References: <20200110115728.13940-1-shahab.vahedi@gmail.com> <8f3c2363-6ab8-ce73-0f4b-b0b9efca6815@redhat.com> Cc: Shahab Vahedi <shahab@synopsys.com>, Andrew Burgess <andrew.burgess@embecosm.com>, Tom Tromey <tom@tromey.com>, Claudiu Zissulescu <claziss@synopsys.com>, Francois Bedard <fbedard@synopsys.com> From: Pedro Alves <palves@redhat.com> Message-ID: <ed397bcd-0562-d18d-b7e8-365bbf8e7b37@redhat.com> Date: Fri, 10 Jan 2020 13:36:58 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <8f3c2363-6ab8-ce73-0f4b-b0b9efca6815@redhat.com> X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit |
Commit Message
Pedro Alves
Jan. 10, 2020, 1:36 p.m. UTC
On 1/10/20 12:53 PM, 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 > 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. There's actually a backtrace in the PR. And I can still (*) reproduce it. Here's the current backtrace I get: (top-gdb) bt #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51 #1 0x00007ffff4633d31 in __GI_abort () at abort.c:79 #2 0x00007ffff4c5d2a5 in __gnu_cxx::__verbose_terminate_handler () at ../../../../libstdc++-v3/libsupc++/vterminate.cc:95 #3 0x00007ffff4c5ae96 in __cxxabiv1::__terminate (handler=<optimized out>) at ../../../../libstdc++-v3/libsupc++/eh_terminate.cc:47 #4 0x00007ffff4c59d99 in __cxa_call_terminate (During symbol reading, Child DIE 0x2cee6 and its abstract origin 0x5a17 have different parents. ue_header=ue_header@entry=0x1a011a0) at ../../../../libstdc++-v3/libsupc++/eh_call.cc:54 #5 0x00007ffff4c5a788 in __cxxabiv1::__gxx_personality_v0 (version=<optimized out>, actions=<optimized out>, exception_class=5138137972254386944, ue_header=0x1a011a0, context=<optimized out>) at ../../../../libstdc++-v3/libsupc++/eh_personality.cc:676 #6 0x00007ffff49c3f33 in _Unwind_RaiseException_Phase2 (exc=exc@entry=0x1a011a0, context=context@entry=0x7fffffffcfa0) at ../../../libgcc/unwind.inc:62 #7 0x00007ffff49c475e in _Unwind_Resume (exc=0x1a011a0) at ../../../libgcc/unwind.inc:230 #8 0x000000000093313e in tui_disasm_window::set_contents (this=0x1c00f10, arch=0x1a3f2a0, sal=...) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-disasm.c:227 #9 0x000000000094c8fd in tui_source_window_base::update_source_window_as_is (this=0x1c00f10, gdbarch=0x1a3f2a0, sal=...) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-winsource.c:184 #10 0x0000000000933428 in tui_disasm_window::do_scroll_vertical (this=0x1c00f10, num_to_scroll=13) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-disasm.c:337 #11 0x000000000094a322 in tui_win_info::forward_scroll (this=0x1c00f10, num_to_scroll=12) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-win.c:476 #12 0x000000000093deac in tui_dispatch_ctrl_char (ch=338) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-io.c:921 #13 0x000000000093e05c in tui_getc (fp=0x7ffff49ae9e0 <_IO_2_1_stdin_>) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-io.c:1005 #14 0x00000000009dbedd in rl_read_key () at /home/pedro/gdb/binutils-gdb/src/readline/readline/input.c:495 #15 0x00000000009be41c in readline_internal_char () at /home/pedro/gdb/binutils-gdb/src/readline/readline/readline.c:573 #16 0x00000000009dca7b in rl_callback_read_char () at /home/pedro/gdb/binutils-gdb/src/readline/readline/callback.c:262 ... The issue is actually crossing readline here, tui_getc -> rl_read_key, not ncurses. * - eh, I was the one who filed this, I forgot, lol. I think we should add this patch, in addition to your fix, or something like it. From abb77826ee2ea565282d675d5c82a98e55601c41 Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Fri, 10 Jan 2020 13:32:07 +0000 Subject: [PATCH] Don't let TUI exceptions escape to readline (PR tui/9765) PR tui/9765 shows a use case where GDB dies from an uncaught exception, here: (top-gdb) bt #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51 #1 0x00007ffff4633d31 in __GI_abort () at abort.c:79 #2 0x00007ffff4c5d2a5 in __gnu_cxx::__verbose_terminate_handler () at ../../../../libstdc++-v3/libsupc++/vterminate.cc:95 #3 0x00007ffff4c5ae96 in __cxxabiv1::__terminate (handler=<optimized out>) at ../../../../libstdc++-v3/libsupc++/eh_terminate.cc:47 #4 0x00007ffff4c59d99 in __cxa_call_terminate (During symbol reading, Child DIE 0x2cee6 and its abstract origin 0x5a17 have different parents. ue_header=ue_header@entry=0x1a011a0) at ../../../../libstdc++-v3/libsupc++/eh_call.cc:54 #5 0x00007ffff4c5a788 in __cxxabiv1::__gxx_personality_v0 (version=<optimized out>, actions=<optimized out>, exception_class=5138137972254386944, ue_header=0x1a011a0, context=<optimized out>) at ../../../../libstdc++-v3/libsupc++/eh_personality.cc:676 #6 0x00007ffff49c3f33 in _Unwind_RaiseException_Phase2 (exc=exc@entry=0x1a011a0, context=context@entry=0x7fffffffcfa0) at ../../../libgcc/unwind.inc:62 #7 0x00007ffff49c475e in _Unwind_Resume (exc=0x1a011a0) at ../../../libgcc/unwind.inc:230 #8 0x000000000093313e in tui_disasm_window::set_contents (this=0x1c00f10, arch=0x1a3f2a0, sal=...) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-disasm.c:227 #9 0x000000000094c8fd in tui_source_window_base::update_source_window_as_is (this=0x1c00f10, gdbarch=0x1a3f2a0, sal=...) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-winsource.c:184 #10 0x0000000000933428 in tui_disasm_window::do_scroll_vertical (this=0x1c00f10, num_to_scroll=13) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-disasm.c:337 #11 0x000000000094a322 in tui_win_info::forward_scroll (this=0x1c00f10, num_to_scroll=12) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-win.c:476 #12 0x000000000093deac in tui_dispatch_ctrl_char (ch=338) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-io.c:921 #13 0x000000000093e05c in tui_getc (fp=0x7ffff49ae9e0 <_IO_2_1_stdin_>) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-io.c:1005 #14 0x00000000009dbedd in rl_read_key () at /home/pedro/gdb/binutils-gdb/src/readline/readline/input.c:495 #15 0x00000000009be41c in readline_internal_char () at /home/pedro/gdb/binutils-gdb/src/readline/readline/readline.c:573 #16 0x00000000009dca7b in rl_callback_read_char () at /home/pedro/gdb/binutils-gdb/src/readline/readline/callback.c:262 ... This is triggered by simply scrolling off the end of the dissasembly window. This commit doesn't fix the actual exception that is being thrown, which will still need to be fixed, but makes sure that we don't ever throw an exception out to readline. gdb/ChangeLog: yyyy-mm-dd Pedro Alves <palves@redhat.com> PR tui/9765 * tui/tui-io.c (tui_getc): Rename to ... (tui_getc_1): ... this. (tui_get): New, reimplent as try/catch wrapper around tui_getc_1. --- gdb/tui/tui-io.c | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) base-commit: ffebb0bbde7deae978ab3e4d3d3d90acf52b7d69
Comments
This patch must be in as well! It takes care of the "call stack 2" from the other e-mail. Cheers, -- Shahab
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> gdb/ChangeLog:
Pedro> yyyy-mm-dd Pedro Alves <palves@redhat.com>
Pedro> PR tui/9765
Pedro> * tui/tui-io.c (tui_getc): Rename to ...
Pedro> (tui_getc_1): ... this.
Pedro> (tui_get): New, reimplent as try/catch wrapper around tui_getc_1.
Pedro> ---
Pedro> gdb/tui/tui-io.c | 31 ++++++++++++++++++++++++++++---
Pedro> 1 file changed, 28 insertions(+), 3 deletions(-)
This seems like a good idea measure to me.
Ideally I suppose these TUI functions shouldn't throw. However, in gdb
it can be difficult to ensure that, so this provides some defense.
thanks,
Tom
Patch #1 is Pedro's patch to stop exceptions trying to cross readline which was posted elsewhere in this thread and really should be included before its lost. Patch #2 is a reworking of assembler window scrolling that allows it to handle trying to disassemble invalid memory, and also makes the whole scrolling experience more robust (I believe). -- Andrew Burgess (1): gdb/tui: asm window handles invalid memory and scrolls better Pedro Alves (1): gdb/tui: Prevent exceptions from trying to cross readline gdb/ChangeLog | 12 ++ gdb/testsuite/ChangeLog | 4 + gdb/testsuite/gdb.tui/tui-layout-asm.exp | 41 ++++++ gdb/tui/tui-disasm.c | 216 +++++++++++++++++++++++-------- gdb/tui/tui-io.c | 31 ++++- 5 files changed, 245 insertions(+), 59 deletions(-)
Hello Andrew, I have tested the patch series against the "hello world" program that I have [1]. Here comes my observations: 1. As you can see in this gif file: https://sourceware.org/bugzilla/attachment.cgi?id=12200 "Going up" sometimes does not work. In this case, it is between "main" and "_start" (garbage?). Nevertheless, I manged to have it working for me by: @@ -161,7 +161,7 @@ tui_find_symbol_backward (CORE_ADDR addr) { struct bound_minimal_symbol msym; - for (int offset = 1; offset <= 1024; offset *= 2) + for (int offset = 1; offset <= 1024; ++offset) { CORE_ADDR tmp = addr - offset; msym = lookup_minimal_symbol_by_pc_section (tmp, 0); 2. I run into a problem that "page-up" does not work when we reach the end of file: https://sourceware.org/bugzilla/attachment.cgi?id=12201 The following fixes that, but breaks elsewhere (see 3): @@ -232,14 +232,14 @@ tui_find_disassembly_address ... /* Disassemble forward a few lines and see ... next_addr = tui_disassemble (gdbarch, asm_lines, ... last_addr = asm_lines.back ().addr; - if (last_addr > pc && msymbol.minsym != nullptr + if (last_addr >= pc && msymbol.minsym != nullptr && asm_lines.size () >= max_lines) { /* This will do if we can't find anything... */ possible_new_low.found = true; possible_new_low.new_low = new_low; } - } while (last_addr > pc && msymbol.minsym != nullptr); + } while (last_addr >= pc && msymbol.minsym != nullptr); 3. With the changes from above, "arrow up" stops working near beginning of the file: https://sourceware.org/bugzilla/attachment.cgi?id=12202 I can summarise what I have learned in the following table: ,-------------.----------.----------------------------------. | usecase | action | condition that works | |-------------+----------+----------------------------------| | very bottom | page-up | as long as last_addr >= pc ..., | | | | go higher (try reducing new_low) | |-------------+----------+----------------------------------| | second line | arrow up | as long as last_addr > pc ..., | | | | go higher (try reducing new_low) | `-------------^----------^----------------------------------' This table portrays contradictory conditions for corner case scenarios. I believe the real solution in the end should be much simpler and ideally as such that it covers the corner cases naturally. I will try to cook something up in my free time. I suggest that this patch series should be in along with the changes proposed at point 1. So we only end-up with none-working page-up at the end of assembly output. Cheers, Shahab [1] that x86_64 elf program is archived here: https://sourceware.org/bugzilla/attachment.cgi?id=12199
This revision addresses the issues raised by Shahab, as well as making the improvements Tom pointed out. I looked at changing the TERM type from ansi to xterm as Tom suggested, but figuring out all of the extra control sequences that are sent was taking too much effort. I might try to revisit this when I have more time, but I don't plan to do this in the immediate future. I did start adding a mechanism to try and detect when the user tries to scroll and we're already at the end of the output (or the beginning), and this helped in the scroll down case, but I still need to figure out how to use this in the scroll up case, so for now I've not included this work in this patch set. There's still some things I think could be improved with the assembler scrolling - the user is currenly "trapped" inside the continuous memory region that the $pc starts in, they can't scroll to any disjoint code region, but this never worked before either, so this isn't a regression. I do have an idea for how to fix this, but I'm hoping to merge this set first, and work on the multi-section support when I can find some time later. Comments/feedback welcome as always, Thanks, Andrew --- Andrew Burgess (1): gdb/tui: asm window handles invalid memory and scrolls better Pedro Alves (1): gdb/tui: Prevent exceptions from trying to cross readline gdb/ChangeLog | 17 +++ gdb/minsyms.c | 41 ++++-- gdb/minsyms.h | 17 ++- gdb/testsuite/ChangeLog | 4 + gdb/testsuite/gdb.tui/tui-layout-asm.exp | 41 ++++++ gdb/tui/tui-disasm.c | 243 +++++++++++++++++++++++-------- gdb/tui/tui-io.c | 31 +++- 7 files changed, 313 insertions(+), 81 deletions(-)
These patches are in now: gdb/tui: Prevent exceptions from trying to cross readline https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=2f267673f0fdee9287e6d404ecd4f2d29da0d2f2 gdb/tui: asm window handles invalid memory and scrolls better https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=733d0a679536628eb1be4b4b8aa6384de24ff1f1 However, for the record, I must add that I have found one failing case. That happens when you have a small program that fits in one window, then you do a page down and reach the end of it. Now, the page up does not work anymore. A sample usecase is attached for your amusement. In case you do not like attachments, just save the base64 code below ... H4sIAAAAAAAAA+2WbW/bNhDH/db8FAclgGOgsiU/FgI2oEtSb0Aeijh9MSxFQUm0rJUWDZJqZBT9 7jtK8kPWtDXSwu0w/l5YJvk/HsnTiScy5nKqtMuFUMxlWezqPHWpWrgqkoLzNEu6jW/DQ8bDoXn6 46FXPft+2V/T8Psjvzcc+P6o38DRnjdowPAb/e5FrjSVAA01p3MafknHpDrEgg6L2Cf+N+cvzi7P n+zDBHg0GHw2/sNxbx1/b2Ti38P3BePvfcd9fpb/efxnqVQa9BzDrKBsBITGdGn6GDiRyGZp4oBi kU5FBmlW9i/oOzZLOSNEsqUUcR6hPSjNliogvwKL5gKc4w+n1xevL6+mH4vjDxd/XJ1PPzoE4LlX 9AYoMnOQkyQO2/AXLCVTCpY0YW4s7jMQWcTgE95s9Jm4LxdSWuRLiAVT2KnhXsh3QLPVQkiGehKz ME8Sszq0hFQrxmdB7R13Jd8z+dDHkdkjZgVoJhdpRvlaXM4Ej4gp+p3jNBsDEqeKKsUWocmecp0h DsjVxjEKyI+OfMVe+U8X8WjQefLb/7X873v9Ov/7Y3/QM/nf93yb/4eg2Ukzzfhbtco0LTCFMBNn aUGaHc0KjY+Ei5A335pT0qR6BKRZCNlktHgG+HPwFgA2gcWmGRY/SSL9R9kr/9ef+yf6+Er++wO/ t63/hmOT/2NvaPP/ENyeT2/NTRb8AuVXnpAjqC59Mjn7Deqh7pLqeVeLLt5lXbxIu3jpJ5IujFoL wRWZvLq+ua3Efq8/IJOXFy8m07LtsgJaimnYXosrd8bpe0zi8uPTqiScrkSuAV+9FplMK/va/M75 kv2dU4u2M9w5hFDOA3PrExLmKY8DOD4xu213FGken5yetsGNtn3gJn1wxaZDGNHFWducwKavPI8d ESuwBEIXAZQujAmeWhvHq/23HyqPQOaZWVJdeJw4+N+B+5RzCJmpTnBDMyZTIduk0mxm3loFOLs5 7dJN7W36uLvdKuffZc3RblnzWBlTjuz6X/tbRwuUjFp7h9dIqMQq8/Hl1ipMxgSFki2EZtuttsqi arMYEf4d54sluDG4l5WXh1vvvPr9+urPACLOaEZI+QhMeG4u2zvh3LX50ZlosVgsFovFYrFYLBaL xWKxWCwWi8XyffgH4PT6ZQAoAAA= ... and then: base64 -d <saved_code_from_email> > lingering-bug.tgz Last but not least, I cannot care less about this now, but I wanted it documented. Cheers, Shahab
Shahab, Thanks for reporting this regression. Here's a fix. I'll give this a few days for feedback, then push it. Thanks, Andrew --- Andrew Burgess (2): gdb/tui: Update help text for scroll commands gdb/tui: Disassembler scrolling of very small programs gdb/ChangeLog | 12 +++++ gdb/testsuite/ChangeLog | 6 +++ gdb/testsuite/gdb.tui/tui-layout-asm-short-prog.S | 22 ++++++++++ .../gdb.tui/tui-layout-asm-short-prog.exp | 51 ++++++++++++++++++++++ gdb/tui/tui-disasm.c | 2 +- gdb/tui/tui-win.c | 16 +++++-- 6 files changed, 104 insertions(+), 5 deletions(-) create mode 100644 gdb/testsuite/gdb.tui/tui-layout-asm-short-prog.S create mode 100644 gdb/testsuite/gdb.tui/tui-layout-asm-short-prog.exp
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c index 9cb41104fe9..d9f23334f57 100644 --- a/gdb/tui/tui-io.c +++ b/gdb/tui/tui-io.c @@ -950,10 +950,12 @@ tui_dispatch_ctrl_char (unsigned int ch) return 0; } -/* Get a character from the command window. This is called from the - readline package. */ +/* Main worker for tui_getc. Get a character from the command window. + This is called from the readline package, but wrapped in a + try/catch by tui_getc. */ + static int -tui_getc (FILE *fp) +tui_getc_1 (FILE *fp) { int ch; WINDOW *w; @@ -1036,6 +1038,29 @@ tui_getc (FILE *fp) return ch; } +/* Get a character from the command window. This is called from the + readline package. */ + +static int +tui_getc (FILE *fp) +{ + try + { + return tui_getc_1 (fp); + } + catch (const gdb_exception &ex) + { + /* Just in case, don't ever let an exception escape to readline. + This shouldn't ever happen, but if it does, print the + exception instead of just crashing GDB. */ + exception_print (gdb_stderr, ex); + + /* If we threw an exception, it's because we recognized the + character. */ + return 0; + } +} + /* See tui-io.h. */ gdb::unique_xmalloc_ptr<char>