[PATCH] Disassemble over end of line table sequence.

[PATCH] Disassemble over end of line table sequence.

[Andrew Burgess]

Within gdb we use line number 0 to mark the end of a sequence within the 
line number tables. When disassembling with source code we sort the 
lines based on the line number. As a result if a request is made to 
disassemble a range of addresses that includes an end of sequence marker 
the disassemble command will give an error rather than any sensible output.

In the included test I request a disassembly for a range of addresses 
that spans a function boundary, currently this will fail. With the fix 
the disassembly I get back still does not quite cover the complete range 
of addresses requested, I am treating this problem as a separate issue 
to be fixed in a later patch, here I am only interested in ordering the 
lines correctly so that I get something sensible back rather than an error.

Thanks,

Andrew



gdb/

2010-12-10  Andrew Burgess  <aburgess@broadcom.com>

           * (compare_lines): Sort by pc for entries where the line
             number is 0, these are the end of sequence markers.

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.


diff --git a/gdb/disasm.c b/gdb/disasm.c
index 9fa34a0..2259f3e 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;
+  /* Work with end of sequence markers
+     where the line number is set to 0 */
+  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.disasm/disasm-end-cu-1.c 
b/gdb/testsuite/gdb.disasm/disasm-end-cu-1.c
new file mode 100644
index 0000000..c031778
--- /dev/null
+++ b/gdb/testsuite/gdb.disasm/disasm-end-cu-1.c
@@ -0,0 +1,14 @@
+#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.disasm/disasm-end-cu-2.c 
b/gdb/testsuite/gdb.disasm/disasm-end-cu-2.c
new file mode 100644
index 0000000..7fddb87
--- /dev/null
+++ b/gdb/testsuite/gdb.disasm/disasm-end-cu-2.c
@@ -0,0 +1,13 @@
+#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.disasm/disasm-end-cu.exp 
b/gdb/testsuite/gdb.disasm/disasm-end-cu.exp
new file mode 100644
index 0000000..fdbd179
--- /dev/null
+++ b/gdb/testsuite/gdb.disasm/disasm-end-cu.exp
@@ -0,0 +1,70 @@
+# Copyright 2010 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 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 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
+}
+
+send_gdb "disassemble /m ${main_addr},${dummy_3_addr}\n"
+gdb_expect {
+    -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"
+    }
+}
+
+gdb_stop_suppressing_tests;

Re: [PATCH] Disassemble over end of line table sequence.

[Andrew Burgess]

Ping!

Could someone cast an eye over this please.

Thanks,

Andrew


On 15/12/2010 14:23, Andrew Burgess wrote:
> Within gdb we use line number 0 to mark the end of a sequence within the
> line number tables. When disassembling with source code we sort the
> lines based on the line number. As a result if a request is made to
> disassemble a range of addresses that includes an end of sequence marker
> the disassemble command will give an error rather than any sensible output.
>
> In the included test I request a disassembly for a range of addresses
> that spans a function boundary, currently this will fail. With the fix
> the disassembly I get back still does not quite cover the complete range
> of addresses requested, I am treating this problem as a separate issue
> to be fixed in a later patch, here I am only interested in ordering the
> lines correctly so that I get something sensible back rather than an error.
>
> Thanks,
>
> Andrew
>
>
>
> gdb/
>
> 2010-12-10  Andrew Burgess<aburgess@broadcom.com>
>
>             * (compare_lines): Sort by pc for entries where the line
>               number is 0, these are the end of sequence markers.
>
> 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.
>
>
> diff --git a/gdb/disasm.c b/gdb/disasm.c
> index 9fa34a0..2259f3e 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;
> +  /* Work with end of sequence markers
> +     where the line number is set to 0 */
> +  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.disasm/disasm-end-cu-1.c
> b/gdb/testsuite/gdb.disasm/disasm-end-cu-1.c
> new file mode 100644
> index 0000000..c031778
> --- /dev/null
> +++ b/gdb/testsuite/gdb.disasm/disasm-end-cu-1.c
> @@ -0,0 +1,14 @@
> +#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.disasm/disasm-end-cu-2.c
> b/gdb/testsuite/gdb.disasm/disasm-end-cu-2.c
> new file mode 100644
> index 0000000..7fddb87
> --- /dev/null
> +++ b/gdb/testsuite/gdb.disasm/disasm-end-cu-2.c
> @@ -0,0 +1,13 @@
> +#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.disasm/disasm-end-cu.exp
> b/gdb/testsuite/gdb.disasm/disasm-end-cu.exp
> new file mode 100644
> index 0000000..fdbd179
> --- /dev/null
> +++ b/gdb/testsuite/gdb.disasm/disasm-end-cu.exp
> @@ -0,0 +1,70 @@
> +# Copyright 2010 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 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 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
> +}
> +
> +send_gdb "disassemble /m ${main_addr},${dummy_3_addr}\n"
> +gdb_expect {
> +    -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"
> +    }
> +}
> +
> +gdb_stop_suppressing_tests;
>
>
>

