From patchwork Tue Aug 22 16:00:38 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 22309 Received: (qmail 98798 invoked by alias); 22 Aug 2017 16:00:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 98610 invoked by uid 89); 22 Aug 2017 16:00:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No 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, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 22 Aug 2017 16:00:44 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4D5E680B2A; Tue, 22 Aug 2017 16:00:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 4D5E680B2A Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves@redhat.com Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7F69917AE2; Tue, 22 Aug 2017 16:00:39 +0000 (UTC) Subject: Re: [PATCH v1 1/1] list actual code around more than one locations To: Zhouyi Zhou , gdb-patches@sourceware.org References: <1500437120-3473-1-git-send-email-zhouzhouyi@gmail.com> From: Pedro Alves Message-ID: <968c6de4-789d-25f8-7bfc-71a810e26f17@redhat.com> Date: Tue, 22 Aug 2017 17:00:38 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1500437120-3473-1-git-send-email-zhouzhouyi@gmail.com> Hi, Thanks for the patch! I pushed it in with a couple minor tweaks. See below. On 07/19/2017 05:05 AM, Zhouyi Zhou wrote: > else if (no_end) > { > int first_line = sal.line - get_lines_to_list () / 2; > + int i; > > if (first_line < 1) first_line = 1; > + > + if (sals.nelts > 1) > + { > + printf_filtered (_("file: \"%s\", line number: %d\n"), > + symtab_to_filename_for_display (sal.symtab), > + sal.line); > + } > > print_source_lines (sal.symtab, > first_line, > first_line + get_lines_to_list (), > 0); > + if (sals.nelts > 1) > + { > + for (i = 1; i < sals.nelts; i++) > + { > + sal = sals.sals[i]; > + first_line = sal.line - get_lines_to_list () / 2; > + if (first_line < 1) first_line = 1; > + printf_filtered (_("file: \"%s\", line number: %d\n"), > + symtab_to_filename_for_display (sal.symtab), > + sal.line); > + print_source_lines (sal.symtab, > + first_line, > + first_line + get_lines_to_list (), > + 0); > + } We can avoid the nelts == 1 and nelts > 1 duplication by simply letting the nelts == 1 case be handled by the loop as well. > + } > } > else > print_source_lines (sal.symtab, sal.line, > @@ -1120,6 +1150,8 @@ list_command (char *arg, int from_tty) > ? sal.line + get_lines_to_list () > : sal_end.line + 1), > 0); > + if (sals.nelts) > + xfree (sals.sals); Also fixed spaces vs tabs throughout, and initialized sals.sals to avoid all the 'if (sals.nelts)'. Note that only really old code uses explicit xfree calls on exits paths like these, since they're not exception-safe in case some code between the xmalloc and the xfree throws an exception. Before GDB's C++ification, that used to be solved with cleanups (grep for make_cleanup). However, we're trying to get rid of cleanups nowadays, in favor of C++ RAII. That's why I sent in this series yesterday: https://sourceware.org/ml/gdb-patches/2017-08/msg00409.html However, since this "list" code was already calling xfree explicitly like that, I though that putting in the patch as is isn't really making things worse. Also, you don't seen to have a a copyright assignment for GDB on file. You have one for GCC, but that doesn't cover GDB. The patch is borderline small enough to push in without a GDB assignment, IMO, particularly if we consider that the boilerplace xfree calls will be removed soon enough. In order to be able accept further patches from you, we'll need that sorted out, though. Please feel free to contact me off list if you need assistance with that. Thanks again for your contribution! Pedro Alves From 0d999a6ef0f98b22430d70951408869864c979e0 Mon Sep 17 00:00:00 2001 From: Zhouyi Zhou Date: Tue, 22 Aug 2017 15:32:19 +0100 Subject: [PATCH] List actual code around more than one location With the following C++ code: int bar() { return 0;} int bar(int) { return 0; } GDB behaves as: (gdb) list bar file: "overload.cc", line number: 1 file: "overload.cc", line number: 2 It would be better for GDB to list the actual code around those two locations, not just print the location. Like: (gdb) list bar file: "overload.cc", line number: 1 1 int bar() { return 0;} 2 int bar(int) { return 0; } file: "overload.cc", line number: 2 1 int bar() { return 0;} 2 int bar(int) { return 0; } That's what this this commit implements. Tested on x86-64 GNU/Linux. gdb/ChangeLog: 2017-08-22 Zhouyi Zhou * cli-cmds.c (list_commands): List actual code around more than one location. --- gdb/ChangeLog | 5 +++++ gdb/cli/cli-cmds.c | 47 +++++++++++++++++++++++++++++++---------------- 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 74506f8..0908b99 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2017-08-22 Zhouyi Zhou + + * cli-cmds.c (list_commands): List actual code around more than + one location. + 2017-08-21 John Baldwin * fbsd-nat.c (fbsd_add_threads): Use array type for `lwps'. diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 3fa2499..d4dc539 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -955,6 +955,8 @@ list_command (char *arg, int from_tty) if (!have_full_symbols () && !have_partial_symbols ()) error (_("No symbol table is loaded. Use the \"file\" command.")); + sals.nelts = 0; + sals.sals = NULL; arg1 = arg; if (*arg1 == ',') dummy_beg = 1; @@ -971,15 +973,8 @@ list_command (char *arg, int from_tty) /* C++ */ return; } - if (sals.nelts > 1) - { - ambiguous_line_spec (&sals); - xfree (sals.sals); - return; - } sal = sals.sals[0]; - xfree (sals.sals); } /* Record whether the BEG arg is all digits. */ @@ -992,6 +987,12 @@ list_command (char *arg, int from_tty) if (*arg1 == ',') { no_end = 0; + if (sals.nelts > 1) + { + ambiguous_line_spec (&sals); + xfree (sals.sals); + return; + } arg1++; while (*arg1 == ' ' || *arg1 == '\t') arg1++; @@ -1010,11 +1011,15 @@ list_command (char *arg, int from_tty) filter_sals (&sals_end); if (sals_end.nelts == 0) - return; + { + xfree (sals.sals); + return; + } if (sals_end.nelts > 1) { ambiguous_line_spec (&sals_end); xfree (sals_end.sals); + xfree (sals.sals); return; } sal_end = sals_end.sals[0]; @@ -1080,14 +1085,23 @@ list_command (char *arg, int from_tty) error (_("No default source file yet. Do \"help list\".")); else if (no_end) { - int first_line = sal.line - get_lines_to_list () / 2; - - if (first_line < 1) first_line = 1; - - print_source_lines (sal.symtab, - first_line, - first_line + get_lines_to_list (), - 0); + for (int i = 0; i < sals.nelts; i++) + { + sal = sals.sals[i]; + int first_line = sal.line - get_lines_to_list () / 2; + if (first_line < 1) + first_line = 1; + if (sals.nelts > 1) + { + printf_filtered (_("file: \"%s\", line number: %d\n"), + symtab_to_filename_for_display (sal.symtab), + sal.line); + } + print_source_lines (sal.symtab, + first_line, + first_line + get_lines_to_list (), + 0); + } } else print_source_lines (sal.symtab, sal.line, @@ -1095,6 +1109,7 @@ list_command (char *arg, int from_tty) ? sal.line + get_lines_to_list () : sal_end.line + 1), 0); + xfree (sals.sals); } /* Subroutine of disassemble_command to simplify it.