[PATCH] Disassemble over end of line table sequence.
Andrew Burgess
aburgess@broadcom.com
Tue Jan 25 17:45:00 GMT 2011
Ping! Having addressed review comments am I ok to commit?
Thanks,
Andrew
On 18/01/2011 11:50, Andrew Burgess wrote:
> 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"
> + }
> +}
>
>
>
>
>
>
>
>
>
>
More information about the Gdb-patches
mailing list