* [PATCH v2] [gdb/tui] Show regs when switching to regs layout @ 2023-11-26 7:52 Tom de Vries 2023-11-27 12:27 ` Alexandra Petlanova Hajkova 2023-12-08 15:29 ` Tom Tromey 0 siblings, 2 replies; 8+ messages in thread From: Tom de Vries @ 2023-11-26 7:52 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey When starting gdb in CLI mode, running to main and switching into the TUI regs layout: ... $ gdb -q a.out -ex start -ex "layout regs" ... we get: ... +---------------------------------+ | | | [ Register Values Unavailable ] | | | +---------------------------------+ ... Fix this by handling this case in tui_data_window::rerender. Tested on x86_64-linux. PR tui/28600 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28600 --- gdb/testsuite/gdb.tui/regs.exp | 10 ++++++---- gdb/tui/tui-regs.c | 17 ++++++++++++++++- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/gdb/testsuite/gdb.tui/regs.exp b/gdb/testsuite/gdb.tui/regs.exp index 520f6ddba96..0be99625b9f 100644 --- a/gdb/testsuite/gdb.tui/regs.exp +++ b/gdb/testsuite/gdb.tui/regs.exp @@ -41,10 +41,12 @@ Term::command "layout regs" Term::check_box "register box" 0 0 80 8 Term::check_box "source box in regs layout" 0 7 80 8 -set text [Term::get_line 1] -# Just check for any register window content at all. -Term::check_contents "any register contents" "\\|.*\[^ \].*\\|" - +# The current frame is main, check that registers are available. +set re_reg_vals_unavailable \ + [string_to_regexp {[ Register Values Unavailable ]}] +gdb_assert \ + { ![Term::check_region_contents_p 0 0 80 8 $re_reg_vals_unavailable] } \ + "Register values available" # Check that we can successfully cause the register window to appear # using the 'tui reg next' and 'tui reg prev' commands. diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c index 01538d49961..fe9cd5ac722 100644 --- a/gdb/tui/tui-regs.c +++ b/gdb/tui/tui-regs.c @@ -417,14 +417,29 @@ tui_data_window::erase_data_content (const char *prompt) void tui_data_window::rerender () { + /* Calling check_register_value calls rerender again. We use this counter + to prevent enless recursion. */ + static int in_rerender; + in_rerender++; + if (m_regs_content.empty ()) - erase_data_content (_("[ Register Values Unavailable ]")); + { + if (has_stack_frames () && in_rerender == 1) + { + frame_info_ptr fi = get_selected_frame (NULL); + check_register_values (fi); + } + else + erase_data_content (_("[ Register Values Unavailable ]")); + } else { erase_data_content (NULL); delete_data_content_windows (); display_registers_from (0); } + + in_rerender--; } base-commit: 476bf7d5e6661b06eb9f8de9258cf48fd81919af -- 2.35.3 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] [gdb/tui] Show regs when switching to regs layout 2023-11-26 7:52 [PATCH v2] [gdb/tui] Show regs when switching to regs layout Tom de Vries @ 2023-11-27 12:27 ` Alexandra Petlanova Hajkova 2023-12-08 15:29 ` Tom Tromey 1 sibling, 0 replies; 8+ messages in thread From: Alexandra Petlanova Hajkova @ 2023-11-27 12:27 UTC (permalink / raw) To: Tom de Vries; +Cc: gdb-patches, Tom Tromey [-- Attachment #1: Type: text/plain, Size: 907 bytes --] On Sun, Nov 26, 2023 at 8:52 AM Tom de Vries <tdevries@suse.de> wrote: > When starting gdb in CLI mode, running to main and switching into the TUI > regs > layout: > ... > $ gdb -q a.out -ex start -ex "layout regs" > ... > we get: > ... > +---------------------------------+ > | | > | [ Register Values Unavailable ] | > | | > +---------------------------------+ > ... > > Fix this by handling this case in tui_data_window::rerender. > > Tested on x86_64-linux. > > PR tui/28600 > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28600 > --- > I can confirm I can see this issue on ppc64le Fedora Rawhide and this patch > fixes this. I'm able to see the Register group now > when running "gdb/gdb -q /bin/ls -ex start -ex "layout regs"". I can also confirm this change does not introduce any regressions. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] [gdb/tui] Show regs when switching to regs layout 2023-11-26 7:52 [PATCH v2] [gdb/tui] Show regs when switching to regs layout Tom de Vries 2023-11-27 12:27 ` Alexandra Petlanova Hajkova @ 2023-12-08 15:29 ` Tom Tromey 2023-12-09 8:30 ` Tom de Vries 2023-12-09 10:01 ` Tom de Vries 1 sibling, 2 replies; 8+ messages in thread From: Tom Tromey @ 2023-12-08 15:29 UTC (permalink / raw) To: Tom de Vries; +Cc: gdb-patches, Tom Tromey >>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes: Tom> void Tom> tui_data_window::rerender () Tom> { Tom> + /* Calling check_register_value calls rerender again. We use this counter Tom> + to prevent enless recursion. */ Tom> + static int in_rerender; There's a typo, the function is check_register_values (missing an "s" in the comment). However, it seems bad to need new state for this. This triggered a memory -- there's already weird state in this area, see tui-hooks.c:tui_refreshing_registers. I have never understood why that is needed. I wonder if this can be untangled. Tom ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] [gdb/tui] Show regs when switching to regs layout 2023-12-08 15:29 ` Tom Tromey @ 2023-12-09 8:30 ` Tom de Vries 2023-12-09 10:01 ` Tom de Vries 1 sibling, 0 replies; 8+ messages in thread From: Tom de Vries @ 2023-12-09 8:30 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 12/8/23 16:29, Tom Tromey wrote: > This triggered a memory -- there's already weird state in this area, see > tui-hooks.c:tui_refreshing_registers. I have never understood why that > is needed. > > I wonder if this can be untangled. I don't know why it's needed either, and I didn't manage to trigger usage of it either. I've submitted a patch ( https://sourceware.org/pipermail/gdb-patches/2023-December/204933.html ) that changes behaviour from recursion prevention to asserting on recursion detection. Thanks, - Tom ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] [gdb/tui] Show regs when switching to regs layout 2023-12-08 15:29 ` Tom Tromey 2023-12-09 8:30 ` Tom de Vries @ 2023-12-09 10:01 ` Tom de Vries 2023-12-16 1:40 ` Tom Tromey 1 sibling, 1 reply; 8+ messages in thread From: Tom de Vries @ 2023-12-09 10:01 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 827 bytes --] On 12/8/23 16:29, Tom Tromey wrote: >>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes: > > Tom> void > Tom> tui_data_window::rerender () > Tom> { > Tom> + /* Calling check_register_value calls rerender again. We use this counter > Tom> + to prevent enless recursion. */ > Tom> + static int in_rerender; > > There's a typo, the function is check_register_values (missing an "s" in > the comment). > > However, it seems bad to need new state for this. > How about this approach? Rather than adding global state, it adds a function parameter. It's currently called toplevel, though I've also consider "active". Another solution is to split out the functionality between two functions, I didn't pursue this because I couldn't think of good names (I came up with rerender and rerender_1). Thanks, - Tom [-- Attachment #2: v2-0001-gdb-tui-Show-regs-when-switching-to-regs-layout.patch --] [-- Type: text/x-patch, Size: 3084 bytes --] From 9590364f9dffc0fbb29f00690bae3df8b2c92cf3 Mon Sep 17 00:00:00 2001 From: Tom de Vries <tdevries@suse.de> Date: Sun, 26 Nov 2023 08:49:28 +0100 Subject: [PATCH v2] [gdb/tui] Show regs when switching to regs layout When starting gdb in CLI mode, running to main and switching into the TUI regs layout: ... $ gdb -q a.out -ex start -ex "layout regs" ... we get: ... +---------------------------------+ | | | [ Register Values Unavailable ] | | | +---------------------------------+ ... Fix this by handling this case in tui_data_window::rerender. Tested on x86_64-linux. PR tui/28600 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28600 --- gdb/testsuite/gdb.tui/regs.exp | 10 ++++++---- gdb/tui/tui-regs.c | 14 +++++++++++--- gdb/tui/tui-regs.h | 6 +++++- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/gdb/testsuite/gdb.tui/regs.exp b/gdb/testsuite/gdb.tui/regs.exp index 520f6ddba96..0be99625b9f 100644 --- a/gdb/testsuite/gdb.tui/regs.exp +++ b/gdb/testsuite/gdb.tui/regs.exp @@ -41,10 +41,12 @@ Term::command "layout regs" Term::check_box "register box" 0 0 80 8 Term::check_box "source box in regs layout" 0 7 80 8 -set text [Term::get_line 1] -# Just check for any register window content at all. -Term::check_contents "any register contents" "\\|.*\[^ \].*\\|" - +# The current frame is main, check that registers are available. +set re_reg_vals_unavailable \ + [string_to_regexp {[ Register Values Unavailable ]}] +gdb_assert \ + { ![Term::check_region_contents_p 0 0 80 8 $re_reg_vals_unavailable] } \ + "Register values available" # Check that we can successfully cause the register window to appear # using the 'tui reg next' and 'tui reg prev' commands. diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c index 4c6ea8aff0d..da2c9a26995 100644 --- a/gdb/tui/tui-regs.c +++ b/gdb/tui/tui-regs.c @@ -192,7 +192,7 @@ tui_data_window::show_registers (const reggroup *group) m_regs_content.clear (); } - rerender (); + rerender (false); } @@ -415,10 +415,18 @@ tui_data_window::erase_data_content (const char *prompt) /* See tui-regs.h. */ void -tui_data_window::rerender () +tui_data_window::rerender (bool toplevel) { if (m_regs_content.empty ()) - erase_data_content (_("[ Register Values Unavailable ]")); + { + if (toplevel && has_stack_frames ()) + { + frame_info_ptr fi = get_selected_frame (NULL); + check_register_values (fi); + } + else + erase_data_content (_("[ Register Values Unavailable ]")); + } else { erase_data_content (NULL); diff --git a/gdb/tui/tui-regs.h b/gdb/tui/tui-regs.h index 5adff6300aa..1abd22cd382 100644 --- a/gdb/tui/tui-regs.h +++ b/gdb/tui/tui-regs.h @@ -75,7 +75,11 @@ struct tui_data_window : public tui_win_info { } - void rerender () override; + void rerender (bool toplevel); + void rerender () override + { + rerender (true); + } private: base-commit: 1340072a5412877d9d98f5b010cd8e6e9e418905 -- 2.35.3 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] [gdb/tui] Show regs when switching to regs layout 2023-12-09 10:01 ` Tom de Vries @ 2023-12-16 1:40 ` Tom Tromey 2023-12-16 8:34 ` Tom de Vries 0 siblings, 1 reply; 8+ messages in thread From: Tom Tromey @ 2023-12-16 1:40 UTC (permalink / raw) To: Tom de Vries; +Cc: Tom Tromey, gdb-patches >>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes: Tom> How about this approach? Tom> Rather than adding global state, it adds a function parameter. Tom> It's currently called toplevel, though I've also consider "active". This is fine by me. Really I suspect the register window refresh code can be untangled and not need any of this, but it's fine to just fix the bug and move on. Approved-By: Tom Tromey <tom@tromey.com> Tom ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] [gdb/tui] Show regs when switching to regs layout 2023-12-16 1:40 ` Tom Tromey @ 2023-12-16 8:34 ` Tom de Vries 2023-12-17 17:31 ` Tom Tromey 0 siblings, 1 reply; 8+ messages in thread From: Tom de Vries @ 2023-12-16 8:34 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 12/16/23 02:40, Tom Tromey wrote: >>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes: > > Tom> How about this approach? > > Tom> Rather than adding global state, it adds a function parameter. > > Tom> It's currently called toplevel, though I've also consider "active". > > This is fine by me. > > Really I suspect the register window refresh code can be untangled and > not need any of this, but it's fine to just fix the bug and move on. > I suspect that as well, but unfortunately I don't understand how yet. > Approved-By: Tom Tromey <tom@tromey.com> Thanks for the review, pushed. - Tom ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] [gdb/tui] Show regs when switching to regs layout 2023-12-16 8:34 ` Tom de Vries @ 2023-12-17 17:31 ` Tom Tromey 0 siblings, 0 replies; 8+ messages in thread From: Tom Tromey @ 2023-12-17 17:31 UTC (permalink / raw) To: Tom de Vries; +Cc: Tom Tromey, gdb-patches Tom> I suspect that as well, but unfortunately I don't understand how yet. I have some patches I'll send to clean this up. It's still a bit ugly at the end but it at least gets rid of the "two rerender methods" problem. (Specifically, the layout code is over-complicated; this is pretty old stuff...) I also found a generic window refresh problem that I'm still investigating. It seems like at some point I tried to tackle this but I fundamentally misunderstood wnoutrefresh. Tom ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-12-17 17:31 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-11-26 7:52 [PATCH v2] [gdb/tui] Show regs when switching to regs layout Tom de Vries 2023-11-27 12:27 ` Alexandra Petlanova Hajkova 2023-12-08 15:29 ` Tom Tromey 2023-12-09 8:30 ` Tom de Vries 2023-12-09 10:01 ` Tom de Vries 2023-12-16 1:40 ` Tom Tromey 2023-12-16 8:34 ` Tom de Vries 2023-12-17 17:31 ` Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).