From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from barracuda.ebox.ca (barracuda.ebox.ca [96.127.255.19]) by sourceware.org (Postfix) with ESMTPS id EECD63858404 for ; Mon, 24 Jan 2022 19:28:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EECD63858404 X-ASG-Debug-ID: 1643052499-0c856e06ab205720001-fS2M51 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id axQgXuRVNtWsJbD6 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 24 Jan 2022 14:28:19 -0500 (EST) X-Barracuda-Envelope-From: simon.marchi@polymtl.ca X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from simark.localdomain (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) by smtp.ebox.ca (Postfix) with ESMTP id 45683441B21; Mon, 24 Jan 2022 14:28:19 -0500 (EST) From: Simon Marchi X-Barracuda-RBL-IP: 192.222.157.6 X-Barracuda-Effective-Source-IP: 192-222-157-6.qc.cable.ebox.net[192.222.157.6] X-Barracuda-Apparent-Source-IP: 192.222.157.6 To: gdb-patches@sourceware.org Cc: Vasili Burdo , Simon Marchi Subject: [PATCH v4] gdb/tui/disassembly view: make symbol name appear on a line of its own Date: Mon, 24 Jan 2022 14:28:11 -0500 X-ASG-Orig-Subj: [PATCH v4] gdb/tui/disassembly view: make symbol name appear on a line of its own Message-Id: <20220124192811.1530670-1-simon.marchi@polymtl.ca> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Barracuda-Connect: smtp.ebox.ca[96.127.255.82] X-Barracuda-Start-Time: 1643052499 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Barracuda-BRTS-Status: 1 X-Virus-Scanned: by bsmtpd at ebox.ca X-Barracuda-Scan-Msg-Size: 13707 X-Barracuda-Spam-Score: 0.00 X-Barracuda-Spam-Status: No, SCORE=0.00 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=8.0 tests= X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.95547 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- X-Spam-Status: No, score=-3612.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_QUARANTINE, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_VALIDITY_RPBL, SPF_HELO_NONE, SPF_SOFTFAIL, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 24 Jan 2022 19:28:29 -0000 From: Vasili Burdo Hi Vasili, Here's a new version of your patch [1], I did the following changes: - Updated gdb.ui tests. - Renamed tal2 to tal_header. - Omitted copying tal to tal2 (tal_header), since we overwrite all the fields anyway. - General formatting. - Added commit message (see below) Since the patch is still in your name, please let me know if that is OK with you. [1] https://sourceware.org/pipermail/gdb-patches/2022-January/185399.html ~~~ When debbugging C++ programs, symbols tend to be quite long. This makes the TUI's disassembly view difficult to use, as each line includes the function's symbol name, which can leave little or no room for the instructions. For example: │ 0x7ffff4037b27 <_Z22read_src_fs_parametersPK8bt_valuePS1_S2_P16ctf_fs_componentP17bt_self_componentP23bt_self_component_class+1390> movb $0x1,-0xa│ │ 0x7ffff4037b2e <_Z22read_src_fs_parametersPK8bt_valuePS1_S2_P16ctf_fs_componentP17bt_self_componentP23bt_self_component_class+1397> lea -0x40(%rb│ │ 0x7ffff4037b32 <_Z22read_src_fs_parametersPK8bt_valuePS1_S2_P16ctf_fs_componentP17bt_self_componentP23bt_self_component_class+1401> mov %rax,%rdx│ │ 0x7ffff4037b35 <_Z22read_src_fs_parametersPK8bt_valuePS1_S2_P16ctf_fs_componentP17bt_self_componentP23bt_self_component_class+1404> shr $0x3,%rdx│ To address this, stop including the symbol name with the offset on each line. Instead, put it on a line of its own just before the address is points to. Because it would be possible for the current symbol name to be outside the window, and therefore it would be difficult for the user to know what function they are currently stepping into, make it so the first symbol name always "sticks" to the top of the window. The instructions from the example above would now look like this: │ _Z22read_src_fs_parametersPK8bt_valuePS1_S2_P16ctf_fs_componentP17bt_self_componentP23bt_self_component_class: │ │ 0x7ffff4037b27 <+1390> movb $0x1,-0xa1(%rbp) │ │ 0x7ffff4037b2e <+1397> lea -0x40(%rbx),%rax │ │ 0x7ffff4037b32 <+1401> mov %rax,%rdx │ Update the gdb.tui tests to match the new output. The changes are all trivial except the scrolling test in gdb.tui/tui-layout-asm.exp. The new behavior means that sometimes, when scrolling down, two lines will disappear at once instead of a single line, when the last instruction mapped to a symbol goes out of view. Update the test to account for that. Change-Id: I9cf46c094058b9382087b54d212f22265b1fc324 Co-Authored-By: Simon Marchi --- gdb/testsuite/gdb.tui/basic.exp | 4 +- gdb/testsuite/gdb.tui/list.exp | 2 +- gdb/testsuite/gdb.tui/new-layout.exp | 4 +- .../gdb.tui/tui-layout-asm-short-prog.exp | 5 +- gdb/testsuite/gdb.tui/tui-layout-asm.exp | 18 +++--- gdb/tui/tui-disasm.c | 62 +++++++++++++++++-- 6 files changed, 76 insertions(+), 19 deletions(-) diff --git a/gdb/testsuite/gdb.tui/basic.exp b/gdb/testsuite/gdb.tui/basic.exp index 0e9f2e3eab28..82fec97319a1 100644 --- a/gdb/testsuite/gdb.tui/basic.exp +++ b/gdb/testsuite/gdb.tui/basic.exp @@ -96,13 +96,13 @@ if {[Term::wait_for $regexp] \ Term::check_box "source box" 0 0 80 15 Term::command "layout asm" -Term::check_contents "asm window shows main" "$hex
" +Term::check_contents "asm window shows main" " main:" Term::check_box "asm box" 0 0 80 15 Term::command "layout split" Term::check_contents "split layout contents" \ - "$main_line *$main_re.*$hex
" + "$main_line *$main_re.* main:" Term::check_box "source box in split layout" 0 0 80 7 Term::check_box "asm box in split layout" 0 6 80 9 diff --git a/gdb/testsuite/gdb.tui/list.exp b/gdb/testsuite/gdb.tui/list.exp index d153052cfd77..00854097a2d3 100644 --- a/gdb/testsuite/gdb.tui/list.exp +++ b/gdb/testsuite/gdb.tui/list.exp @@ -32,7 +32,7 @@ if {![Term::enter_tui]} { Term::check_contents "initial source listing" "21 *return 0" Term::command "layout asm" -Term::check_contents "asm window shows main" "$hex
" +Term::check_contents "asm window shows main" " main:" Term::command "list -q main" Term::check_contents "list -q main" "21 *return 0" diff --git a/gdb/testsuite/gdb.tui/new-layout.exp b/gdb/testsuite/gdb.tui/new-layout.exp index 2b5e07db612c..f64b11c6ea44 100644 --- a/gdb/testsuite/gdb.tui/new-layout.exp +++ b/gdb/testsuite/gdb.tui/new-layout.exp @@ -81,13 +81,13 @@ gdb_assert {![string match "No Source Available" $text]} \ Term::command "layout example" Term::check_contents "example layout shows assembly" \ - "$hex
" + " main:" Term::command "layout h" Term::check_box "left window box" 0 0 40 15 Term::check_box "right window box" 39 0 41 15 Term::check_contents "horizontal display" \ - "$hex
.*21.*return 0" + " main:.*21.*return 0" Term::command "winheight src - 5" Term::check_box "left window box after shrink" 0 0 40 10 diff --git a/gdb/testsuite/gdb.tui/tui-layout-asm-short-prog.exp b/gdb/testsuite/gdb.tui/tui-layout-asm-short-prog.exp index a6be799113df..6f43b4cac73c 100644 --- a/gdb/testsuite/gdb.tui/tui-layout-asm-short-prog.exp +++ b/gdb/testsuite/gdb.tui/tui-layout-asm-short-prog.exp @@ -34,7 +34,7 @@ if {![Term::prepare_for_tui]} { # This puts us into TUI mode, and should display the ASM window. Term::command_no_prompt_prefix "layout asm" -Term::check_box_contents "check asm box contents" 0 0 80 15 "<_start>" +Term::check_box_contents "check asm box contents" 0 0 80 15 " _start:" # Record the first line of output, we'll need this later. set first_line [Term::get_line 1] @@ -44,7 +44,8 @@ set first_line [Term::get_line 1] Term::command "+ 13" Term::check_box_contents "check asm box contents again" 0 0 80 15 \ [multi_line \ - "^ *$hex\[^\r\n\]+" \ + " *_start: *" \ + " *$hex\[^\r\n\]+" \ "\\s+"] # Now scroll backward again, we should return to the start of the diff --git a/gdb/testsuite/gdb.tui/tui-layout-asm.exp b/gdb/testsuite/gdb.tui/tui-layout-asm.exp index a76e637b4c93..6eda03d922f4 100644 --- a/gdb/testsuite/gdb.tui/tui-layout-asm.exp +++ b/gdb/testsuite/gdb.tui/tui-layout-asm.exp @@ -40,7 +40,7 @@ proc count_whitespace { string } { # This puts us into TUI mode, and should display the ASM window. Term::command_no_prompt_prefix "layout asm" -Term::check_box_contents "check asm box contents" 0 0 ${tui_asm_window_width} 15 "
" +Term::check_box_contents "check asm box contents" 0 0 ${tui_asm_window_width} 15 " main:" # Scroll the ASM window down using the down arrow key. In an ideal # world we'd like to use PageDown here, but currently our terminal @@ -48,10 +48,13 @@ Term::check_box_contents "check asm box contents" 0 0 ${tui_asm_window_width} 15 set testname "scroll to end of assembler" set down_count 0 while (1) { - # Grab the second line, this is about to become the first line. - set line [Term::get_line 2] + # Grab the third line, this is about to become either the first or the + # second line. If the last instruction of a function goes out of the window + # the scroll will make two lines disappear, the line with the symbol name and + # the line with the instruction. + set line [Term::get_line 3] - # Except, if the second line is blank then we are at the end of + # Except, if the third line is blank then we are at the end of # the available asm output. Pressing down again _shouldn't_ # change the output, however, if GDB is working, and we press down # then the screen won't change, so the call to Term::wait_for @@ -70,11 +73,12 @@ while (1) { # Ignore whitespace mismatches. regsub -all {\s+} $re_line {\s+} re_line if {[Term::wait_for $re_line] \ - && [regexp $re_line [Term::get_line 1]]} { + && ([regexp $re_line [Term::get_line 1]] + || [regexp $re_line [Term::get_line 2]])} { # We scrolled successfully. } else { if {[count_whitespace ${line}] != \ - [count_whitespace [Term::get_line 1]]} { + [count_whitespace [Term::get_line 2]]} { # GDB's TUI assembler display will widen columns based on # the longest item that appears in a column on any line. # As we have just scrolled, and so revealed a new line, it @@ -93,7 +97,7 @@ while (1) { verbose -log " details." } - fail "$testname (scroll failed)" + fail "$testname (scroll failed, down_count=$down_count)" Term::dump_screen break } diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c index f40d4e2e9f1c..027231e466b1 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" @@ -93,6 +94,10 @@ len_without_escapes (const std::string &str) 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 reason, for example, we hit invalid memory, then ASM_LINES can have @@ -101,7 +106,8 @@ static CORE_ADDR tui_disassemble (struct gdbarch *gdbarch, std::vector &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,54 @@ 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 (build_address_symbolic (gdbarch, orig_pc, asm_demangle, + false, &name, &offset, &filename, &line, &unmapped) == 0) + { + if (offset == 0 || 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. */ + fputs_styled (name.c_str (), function_name_style.style (), + &gdb_dis_out); + fputs_filtered (":", &gdb_dis_out); + + tui_asm_line tal_header; + tal_header.addr = -1; + tal_header.addr_string = std::move (gdb_dis_out.string ()); + tal_header.addr_size = 0; + tal_header.insn.clear (); + asm_lines.push_back (std::move (tal_header)); + + gdb_dis_out.clear (); + } + + 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 +394,7 @@ tui_disasm_window::set_contents (struct gdbarch *arch, /* Get temporary table that will hold all strings (addr & insn). */ std::vector 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; base-commit: 2f279a64a27bca5be4393e84cbb579e18ef4a4ad -- 2.34.1