Re: [PATCH] Disassemble over end of line table sequence.

[Joel Brobecker]

Andrew,

First, I apologize for the delay in reviewing your patch.  Looks like
we've all been very busy the last few weeks.

> 2010-12-10  Andrew Burgess  <aburgess@broadcom.com>
> 
>           * (compare_lines): Sort by pc for entries where the line
>             number is 0, these are the end of sequence markers.

I think that the patch is correct in principle, there are are a few nits
that need to be fixed before it can go in.  See below.

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

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

> +  /* 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. */

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

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

> +# 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.

-- 
Joel

Re: [PATCH] Disassemble over end of line table sequence.

[Doug Evans]

I wonder if now is a good time to discuss whether we want to keep the
"source-centric" disassembly.
Does anyone actually find the current way useful?
[i.e. display the disassembly with possibly non-contiguous pcs, sorted
by source line number instead]

On Thu, Jan 13, 2011 at 2:39 AM, Andrew Burgess <aburgess@broadcom.com> wrote:
> Ping!
>
> Could someone cast an eye over this please.
>
> Thanks,
>
> Andrew
>
>
> On 15/12/2010 14:23, Andrew Burgess wrote:
>>
>> Within gdb we use line number 0 to mark the end of a sequence within the
>> line number tables. When disassembling with source code we sort the
>> lines based on the line number. As a result if a request is made to
>> disassemble a range of addresses that includes an end of sequence marker
>> the disassemble command will give an error rather than any sensible
>> output.
>>
>> In the included test I request a disassembly for a range of addresses
>> that spans a function boundary, currently this will fail. With the fix
>> the disassembly I get back still does not quite cover the complete range
>> of addresses requested, I am treating this problem as a separate issue
>> to be fixed in a later patch, here I am only interested in ordering the
>> lines correctly so that I get something sensible back rather than an
>> error.
>>
>> Thanks,
>>
>> Andrew
>>
>>
>>
>> gdb/
>>
>> 2010-12-10  Andrew Burgess<aburgess@broadcom.com>
>>
>>            * (compare_lines): Sort by pc for entries where the line
>>              number is 0, these are the end of sequence markers.
>>
>> 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.
>>
>>
>> diff --git a/gdb/disasm.c b/gdb/disasm.c
>> index 9fa34a0..2259f3e 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;
>> +  /* Work with end of sequence markers
>> +     where the line number is set to 0 */
>> +  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.disasm/disasm-end-cu-1.c
>> b/gdb/testsuite/gdb.disasm/disasm-end-cu-1.c
>> new file mode 100644
>> index 0000000..c031778
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.disasm/disasm-end-cu-1.c
>> @@ -0,0 +1,14 @@
>> +#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.disasm/disasm-end-cu-2.c
>> b/gdb/testsuite/gdb.disasm/disasm-end-cu-2.c
>> new file mode 100644
>> index 0000000..7fddb87
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.disasm/disasm-end-cu-2.c
>> @@ -0,0 +1,13 @@
>> +#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.disasm/disasm-end-cu.exp
>> b/gdb/testsuite/gdb.disasm/disasm-end-cu.exp
>> new file mode 100644
>> index 0000000..fdbd179
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.disasm/disasm-end-cu.exp
>> @@ -0,0 +1,70 @@
>> +# Copyright 2010 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 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 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
>> +}
>> +
>> +send_gdb "disassemble /m ${main_addr},${dummy_3_addr}\n"
>> +gdb_expect {
>> +    -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"
>> +    }
>> +}
>> +
>> +gdb_stop_suppressing_tests;
>>
>>
>>
>
>
>

