public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] TUI disassembly window improvement, take 2
@ 2021-06-22 13:56 Vasili Burdo
  2021-06-28 14:45 ` [PATCH] [PING] " Vasili Burdo
  2022-01-23 19:53 ` [PATCH] " Simon Marchi
  0 siblings, 2 replies; 13+ messages in thread
From: Vasili Burdo @ 2021-06-22 13:56 UTC (permalink / raw)
  To: Vasili Burdo via Gdb-patches

[-- Attachment #1: Type: text/plain, Size: 563 bytes --]

Hi,

I've got GDB copyright assignment RT:1731775, so, I'm resending my
patch - see attachment.

Previous discussion thread and exlanation what's changed is here:
https://sourceware.org/pipermail/gdb-patches/2021-June/179693.html and
here: https://sourceware.org/pipermail/gdb-patches/2021-June/179695.html

Changes in current patch:
- Added changelog entry
- Tom Tromey's comments are addressed.
- Disasm window title is kept intact. Reason - everything important is
already displayed in status line, so, no useful information for title
remains.

Thanks.
Vasili

[-- Attachment #2: vburdo-tui-asm.patch --]
[-- Type: text/x-patch, Size: 5145 bytes --]

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2b04c3c6d73..dda83efed46 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2021-06-22  Vasili Burdo <vasili.burdo@gmail.com>
+
+	PR tui/25347
+	* tui/tui-disasm.c (tui_disassemble,tui_disasm_window::set_content): Hide
+	the function names in TUI disassembly view. Instead, display them like
+	assembly language labels.
+
 2021-06-22  Andrew Burgess  <andrew.burgess@embecosm.com>

 	* breakpoint.c (insert_bp_location): If we catch a
diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
index 163552aede4..b003c11f7ee 100644
--- a/gdb/tui/tui-disasm.c
+++ b/gdb/tui/tui-disasm.c
@@ -27,6 +27,7 @@
 #include "value.h"
 #include "source.h"
 #include "disasm.h"
+#include "valprint.h"
 #include "tui/tui.h"
 #include "tui/tui-command.h"
 #include "tui/tui-data.h"
@@ -50,6 +51,7 @@ struct tui_asm_line
   std::string addr_string;
   size_t addr_size;
   std::string insn;
+  std::string func_string;
 };

 /* Helper function to find the number of characters in STR, skipping
@@ -92,6 +94,9 @@ len_without_escapes (const std::string &str)
    label into the value pointed to by ADDR_SIZE, and set the addr_size
    field on each item in ASM_LINES, otherwise the addr_size fields within
    ASM_LINES are undefined.
+   FOR_UI boolean flag shows the purpose this function is called for. TRUE
+   means we're going to show ASM_LINES on screen, FALSE means ASM_LINES are
+   used to figure out current scroll position or any other purpose.

    It is worth noting that ASM_LINES might not have COUNT entries when this
    function returns.  If the disassembly is truncated for some other
@@ -101,7 +106,8 @@ static CORE_ADDR
 tui_disassemble (struct gdbarch *gdbarch,
 		 std::vector<tui_asm_line> &asm_lines,
 		 CORE_ADDR pc, int count,
-		 size_t *addr_size = nullptr)
+		 size_t *addr_size = nullptr,
+		 bool for_ui = false)
 {
   bool term_out = source_styling && gdb_stdout->can_emit_style_escape ();
   string_file gdb_dis_out (term_out);
@@ -110,7 +116,7 @@ tui_disassemble (struct gdbarch *gdbarch,
   asm_lines.clear ();

   /* Now construct each line.  */
-  for (int i = 0; i < count; ++i)
+  while (asm_lines.size () < count)
     {
       tui_asm_line tal;
       CORE_ADDR orig_pc = pc;
@@ -135,8 +141,52 @@ tui_disassemble (struct gdbarch *gdbarch,

       /* And capture the address the instruction is at.  */
       tal.addr = orig_pc;
-      print_address (gdbarch, orig_pc, &gdb_dis_out);
-      tal.addr_string = std::move (gdb_dis_out.string ());
+      if (for_ui)
+        { /* If we're rendering for UI... */
+          std::string name, filename;
+          int unmapped = 0, offset = 0, line = 0;
+
+          if (0 == build_address_symbolic (gdbarch, orig_pc, asm_demangle,
+                false, &name, &offset, &filename, &line, &unmapped))
+            {
+              if (0 == offset || asm_lines.empty ())
+                {/* "offset == 0" means function start,
+                    "asm_lines.empty ()" means top line of output.
+                    For these cases, insert "function header" on top of
+                    "real" disassembly line. */
+                  tui_asm_line tal2 = tal;
+                  fputs_styled (name.c_str (), function_name_style.style (),
+                                &gdb_dis_out);
+                  fputs_filtered (":", &gdb_dis_out);
+                  tal2.addr_string = std::move (gdb_dis_out.string ());
+                  tal2.addr_size = 0;
+                  tal2.insn.clear ();
+                  tal2.func_string = name;
+                  gdb_dis_out.clear ();
+
+                  asm_lines.push_back (std::move (tal2));
+                }
+
+              fputs_styled (paddress (gdbarch, orig_pc),
+                            address_style.style (), &gdb_dis_out);
+              fputs_filtered (" <", &gdb_dis_out);
+              if (unmapped)
+                fputs_filtered ("*", &gdb_dis_out);
+              fprintf_styled (&gdb_dis_out, address_style.style (),
+                              "%+d", offset);
+              if (unmapped)
+                fputs_filtered ("*", &gdb_dis_out);
+              fputs_filtered (">", &gdb_dis_out);
+              tal.addr_string = std::move (gdb_dis_out.string ());
+              tal.func_string = std::move (name);
+            }
+        }
+      else
+        { /* If we're rendering for some other purpose than UI...
+             Fall back to legacy disassembly line style. */
+          print_address (gdbarch, orig_pc, &gdb_dis_out);
+          tal.addr_string = std::move (gdb_dis_out.string ());
+        }
       gdb_dis_out.clear ();

       if (addr_size != nullptr)
@@ -342,7 +392,7 @@ tui_disasm_window::set_contents (struct gdbarch *arch,
   /* Get temporary table that will hold all strings (addr & insn).  */
   std::vector<tui_asm_line> asm_lines;
   size_t addr_size = 0;
-  tui_disassemble (m_gdbarch, asm_lines, pc, max_lines, &addr_size);
+  tui_disassemble (m_gdbarch, asm_lines, pc, max_lines, &addr_size, true);

   /* Align instructions to the same column.  */
   insn_pos = (1 + (addr_size / tab_len)) * tab_len;

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] [PING] TUI disassembly window improvement, take 2
  2021-06-22 13:56 [PATCH] TUI disassembly window improvement, take 2 Vasili Burdo
@ 2021-06-28 14:45 ` Vasili Burdo
  2021-07-02 11:43   ` [PATCH] [PING #2] " Vasili Burdo
  2022-01-23 19:53 ` [PATCH] " Simon Marchi
  1 sibling, 1 reply; 13+ messages in thread
From: Vasili Burdo @ 2021-06-28 14:45 UTC (permalink / raw)
  To: Vasili Burdo via Gdb-patches

Ping!

вт, 22 июн. 2021 г. в 16:56, Vasili Burdo <vasili.burdo@gmail.com>:
>
> Hi,
>
> I've got GDB copyright assignment RT:1731775, so, I'm resending my
> patch - see attachment.
>
> Previous discussion thread and exlanation what's changed is here:
> https://sourceware.org/pipermail/gdb-patches/2021-June/179693.html and
> here: https://sourceware.org/pipermail/gdb-patches/2021-June/179695.html
>
> Changes in current patch:
> - Added changelog entry
> - Tom Tromey's comments are addressed.
> - Disasm window title is kept intact. Reason - everything important is
> already displayed in status line, so, no useful information for title
> remains.
>
> Thanks.
> Vasili

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] [PING #2] TUI disassembly window improvement, take 2
  2021-06-28 14:45 ` [PATCH] [PING] " Vasili Burdo
@ 2021-07-02 11:43   ` Vasili Burdo
  2021-07-06 11:18     ` [PATCH] [PING #3] " Vasili Burdo
  0 siblings, 1 reply; 13+ messages in thread
From: Vasili Burdo @ 2021-07-02 11:43 UTC (permalink / raw)
  To: Vasili Burdo via Gdb-patches

Ping!
2 packets transmitted, 0 received, 100% packet loss, time 3 weeks

Thanks.
Vasili

пн, 28 июн. 2021 г. в 17:45, Vasili Burdo <vasili.burdo@gmail.com>:
>
> Ping!
>
> вт, 22 июн. 2021 г. в 16:56, Vasili Burdo <vasili.burdo@gmail.com>:
> >
> > Hi,
> >
> > I've got GDB copyright assignment RT:1731775, so, I'm resending my
> > patch - see attachment.
> >
> > Previous discussion thread and exlanation what's changed is here:
> > https://sourceware.org/pipermail/gdb-patches/2021-June/179693.html and
> > here: https://sourceware.org/pipermail/gdb-patches/2021-June/179695.html
> >
> > Changes in current patch:
> > - Added changelog entry
> > - Tom Tromey's comments are addressed.
> > - Disasm window title is kept intact. Reason - everything important is
> > already displayed in status line, so, no useful information for title
> > remains.
> >
> > Thanks.
> > Vasili

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH] [PING #3] TUI disassembly window improvement, take 2
  2021-07-02 11:43   ` [PATCH] [PING #2] " Vasili Burdo
@ 2021-07-06 11:18     ` Vasili Burdo
  2021-07-12 20:51       ` [PING][#4][PATCH] " Vasili Burdo
  0 siblings, 1 reply; 13+ messages in thread
From: Vasili Burdo @ 2021-07-06 11:18 UTC (permalink / raw)
  To: Vasili Burdo via Gdb-patches

Ping!
3 packets transmitted, 0 received, 100% packet loss, time 3.5 weeks

Thanks.
Vasili

пт, 2 июл. 2021 г. в 14:43, Vasili Burdo <vasili.burdo@gmail.com>:
>
> Ping!
> 2 packets transmitted, 0 received, 100% packet loss, time 3 weeks
>
> Thanks.
> Vasili
>
> пн, 28 июн. 2021 г. в 17:45, Vasili Burdo <vasili.burdo@gmail.com>:
> >
> > Ping!
> >
> > вт, 22 июн. 2021 г. в 16:56, Vasili Burdo <vasili.burdo@gmail.com>:
> > >
> > > Hi,
> > >
> > > I've got GDB copyright assignment RT:1731775, so, I'm resending my
> > > patch - see attachment.
> > >
> > > Previous discussion thread and exlanation what's changed is here:
> > > https://sourceware.org/pipermail/gdb-patches/2021-June/179693.html and
> > > here: https://sourceware.org/pipermail/gdb-patches/2021-June/179695.html
> > >
> > > Changes in current patch:
> > > - Added changelog entry
> > > - Tom Tromey's comments are addressed.
> > > - Disasm window title is kept intact. Reason - everything important is
> > > already displayed in status line, so, no useful information for title
> > > remains.
> > >
> > > Thanks.
> > > Vasili

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PING][#4][PATCH] TUI disassembly window improvement, take 2
  2021-07-06 11:18     ` [PATCH] [PING #3] " Vasili Burdo
@ 2021-07-12 20:51       ` Vasili Burdo
  2021-07-16 11:25         ` [PING][#5][PATCH] " Vasili Burdo
  2021-07-28 10:57         ` Vasili Burdo
  0 siblings, 2 replies; 13+ messages in thread
From: Vasili Burdo @ 2021-07-12 20:51 UTC (permalink / raw)
  To: Vasili Burdo via Gdb-patches

Ping!
4 packets transmitted, 0 received, 100% packet loss, time 4 weeks

Thanks.
Vasili

вт, 6 июл. 2021 г. в 14:18, Vasili Burdo <vasili.burdo@gmail.com>:
>
> Ping!
> 3 packets transmitted, 0 received, 100% packet loss, time 3.5 weeks
>
> Thanks.
> Vasili
>
> пт, 2 июл. 2021 г. в 14:43, Vasili Burdo <vasili.burdo@gmail.com>:
> >
> > Ping!
> > 2 packets transmitted, 0 received, 100% packet loss, time 3 weeks
> >
> > Thanks.
> > Vasili
> >
> > пн, 28 июн. 2021 г. в 17:45, Vasili Burdo <vasili.burdo@gmail.com>:
> > >
> > > Ping!
> > >
> > > вт, 22 июн. 2021 г. в 16:56, Vasili Burdo <vasili.burdo@gmail.com>:
> > > >
> > > > Hi,
> > > >
> > > > I've got GDB copyright assignment RT:1731775, so, I'm resending my
> > > > patch - see attachment.
> > > >
> > > > Previous discussion thread and exlanation what's changed is here:
> > > > https://sourceware.org/pipermail/gdb-patches/2021-June/179693.html and
> > > > here: https://sourceware.org/pipermail/gdb-patches/2021-June/179695.html
> > > >
> > > > Changes in current patch:
> > > > - Added changelog entry
> > > > - Tom Tromey's comments are addressed.
> > > > - Disasm window title is kept intact. Reason - everything important is
> > > > already displayed in status line, so, no useful information for title
> > > > remains.
> > > >
> > > > Thanks.
> > > > Vasili

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PING][#5][PATCH] TUI disassembly window improvement, take 2
  2021-07-12 20:51       ` [PING][#4][PATCH] " Vasili Burdo
@ 2021-07-16 11:25         ` Vasili Burdo
  2021-07-28 10:57         ` Vasili Burdo
  1 sibling, 0 replies; 13+ messages in thread
From: Vasili Burdo @ 2021-07-16 11:25 UTC (permalink / raw)
  To: Vasili Burdo via Gdb-patches

Ping!
5 packets transmitted, 0 received, 100% packet loss, time 4.5 weeks

Thanks.
Vasili

пн, 12 июл. 2021 г. в 23:51, Vasili Burdo <vasili.burdo@gmail.com>:
>
> Ping!
> 4 packets transmitted, 0 received, 100% packet loss, time 4 weeks
>
> Thanks.
> Vasili
>
> вт, 6 июл. 2021 г. в 14:18, Vasili Burdo <vasili.burdo@gmail.com>:
> >
> > Ping!
> > 3 packets transmitted, 0 received, 100% packet loss, time 3.5 weeks
> >
> > Thanks.
> > Vasili
> >
> > пт, 2 июл. 2021 г. в 14:43, Vasili Burdo <vasili.burdo@gmail.com>:
> > >
> > > Ping!
> > > 2 packets transmitted, 0 received, 100% packet loss, time 3 weeks
> > >
> > > Thanks.
> > > Vasili
> > >
> > > пн, 28 июн. 2021 г. в 17:45, Vasili Burdo <vasili.burdo@gmail.com>:
> > > >
> > > > Ping!
> > > >
> > > > вт, 22 июн. 2021 г. в 16:56, Vasili Burdo <vasili.burdo@gmail.com>:
> > > > >
> > > > > Hi,
> > > > >
> > > > > I've got GDB copyright assignment RT:1731775, so, I'm resending my
> > > > > patch - see attachment.
> > > > >
> > > > > Previous discussion thread and exlanation what's changed is here:
> > > > > https://sourceware.org/pipermail/gdb-patches/2021-June/179693.html and
> > > > > here: https://sourceware.org/pipermail/gdb-patches/2021-June/179695.html
> > > > >
> > > > > Changes in current patch:
> > > > > - Added changelog entry
> > > > > - Tom Tromey's comments are addressed.
> > > > > - Disasm window title is kept intact. Reason - everything important is
> > > > > already displayed in status line, so, no useful information for title
> > > > > remains.
> > > > >
> > > > > Thanks.
> > > > > Vasili

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PING][#5][PATCH] TUI disassembly window improvement, take 2
  2021-07-12 20:51       ` [PING][#4][PATCH] " Vasili Burdo
  2021-07-16 11:25         ` [PING][#5][PATCH] " Vasili Burdo
@ 2021-07-28 10:57         ` Vasili Burdo
  2021-08-05 10:46           ` [PING][#6][PATCH] " Vasili Burdo
  1 sibling, 1 reply; 13+ messages in thread
From: Vasili Burdo @ 2021-07-28 10:57 UTC (permalink / raw)
  To: Vasili Burdo via Gdb-patches

5 packets transmitted, 0 received, 100% packet loss, time 5 weeks

Thanks.
Vasili


пн, 12 июл. 2021 г. в 23:51, Vasili Burdo <vasili.burdo@gmail.com>:
>
> Ping!
> 4 packets transmitted, 0 received, 100% packet loss, time 4 weeks
>
> Thanks.
> Vasili
>
> вт, 6 июл. 2021 г. в 14:18, Vasili Burdo <vasili.burdo@gmail.com>:
> >
> > Ping!
> > 3 packets transmitted, 0 received, 100% packet loss, time 3.5 weeks
> >
> > Thanks.
> > Vasili
> >
> > пт, 2 июл. 2021 г. в 14:43, Vasili Burdo <vasili.burdo@gmail.com>:
> > >
> > > Ping!
> > > 2 packets transmitted, 0 received, 100% packet loss, time 3 weeks
> > >
> > > Thanks.
> > > Vasili
> > >
> > > пн, 28 июн. 2021 г. в 17:45, Vasili Burdo <vasili.burdo@gmail.com>:
> > > >
> > > > Ping!
> > > >
> > > > вт, 22 июн. 2021 г. в 16:56, Vasili Burdo <vasili.burdo@gmail.com>:
> > > > >
> > > > > Hi,
> > > > >
> > > > > I've got GDB copyright assignment RT:1731775, so, I'm resending my
> > > > > patch - see attachment.
> > > > >
> > > > > Previous discussion thread and exlanation what's changed is here:
> > > > > https://sourceware.org/pipermail/gdb-patches/2021-June/179693.html and
> > > > > here: https://sourceware.org/pipermail/gdb-patches/2021-June/179695.html
> > > > >
> > > > > Changes in current patch:
> > > > > - Added changelog entry
> > > > > - Tom Tromey's comments are addressed.
> > > > > - Disasm window title is kept intact. Reason - everything important is
> > > > > already displayed in status line, so, no useful information for title
> > > > > remains.
> > > > >
> > > > > Thanks.
> > > > > Vasili

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PING][#6][PATCH] TUI disassembly window improvement, take 2
  2021-07-28 10:57         ` Vasili Burdo
@ 2021-08-05 10:46           ` Vasili Burdo
  2021-08-05 14:40             ` Simon Marchi
  0 siblings, 1 reply; 13+ messages in thread
From: Vasili Burdo @ 2021-08-05 10:46 UTC (permalink / raw)
  To: Vasili Burdo via Gdb-patches

6 packets transmitted, 0 received, 100% packet loss, time 6 weeks

Thanks.
Vasili

ср, 28 июл. 2021 г. в 13:57, Vasili Burdo <vasili.burdo@gmail.com>:
>
> 5 packets transmitted, 0 received, 100% packet loss, time 5 weeks
>
> Thanks.
> Vasili
>
>
> пн, 12 июл. 2021 г. в 23:51, Vasili Burdo <vasili.burdo@gmail.com>:
> >
> > Ping!
> > 4 packets transmitted, 0 received, 100% packet loss, time 4 weeks
> >
> > Thanks.
> > Vasili
> >
> > вт, 6 июл. 2021 г. в 14:18, Vasili Burdo <vasili.burdo@gmail.com>:
> > >
> > > Ping!
> > > 3 packets transmitted, 0 received, 100% packet loss, time 3.5 weeks
> > >
> > > Thanks.
> > > Vasili
> > >
> > > пт, 2 июл. 2021 г. в 14:43, Vasili Burdo <vasili.burdo@gmail.com>:
> > > >
> > > > Ping!
> > > > 2 packets transmitted, 0 received, 100% packet loss, time 3 weeks
> > > >
> > > > Thanks.
> > > > Vasili
> > > >
> > > > пн, 28 июн. 2021 г. в 17:45, Vasili Burdo <vasili.burdo@gmail.com>:
> > > > >
> > > > > Ping!
> > > > >
> > > > > вт, 22 июн. 2021 г. в 16:56, Vasili Burdo <vasili.burdo@gmail.com>:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > I've got GDB copyright assignment RT:1731775, so, I'm resending my
> > > > > > patch - see attachment.
> > > > > >
> > > > > > Previous discussion thread and exlanation what's changed is here:
> > > > > > https://sourceware.org/pipermail/gdb-patches/2021-June/179693.html and
> > > > > > here: https://sourceware.org/pipermail/gdb-patches/2021-June/179695.html
> > > > > >
> > > > > > Changes in current patch:
> > > > > > - Added changelog entry
> > > > > > - Tom Tromey's comments are addressed.
> > > > > > - Disasm window title is kept intact. Reason - everything important is
> > > > > > already displayed in status line, so, no useful information for title
> > > > > > remains.
> > > > > >
> > > > > > Thanks.
> > > > > > Vasili

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PING][#6][PATCH] TUI disassembly window improvement, take 2
  2021-08-05 10:46           ` [PING][#6][PATCH] " Vasili Burdo
@ 2021-08-05 14:40             ` Simon Marchi
  0 siblings, 0 replies; 13+ messages in thread
From: Simon Marchi @ 2021-08-05 14:40 UTC (permalink / raw)
  To: Vasili Burdo, Vasili Burdo via Gdb-patches

Sorry about the delay. Tom is the best person to review this, I'm sure he'll
get to it when he has time.  In the mean time, keep pinging, thanks!

Simon

On 2021-08-05 6:46 a.m., Vasili Burdo via Gdb-patches wrote:
> 6 packets transmitted, 0 received, 100% packet loss, time 6 weeks
> 
> Thanks.
> Vasili
> 
> ср, 28 июл. 2021 г. в 13:57, Vasili Burdo <vasili.burdo@gmail.com>:
>>
>> 5 packets transmitted, 0 received, 100% packet loss, time 5 weeks
>>
>> Thanks.
>> Vasili
>>
>>
>> пн, 12 июл. 2021 г. в 23:51, Vasili Burdo <vasili.burdo@gmail.com>:
>>>
>>> Ping!
>>> 4 packets transmitted, 0 received, 100% packet loss, time 4 weeks
>>>
>>> Thanks.
>>> Vasili
>>>
>>> вт, 6 июл. 2021 г. в 14:18, Vasili Burdo <vasili.burdo@gmail.com>:
>>>>
>>>> Ping!
>>>> 3 packets transmitted, 0 received, 100% packet loss, time 3.5 weeks
>>>>
>>>> Thanks.
>>>> Vasili
>>>>
>>>> пт, 2 июл. 2021 г. в 14:43, Vasili Burdo <vasili.burdo@gmail.com>:
>>>>>
>>>>> Ping!
>>>>> 2 packets transmitted, 0 received, 100% packet loss, time 3 weeks
>>>>>
>>>>> Thanks.
>>>>> Vasili
>>>>>
>>>>> пн, 28 июн. 2021 г. в 17:45, Vasili Burdo <vasili.burdo@gmail.com>:
>>>>>>
>>>>>> Ping!
>>>>>>
>>>>>> вт, 22 июн. 2021 г. в 16:56, Vasili Burdo <vasili.burdo@gmail.com>:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> I've got GDB copyright assignment RT:1731775, so, I'm resending my
>>>>>>> patch - see attachment.
>>>>>>>
>>>>>>> Previous discussion thread and exlanation what's changed is here:
>>>>>>> https://sourceware.org/pipermail/gdb-patches/2021-June/179693.html and
>>>>>>> here: https://sourceware.org/pipermail/gdb-patches/2021-June/179695.html
>>>>>>>
>>>>>>> Changes in current patch:
>>>>>>> - Added changelog entry
>>>>>>> - Tom Tromey's comments are addressed.
>>>>>>> - Disasm window title is kept intact. Reason - everything important is
>>>>>>> already displayed in status line, so, no useful information for title
>>>>>>> remains.
>>>>>>>
>>>>>>> Thanks.
>>>>>>> Vasili

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] TUI disassembly window improvement, take 2
  2021-06-22 13:56 [PATCH] TUI disassembly window improvement, take 2 Vasili Burdo
  2021-06-28 14:45 ` [PATCH] [PING] " Vasili Burdo
@ 2022-01-23 19:53 ` Simon Marchi
  2022-01-23 20:03   ` Simon Marchi
  1 sibling, 1 reply; 13+ messages in thread
From: Simon Marchi @ 2022-01-23 19:53 UTC (permalink / raw)
  To: Vasili Burdo, Vasili Burdo via Gdb-patches



On 2021-06-22 09:56, Vasili Burdo via Gdb-patches wrote:
> Hi,
> 
> I've got GDB copyright assignment RT:1731775, so, I'm resending my
> patch - see attachment.
> 
> Previous discussion thread and exlanation what's changed is here:
> https://sourceware.org/pipermail/gdb-patches/2021-June/179693.html and
> here: https://sourceware.org/pipermail/gdb-patches/2021-June/179695.html
> 
> Changes in current patch:
> - Added changelog entry
> - Tom Tromey's comments are addressed.
> - Disasm window title is kept intact. Reason - everything important is
> already displayed in status line, so, no useful information for title
> remains.
> 
> Thanks.
> Vasili

Hi,

Sorry for the wait again.  Somebody pinged me about this patch because
they wanted to see it merged.  I want it to, I think it's a really good
improvement.  It's just slightly strange now because when going up or
down, one keystroke might mean going up or down two rows, if you get
into the range of another function.  But this is minor, and we can
always iterate on that later.

Just one comment about the code, see below.  If agree with the change, I
can simply apply it and merge the patch, so that we don't need to go
through another iteration.

> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 2b04c3c6d73..dda83efed46 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,10 @@
> +2021-06-22  Vasili Burdo <vasili.burdo@gmail.com>
> +
> +	PR tui/25347
> +	* tui/tui-disasm.c (tui_disassemble,tui_disasm_window::set_content): Hide
> +	the function names in TUI disassembly view. Instead, display them like
> +	assembly language labels.
> +
>  2021-06-22  Andrew Burgess  <andrew.burgess@embecosm.com>
> 
>  	* breakpoint.c (insert_bp_location): If we catch a
> diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
> index 163552aede4..b003c11f7ee 100644
> --- a/gdb/tui/tui-disasm.c
> +++ b/gdb/tui/tui-disasm.c
> @@ -27,6 +27,7 @@
>  #include "value.h"
>  #include "source.h"
>  #include "disasm.h"
> +#include "valprint.h"
>  #include "tui/tui.h"
>  #include "tui/tui-command.h"
>  #include "tui/tui-data.h"
> @@ -50,6 +51,7 @@ struct tui_asm_line
>    std::string addr_string;
>    size_t addr_size;
>    std::string insn;
> +  std::string func_string;

In this iteration, it seems like this field is only set, never read, so
I would remove it.

Simon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] TUI disassembly window improvement, take 2
  2022-01-23 19:53 ` [PATCH] " Simon Marchi
@ 2022-01-23 20:03   ` Simon Marchi
  2022-01-24 14:51     ` Simon Marchi
  0 siblings, 1 reply; 13+ messages in thread
From: Simon Marchi @ 2022-01-23 20:03 UTC (permalink / raw)
  To: Vasili Burdo, Vasili Burdo via Gdb-patches

On 2022-01-23 14:53, Simon Marchi via Gdb-patches wrote:
> 
> 
> On 2021-06-22 09:56, Vasili Burdo via Gdb-patches wrote:
>> Hi,
>>
>> I've got GDB copyright assignment RT:1731775, so, I'm resending my
>> patch - see attachment.
>>
>> Previous discussion thread and exlanation what's changed is here:
>> https://sourceware.org/pipermail/gdb-patches/2021-June/179693.html and
>> here: https://sourceware.org/pipermail/gdb-patches/2021-June/179695.html
>>
>> Changes in current patch:
>> - Added changelog entry
>> - Tom Tromey's comments are addressed.
>> - Disasm window title is kept intact. Reason - everything important is
>> already displayed in status line, so, no useful information for title
>> remains.
>>
>> Thanks.
>> Vasili
> 
> Hi,
> 
> Sorry for the wait again.  Somebody pinged me about this patch because
> they wanted to see it merged.  I want it to, I think it's a really good
> improvement.  It's just slightly strange now because when going up or
> down, one keystroke might mean going up or down two rows, if you get
> into the range of another function.  But this is minor, and we can
> always iterate on that later.
> 
> Just one comment about the code, see below.  If agree with the change, I
> can simply apply it and merge the patch, so that we don't need to go
> through another iteration.

Hmm, actually, running

  make check TESTS="gdb.tui/*.exp"

shows 9 failures (all of gdb.tui/*.exp" passes cleanly here without the
patch).  These tests probably just need to be updated to reflect the new
output, I will look into it.

Simon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] TUI disassembly window improvement, take 2
  2022-01-23 20:03   ` Simon Marchi
@ 2022-01-24 14:51     ` Simon Marchi
  2022-01-24 16:07       ` Vasili Burdo
  0 siblings, 1 reply; 13+ messages in thread
From: Simon Marchi @ 2022-01-24 14:51 UTC (permalink / raw)
  To: Vasili Burdo, Vasili Burdo via Gdb-patches

> Hmm, actually, running
> 
>   make check TESTS="gdb.tui/*.exp"
> 
> shows 9 failures (all of gdb.tui/*.exp" passes cleanly here without the
> patch).  These tests probably just need to be updated to reflect the new
> output, I will look into it.
> 
> Simon

The only test that is tricky to update is this one:

  FAIL: gdb.tui/tui-layout-asm.exp: scroll to end of assembler (scroll failed)

This tests scrolling down in the assembly view by pressing "down" a few
times, expecting the view to scroll down by one line each time.  With
the change in behavior, when we cross the threshold of a symbol, the
view goes down two lines with one keystroke.  We can either make the
test a bit smarter, or try to find a good way to make the view still
always scroll one line at a time, all the time.

I also found this buglet that should probably be fixed.  I start with
the program of gdb.tui/tui-layout-asm.exp, and place a breakpoint on
"main".  The initial view is:

│    main:                                                                                                                                                                                        │
│    0x1119 <+0>     push   %rbp                                                                                                                                                                  │
│    0x111a <+1>     mov    %rsp,%rbp                                                                                                                                                             │
│b+  0x111d <+4>     mov    $0x0,%eax                                                                                                                                                             │
│    0x1122 <+9>     pop    %rbp                                                                                                                                                                  │
│    0x1123 <+10>    ret                                                                                                                                                                          │
│                    cs nopw 0x0(%rax,%rax,1)                                                                                                                                                     │
│                    xchg   %ax,%ax      

Going down twice, it down looks like this:

│b+  main:                                                                                                                                                                                        │
│b+  0x111d <+4>     mov    $0x0,%eax                                                                                                                                                             │
│    0x1122 <+9>     pop    %rbp                                                                                                                                                                  │
│    0x1123 <+10>    ret                                                                                                                                                                          │
│                    cs nopw 0x0(%rax,%rax,1)                                                                                                                                                     │
│                    xchg   %ax,%ax     


Notice the spurious "b+".  I don't think it should be there.

Simon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] TUI disassembly window improvement, take 2
  2022-01-24 14:51     ` Simon Marchi
@ 2022-01-24 16:07       ` Vasili Burdo
  0 siblings, 0 replies; 13+ messages in thread
From: Vasili Burdo @ 2022-01-24 16:07 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Vasili Burdo via Gdb-patches

[-- Attachment #1: Type: text/plain, Size: 4259 bytes --]

Hi, Simon

I'm glad my patch finally got attention.

>The only test that is tricky to update is this one
Yes. My patch adds a "virtual" header line at function start. It skews
scrolling a bit. Currently I see no way to easily fix this.
It's necessary to change function tui_find_disassembly_address() but
my attempts to do it just made scrolling problems worse.

>Notice the spurious "b+".  I don't think it should be there.
New patch version (attached) fixes this.

Thanks.
Vasili

пн, 24 янв. 2022 г. в 17:51, Simon Marchi <simon.marchi@polymtl.ca>:
>
> > Hmm, actually, running
> >
> >   make check TESTS="gdb.tui/*.exp"
> >
> > shows 9 failures (all of gdb.tui/*.exp" passes cleanly here without the
> > patch).  These tests probably just need to be updated to reflect the new
> > output, I will look into it.
> >
> > Simon
>
> The only test that is tricky to update is this one:
>
>   FAIL: gdb.tui/tui-layout-asm.exp: scroll to end of assembler (scroll failed)
>
> This tests scrolling down in the assembly view by pressing "down" a few
> times, expecting the view to scroll down by one line each time.  With
> the change in behavior, when we cross the threshold of a symbol, the
> view goes down two lines with one keystroke.  We can either make the
> test a bit smarter, or try to find a good way to make the view still
> always scroll one line at a time, all the time.
>
> I also found this buglet that should probably be fixed.  I start with
> the program of gdb.tui/tui-layout-asm.exp, and place a breakpoint on
> "main".  The initial view is:
>
> │    main:                                                                                                                                                                                        │
> │    0x1119 <+0>     push   %rbp                                                                                                                                                                  │
> │    0x111a <+1>     mov    %rsp,%rbp                                                                                                                                                             │
> │b+  0x111d <+4>     mov    $0x0,%eax                                                                                                                                                             │
> │    0x1122 <+9>     pop    %rbp                                                                                                                                                                  │
> │    0x1123 <+10>    ret                                                                                                                                                                          │
> │                    cs nopw 0x0(%rax,%rax,1)                                                                                                                                                     │
> │                    xchg   %ax,%ax
>
> Going down twice, it down looks like this:
>
> │b+  main:                                                                                                                                                                                        │
> │b+  0x111d <+4>     mov    $0x0,%eax                                                                                                                                                             │
> │    0x1122 <+9>     pop    %rbp                                                                                                                                                                  │
> │    0x1123 <+10>    ret                                                                                                                                                                          │
> │                    cs nopw 0x0(%rax,%rax,1)                                                                                                                                                     │
> │                    xchg   %ax,%ax
>
>
> Notice the spurious "b+".  I don't think it should be there.
>
> Simon

[-- Attachment #2: vburdo-tui-asm.patch --]
[-- Type: text/x-patch, Size: 4381 bytes --]

diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
index f40d4e2e9f1..be0cc0599c7 100644
--- a/gdb/tui/tui-disasm.c
+++ b/gdb/tui/tui-disasm.c
@@ -27,6 +27,7 @@
 #include "value.h"
 #include "source.h"
 #include "disasm.h"
+#include "valprint.h"
 #include "tui/tui.h"
 #include "tui/tui-command.h"
 #include "tui/tui-data.h"
@@ -92,6 +93,9 @@ len_without_escapes (const std::string &str)
    label into the value pointed to by ADDR_SIZE, and set the addr_size
    field on each item in ASM_LINES, otherwise the addr_size fields within
    ASM_LINES are undefined.
+   FOR_UI boolean flag shows the purpose this function is called for. TRUE
+   means we're going to show ASM_LINES on screen, FALSE means ASM_LINES are
+   used to figure out current scroll position or any other purpose.
 
    It is worth noting that ASM_LINES might not have COUNT entries when this
    function returns.  If the disassembly is truncated for some other
@@ -101,7 +105,8 @@ static CORE_ADDR
 tui_disassemble (struct gdbarch *gdbarch,
 		 std::vector<tui_asm_line> &asm_lines,
 		 CORE_ADDR pc, int count,
-		 size_t *addr_size = nullptr)
+		 size_t *addr_size = nullptr,
+		 bool for_ui = false)
 {
   bool term_out = source_styling && gdb_stdout->can_emit_style_escape ();
   string_file gdb_dis_out (term_out);
@@ -110,7 +115,7 @@ tui_disassemble (struct gdbarch *gdbarch,
   asm_lines.clear ();
 
   /* Now construct each line.  */
-  for (int i = 0; i < count; ++i)
+  while (asm_lines.size () < count)
     {
       tui_asm_line tal;
       CORE_ADDR orig_pc = pc;
@@ -135,8 +140,51 @@ tui_disassemble (struct gdbarch *gdbarch,
 
       /* And capture the address the instruction is at.  */
       tal.addr = orig_pc;
-      print_address (gdbarch, orig_pc, &gdb_dis_out);
-      tal.addr_string = std::move (gdb_dis_out.string ());
+      if (for_ui)
+        { /* If we're rendering for UI... */
+          std::string name, filename;
+          int unmapped = 0, offset = 0, line = 0;
+
+          if (0 == build_address_symbolic (gdbarch, orig_pc, asm_demangle,
+                false, &name, &offset, &filename, &line, &unmapped))
+            {
+              if (0 == offset || asm_lines.empty ())
+                {/* "offset == 0" means function start,
+                    "asm_lines.empty ()" means top line of output.
+                    For these cases, insert "function header" on top of
+                    "real" disassembly line. */
+                  tui_asm_line tal2 = tal;
+                  fputs_styled (name.c_str (), function_name_style.style (),
+                                &gdb_dis_out);
+                  fputs_filtered (":", &gdb_dis_out);
+                  tal2.addr = -1;
+                  tal2.addr_string = std::move (gdb_dis_out.string ());
+                  tal2.addr_size = 0;
+                  tal2.insn.clear ();
+                  gdb_dis_out.clear ();
+
+                  asm_lines.push_back (std::move (tal2));
+                }
+
+              fputs_styled (paddress (gdbarch, orig_pc),
+                            address_style.style (), &gdb_dis_out);
+              fputs_filtered (" <", &gdb_dis_out);
+              if (unmapped)
+                fputs_filtered ("*", &gdb_dis_out);
+              fprintf_styled (&gdb_dis_out, address_style.style (),
+                              "%+d", offset);
+              if (unmapped)
+                fputs_filtered ("*", &gdb_dis_out);
+              fputs_filtered (">", &gdb_dis_out);
+              tal.addr_string = std::move (gdb_dis_out.string ());
+            }
+        }
+      else
+        { /* If we're rendering for some other purpose than UI...
+             Fall back to legacy disassembly line style. */
+          print_address (gdbarch, orig_pc, &gdb_dis_out);
+          tal.addr_string = std::move (gdb_dis_out.string ());
+        }
       gdb_dis_out.clear ();
 
       if (addr_size != nullptr)
@@ -342,7 +390,7 @@ tui_disasm_window::set_contents (struct gdbarch *arch,
   /* Get temporary table that will hold all strings (addr & insn).  */
   std::vector<tui_asm_line> asm_lines;
   size_t addr_size = 0;
-  tui_disassemble (m_gdbarch, asm_lines, pc, max_lines, &addr_size);
+  tui_disassemble (m_gdbarch, asm_lines, pc, max_lines, &addr_size, true);
 
   /* Align instructions to the same column.  */
   insn_pos = (1 + (addr_size / tab_len)) * tab_len;

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-01-24 16:07 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-22 13:56 [PATCH] TUI disassembly window improvement, take 2 Vasili Burdo
2021-06-28 14:45 ` [PATCH] [PING] " Vasili Burdo
2021-07-02 11:43   ` [PATCH] [PING #2] " Vasili Burdo
2021-07-06 11:18     ` [PATCH] [PING #3] " Vasili Burdo
2021-07-12 20:51       ` [PING][#4][PATCH] " Vasili Burdo
2021-07-16 11:25         ` [PING][#5][PATCH] " Vasili Burdo
2021-07-28 10:57         ` Vasili Burdo
2021-08-05 10:46           ` [PING][#6][PATCH] " Vasili Burdo
2021-08-05 14:40             ` Simon Marchi
2022-01-23 19:53 ` [PATCH] " Simon Marchi
2022-01-23 20:03   ` Simon Marchi
2022-01-24 14:51     ` Simon Marchi
2022-01-24 16:07       ` Vasili Burdo

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