* [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).