Re: [PATCH] Disassemble over end of line table sequence.

[Andrew Burgess]

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"
+    }
+}





 

Re: [PATCH] Disassemble over end of line table sequence.

[Andrew Burgess]

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"
> +    }
> +}
>
>
>
>
>
>
>
>
>
>

Re: [PATCH] Disassemble over end of line table sequence.

[Joel Brobecker]

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

Sorry for the late review.  For some reason, I completely missed this
email.  Thanks for pinging.

I think we're almost there.  The disasm.c change looks good to me.
But I still have a couple of requests for the testcase.

> +   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>

Can you make your testcase not depend on stdio.h? IO is not always
available, particularly on bareboard targets, so it'd be nice to be
able to build the example on these platforms too... This should be easily
doable by just having another unit that provides a function with the
same signature.

> +# 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.

Just a nit: missing second space after the period on the second line.

> +gdb_test_multiple "disassemble /m ${main_addr},${dummy_3_addr}" "Disassemble address range with source" {
[...]
> +    timeout {
> +        fail "(timeout) waiting for output of disassemble command"

The "timeout" branch is unnecessary (it's already handled by
gdb_test_multiple).

-- 
Joel

Re: [PATCH] Disassemble over end of line table sequence.

[Andrew Burgess]

On 31/01/2011 02:41, Joel Brobecker wrote:
>
> I think we're almost there.  The disasm.c change looks good to me.
> But I still have a couple of requests for the testcase.
>
>> +   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>
>
> Can you make your testcase not depend on stdio.h? IO is not always
> available, particularly on bareboard targets, so it'd be nice to be
> able to build the example on these platforms too... This should be easily
> doable by just having another unit that provides a function with the
> same signature.

Removed, I only included the call to printf to make sure that the 
functions had some content, some simple arithmetic does the job fine.

>
>> +# 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.
>
> Just a nit: missing second space after the period on the second line.

Fixed.

>
>> +gdb_test_multiple "disassemble /m ${main_addr},${dummy_3_addr}" "Disassemble address range with source" {
> [...]
>> +    timeout {
>> +        fail "(timeout) waiting for output of disassemble command"
>
> The "timeout" branch is unnecessary (it's already handled by
> gdb_test_multiple).
>

Removed.

New patch is below.

As always, thanks for the review,
Andrew


gdb/ChangeLog
2011-02-02  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-02-02  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..194b8b1
--- /dev/null
+++ b/gdb/testsuite/gdb.base/disasm-end-cu-1.c
@@ -0,0 +1,28 @@
+/* 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/>.  */
+
+int
+dummy_1 (int a, int b, int c)
+{
+  return a + b + c;
+}
+
+int
+main ()
+{
+  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..be716e7
--- /dev/null
+++ b/gdb/testsuite/gdb.base/disasm-end-cu-2.c
@@ -0,0 +1,28 @@
+/* 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/>.  */
+
+int
+dummy_2 (int a, int b, int c)
+{
+  return a + b + c;
+}
+
+int
+dummy_3 (int a, int b, int c)
+{
+  return a - b - c;
+}
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..5184f11
--- /dev/null
+++ b/gdb/testsuite/gdb.base/disasm-end-cu.exp
@@ -0,0 +1,49 @@
+# 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"
+    }
+}

Re: [PATCH] Disassemble over end of line table sequence.

[Joel Brobecker]

> gdb/ChangeLog
> 2011-02-02  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-02-02  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.

OK to commit!

Thanks for this fix,
-- 
Joel

Re: [PATCH] [COMMIT] Disassemble over end of line table sequence.

[Andrew Burgess]

On 03/02/2011 05:01, Joel Brobecker wrote:
>> gdb/ChangeLog
>> 2011-02-02  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-02-02  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.
>
> OK to commit!

Committed.