This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Disassemble over end of line table sequence.
- From: "Andrew Burgess" <aburgess at broadcom dot com>
- To: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Tue, 18 Jan 2011 11:50:37 +0000
- Subject: Re: [PATCH] Disassemble over end of line table sequence.
- References: <4D08CF76.1000809@broadcom.com> <20110114160919.GP2504@adacore.com>
On 14/01/2011 16:09, Joel Brobecker wrote:
> Andrew,
>
> Note that ever line in the CL entry needs to be aligned - I can't remember
> if it was you already told about this. I may have, this patch is from
> 4 weeks ago...
Sorry about that, I made this patch before I was told about this, I should have refreshed the patch before ping-ing it.
>
>> gdb/testsuite/
>>
>> 2010-12-10 Andrew Burgess<aburgess@broadcom.com>
>>
>> * gdb.disasm/disasm-end-cu-1.c, gdb.disasm/disasm-end-cu-2.c,
>> gdb.disasm/disasm-end-cu.exp: New test for disassembling
>> over the boundary between two compilation units.
>
> I think you got mislead a bit by the directory name when choosing
> the directory where to put your testcase. This directory contains
> testcase that use files with assembly code, and thus limited to
> selected architectures. Yours uses plain C files, which can be compiled
> on all hosts. So, let's move your testcase to gdb.base.
Moved.
>> + /* Work with end of sequence markers
>> + where the line number is set to 0 */
>
> The lines are too short - please join them into 1 (actually, the guideline
> is 70 characters, and that means it doesn't fit, but let's make the first
> line a little longer, which is a more natural length). Also, the GNU Coding
> Standards (GCS) require that we put a period at the end of every sentence,
> and periods are followed by 2 spaces. Thus:
>
> /* Work with end of sequence markers where the line number is set
> to 0. */
Comment now says something a little better, and is wrapped correctly.
>> + if ( mle1->line == 0 || mle2->line == 0 )
>
> Formatting, no space after '(' and before ')':
>
> if (mle1->line == 0 || mle2->line == 0)
>
>> + if ( val == 0 )
>
> Same here.
Fixed.
>
>> +++ b/gdb/testsuite/gdb.disasm/disasm-end-cu-1.c
>> @@ -0,0 +1,14 @@
>> +#include<stdio.h>
>
> This file needs a copyright header.
>
>> +++ b/gdb/testsuite/gdb.disasm/disasm-end-cu-2.c
>> @@ -0,0 +1,13 @@
>> +#include<stdio.h>
>
> Same here.
>
>> +++ b/gdb/testsuite/gdb.disasm/disasm-end-cu.exp
>> @@ -0,0 +1,70 @@
>> +# Copyright 2010 Free Software Foundation, Inc.
>
> Can you add 2011? (And a `(C)' - this is not strictly mandated by the FSF,
> but the script I'm planning on using to perform yearly updates requires it).
All fixed.
>
>> +# This test can only be run on targets which support DWARF-2 and use gas.
>> +# For now pick a sampling of likely targets.
>> +if {![istarget *-*-linux*]
>> +&& ![istarget *-*-gnu*]
>> +&& ![istarget *-*-elf*]
>> +&& ![istarget *-*-openbsd*]
>> +&& ![istarget arm-*-eabi*]
>> +&& ![istarget powerpc-*-eabi*]} {
>> + return 0
>
> This is not necessary in your case.
>
>> +send_gdb "disassemble /m ${main_addr},${dummy_3_addr}\n"
>> +gdb_expect {
>> + -re "Dump of assembler code from ${main_addr} to
>
> We should not be using gdb_send/gdb_expect. Can you use gdb_test_multiple
> instead?
>
>> +gdb_stop_suppressing_tests;
>
> You can remove this line.
... and fixed.
Thanks for the review.
Andrew
gdb/ChangeLog
2011-01-18 Andrew Burgess <aburgess@broadcom.com>
* disasm.c (compare_lines): Handle the end of sequence markers
within the line table to better support disassembling over
compilation unit boundaries.
gdb/testsuite/ChangeLog
2011-01-18 Andrew Burgess <aburgess@broadcom.com>
* gdb.base/disasm-end-cu-1.c, gdb.base/disasm-end-cu-2.c,
gdb.base/disasm-end-cu.exp: New test for disassembling over the
boundary between two compilation units.
diff --git a/gdb/disasm.c b/gdb/disasm.c
index c51f0bf..f2428b5 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -77,12 +77,22 @@ compare_lines (const void *mle1p, const void *mle2p)
mle1 = (struct dis_line_entry *) mle1p;
mle2 = (struct dis_line_entry *) mle2p;
- val = mle1->line - mle2->line;
-
- if (val != 0)
- return val;
+ /* End of sequence markers have a line number of 0 but don't want to
+ be sorted to the head of the list, instead sort by PC. */
+ if (mle1->line == 0 || mle2->line == 0)
+ {
+ val = mle1->start_pc - mle2->start_pc;
+ if (val == 0)
+ val = mle1->line - mle2->line;
+ }
+ else
+ {
+ val = mle1->line - mle2->line;
+ if (val == 0)
+ val = mle1->start_pc - mle2->start_pc;
+ }
- return mle1->start_pc - mle2->start_pc;
+ return val;
}
static int
diff --git a/gdb/testsuite/gdb.base/disasm-end-cu-1.c b/gdb/testsuite/gdb.base/disasm-end-cu-1.c
new file mode 100644
index 0000000..ea7b1a3
--- /dev/null
+++ b/gdb/testsuite/gdb.base/disasm-end-cu-1.c
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright (C) 2010, 2011 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include <stdio.h>
+
+void
+dummy_1 ()
+{
+ printf ("In dummy_1 ()\n");
+}
+
+int
+main ()
+{
+ printf ("Hello World!\n");
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/disasm-end-cu-2.c b/gdb/testsuite/gdb.base/disasm-end-cu-2.c
new file mode 100644
index 0000000..f4b6152
--- /dev/null
+++ b/gdb/testsuite/gdb.base/disasm-end-cu-2.c
@@ -0,0 +1,30 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright (C) 2010, 2011 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include <stdio.h>
+
+void
+dummy_2 ()
+{
+ printf ("In dummy_2 ()\n");
+}
+
+void
+dummy_3 ()
+{
+ printf ("In dummy_3 ()\n");
+}
diff --git a/gdb/testsuite/gdb.base/disasm-end-cu.exp b/gdb/testsuite/gdb.base/disasm-end-cu.exp
new file mode 100644
index 0000000..4a145f8
--- /dev/null
+++ b/gdb/testsuite/gdb.base/disasm-end-cu.exp
@@ -0,0 +1,52 @@
+# Copyright (C) 2010, 2011 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# This test tries to disassemble over the boundary between two compilation
+# units displaying source lines. This checks that the disassemble routine
+# can handle our use of line number 0 to mark the end of sequence.
+
+if { [prepare_for_testing disasm-end-cu.exp "disasm-end-cu" {disasm-end-cu-1.c disasm-end-cu-2.c} {debug}] } {
+ return -1
+}
+
+if ![runto_main] {
+ return -1
+}
+
+set main_addr [get_hexadecimal_valueof "&main" "0"]
+set dummy_3_addr [get_hexadecimal_valueof "&dummy_3" "0"]
+
+if {$main_addr == 0 || $dummy_3_addr == 0 || $dummy_3_addr <= $main_addr} {
+ fail "Unable to extract required addresses, or addresses out of order"
+ return -1
+}
+
+gdb_test_multiple "disassemble /m ${main_addr},${dummy_3_addr}" "Disassemble address range with source" {
+ -re "Dump of assembler code from ${main_addr} to ${dummy_3_addr}:\r\nEnd of assembler dump\." {
+ fail "No output from the disassemble command"
+ }
+ -re "Line number 0 out of range;.* has $decimal lines\." {
+ fail "The disassemble command failed"
+ }
+ -re "Dump of assembler code from ${main_addr} to ${dummy_3_addr}:\r\n.*main.*End of assembler dump\." {
+ pass "disassemble command returned some output"
+ }
+ -re ".*$gdb_prompt $" {
+ fail "Unexpected output from disassemble command"
+ }
+ timeout {
+ fail "(timeout) waiting for output of disassemble command"
+ }
+}