public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH][gdb/symtab] Remove superfluous end-of-sequence marker
@ 2021-01-04 13:59 Tom de Vries
  2021-01-04 16:27 ` Bernd Edlinger
  0 siblings, 1 reply; 3+ messages in thread
From: Tom de Vries @ 2021-01-04 13:59 UTC (permalink / raw)
  To: gdb-patches

Hi,

While working on PR26935 I noticed that when running test-case
gdb.base/morestack.exp with target board unix/-m32/-fPIE/-pie and ld linker,
I get this linetable fragment for morestack.S using readelf -wL:
...
CU: ../../../../libgcc/config/i386/morestack.S:
Line number    Starting address    View    Stmt
109               0xc9c               x
  ...
838               0xe03               x
  -               0xe04

636                   0               x
637                 0x3               x
  -                 0x4
...
but with "maint info line-table" I get:
...
INDEX  LINE   ADDRESS            IS-STMT
0      END    0x00000004         Y
1      109    0x00000c9c         Y
  ...
110    838    0x00000e03         Y
111    END    0x00000e04         Y
...

So, apparently the entries with addresses 0x0 and 0x3 are filtered out
because the addresses are out of range, but the same doesn't happen with the
end-of-seq terminator.

Fix this by filtering out end-of-seq terminators that do not actually
terminate anything.

Tested on x86_64-linux.

Any comments?

Thanks,
- Tom

[gdb/symtab] Remove superfluous end-of-sequence marker

gdb/ChangeLog:

2021-01-04  Tom de Vries  <tdevries@suse.de>

	* buildsym.c (buildsym_compunit::record_line): Filter out end-of-seq
	terminators that do not terminate anything.

gdb/testsuite/ChangeLog:

2021-01-04  Tom de Vries  <tdevries@suse.de>

	* gdb.dwarf2/dw2-out-of-range-end-of-seq.exp: New file.

---
 gdb/buildsym.c                                     |  8 ++
 .../gdb.dwarf2/dw2-out-of-range-end-of-seq.exp     | 94 ++++++++++++++++++++++
 2 files changed, 102 insertions(+)

diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 1d8c579b7b8..245aecf0c38 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -714,6 +714,14 @@ buildsym_compunit::record_line (struct subfile *subfile, int line,
 	    break;
 	  subfile->line_vector->nitems--;
 	}
+
+      /* Ignore an end-of-sequence marker marking an empty sequence.  */
+      struct linetable_entry *last
+	= (subfile->line_vector->nitems == 0
+	   ? nullptr
+	   : &subfile->line_vector->item[subfile->line_vector->nitems - 1]);
+      if (last == nullptr || last->line == 0)
+	return;
     }
 
   e = subfile->line_vector->item + subfile->line_vector->nitems++;
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-out-of-range-end-of-seq.exp b/gdb/testsuite/gdb.dwarf2/dw2-out-of-range-end-of-seq.exp
new file mode 100644
index 00000000000..7a32a01389c
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-out-of-range-end-of-seq.exp
@@ -0,0 +1,94 @@
+# Copyright 2021 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/>.
+
+# Check that an out-of-range DW_LNE_end_sequence is removed from the
+# line table.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    verbose "Skipping $gdb_test_file_name."
+    return 0
+}
+
+# The .c files use __attribute__.
+if [get_compiler_info] {
+    return -1
+}
+if !$gcc_compiled {
+    verbose "Skipping $gdb_test_file_name."
+    return 0
+}
+
+standard_testfile main.c -dw.S
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    declare_labels Llines
+    global srcdir subdir srcfile
+
+    cu {} {
+	compile_unit {
+	    {language @DW_LANG_C}
+	    {name $srcfile}
+	    {stmt_list $Llines DW_FORM_sec_offset}
+	} {
+	    subprogram {
+		{external 1 flag}
+		{MACRO_AT_func {main}}
+	    }
+	}
+    }
+
+    lines {version 2} Llines {
+	include_dir "${srcdir}/${subdir}"
+	file_name "$srcfile" 1
+
+	program {
+	    {DW_LNE_set_address main_label}
+	    {line 1}
+	    {DW_LNS_copy}
+
+	    {DW_LNS_advance_pc 1}
+	    {DW_LNE_end_sequence}
+
+	    {DW_LNE_set_address 0}
+	    {line 2}
+	    {DW_LNS_copy}
+
+	    {DW_LNS_advance_pc 1}
+	    {DW_LNE_end_sequence}
+	}
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_test_multiple "maint info line-table" "END with address 1 eliminated" {
+    -re -wrap "END *0x0*1 Y \r\n.*" {
+	fail $gdb_test_name
+    }
+    -re -wrap "" {
+	pass $gdb_test_name
+    }
+}

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH][gdb/symtab] Remove superfluous end-of-sequence marker
  2021-01-04 13:59 [PATCH][gdb/symtab] Remove superfluous end-of-sequence marker Tom de Vries
@ 2021-01-04 16:27 ` Bernd Edlinger
  2021-01-04 18:44   ` Tom de Vries
  0 siblings, 1 reply; 3+ messages in thread
From: Bernd Edlinger @ 2021-01-04 16:27 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

On 1/4/21 2:59 PM, Tom de Vries wrote:
> Hi,
> 
> While working on PR26935 I noticed that when running test-case
> gdb.base/morestack.exp with target board unix/-m32/-fPIE/-pie and ld linker,
> I get this linetable fragment for morestack.S using readelf -wL:
> ...
> CU: ../../../../libgcc/config/i386/morestack.S:
> Line number    Starting address    View    Stmt
> 109               0xc9c               x
>   ...
> 838               0xe03               x
>   -               0xe04
> 
> 636                   0               x
> 637                 0x3               x
>   -                 0x4
> ...
> but with "maint info line-table" I get:
> ...
> INDEX  LINE   ADDRESS            IS-STMT
> 0      END    0x00000004         Y
> 1      109    0x00000c9c         Y
>   ...
> 110    838    0x00000e03         Y
> 111    END    0x00000e04         Y
> ...
> 
> So, apparently the entries with addresses 0x0 and 0x3 are filtered out
> because the addresses are out of range, but the same doesn't happen with the
> end-of-seq terminator.
> 
> Fix this by filtering out end-of-seq terminators that do not actually
> terminate anything.
> 
> Tested on x86_64-linux.
> 
> Any comments?
> 
> Thanks,
> - Tom
> 
> [gdb/symtab] Remove superfluous end-of-sequence marker
> 
> gdb/ChangeLog:
> 
> 2021-01-04  Tom de Vries  <tdevries@suse.de>
> 
> 	* buildsym.c (buildsym_compunit::record_line): Filter out end-of-seq
> 	terminators that do not terminate anything.
> 
> gdb/testsuite/ChangeLog:
> 
> 2021-01-04  Tom de Vries  <tdevries@suse.de>
> 
> 	* gdb.dwarf2/dw2-out-of-range-end-of-seq.exp: New file.
> 
> ---
>  gdb/buildsym.c                                     |  8 ++
>  .../gdb.dwarf2/dw2-out-of-range-end-of-seq.exp     | 94 ++++++++++++++++++++++
>  2 files changed, 102 insertions(+)
> 
> diff --git a/gdb/buildsym.c b/gdb/buildsym.c
> index 1d8c579b7b8..245aecf0c38 100644
> --- a/gdb/buildsym.c
> +++ b/gdb/buildsym.c
> @@ -714,6 +714,14 @@ buildsym_compunit::record_line (struct subfile *subfile, int line,
>  	    break;
>  	  subfile->line_vector->nitems--;
>  	}
> +
> +      /* Ignore an end-of-sequence marker marking an empty sequence.  */
> +      struct linetable_entry *last
> +	= (subfile->line_vector->nitems == 0
> +	   ? nullptr
> +	   : &subfile->line_vector->item[subfile->line_vector->nitems - 1]);
> +      if (last == nullptr || last->line == 0)
> +	return;
>      }
>  

What you want to achieve here can probably be done more easily this way:

diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 1d8c579..dda83f7 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -710,6 +710,8 @@ struct blockvector *
       while (subfile->line_vector->nitems > 0)
        {
          e = subfile->line_vector->item + subfile->line_vector->nitems - 1;
+         if (e->line == 0)
+           return;
          if (e->pc != pc)
            break;
          subfile->line_vector->nitems--;


I wonder however where the out-of-range line entries are filtered away.
Would it be better to filter the end of sequence markers there?


Bernd.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH][gdb/symtab] Remove superfluous end-of-sequence marker
  2021-01-04 16:27 ` Bernd Edlinger
@ 2021-01-04 18:44   ` Tom de Vries
  0 siblings, 0 replies; 3+ messages in thread
From: Tom de Vries @ 2021-01-04 18:44 UTC (permalink / raw)
  To: Bernd Edlinger, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 3672 bytes --]

On 1/4/21 5:27 PM, Bernd Edlinger wrote:
> On 1/4/21 2:59 PM, Tom de Vries wrote:
>> Hi,
>>
>> While working on PR26935 I noticed that when running test-case
>> gdb.base/morestack.exp with target board unix/-m32/-fPIE/-pie and ld linker,
>> I get this linetable fragment for morestack.S using readelf -wL:
>> ...
>> CU: ../../../../libgcc/config/i386/morestack.S:
>> Line number    Starting address    View    Stmt
>> 109               0xc9c               x
>>   ...
>> 838               0xe03               x
>>   -               0xe04
>>
>> 636                   0               x
>> 637                 0x3               x
>>   -                 0x4
>> ...
>> but with "maint info line-table" I get:
>> ...
>> INDEX  LINE   ADDRESS            IS-STMT
>> 0      END    0x00000004         Y
>> 1      109    0x00000c9c         Y
>>   ...
>> 110    838    0x00000e03         Y
>> 111    END    0x00000e04         Y
>> ...
>>
>> So, apparently the entries with addresses 0x0 and 0x3 are filtered out
>> because the addresses are out of range, but the same doesn't happen with the
>> end-of-seq terminator.
>>
>> Fix this by filtering out end-of-seq terminators that do not actually
>> terminate anything.
>>
>> Tested on x86_64-linux.
>>
>> Any comments?
>>
>> Thanks,
>> - Tom
>>
>> [gdb/symtab] Remove superfluous end-of-sequence marker
>>
>> gdb/ChangeLog:
>>
>> 2021-01-04  Tom de Vries  <tdevries@suse.de>
>>
>> 	* buildsym.c (buildsym_compunit::record_line): Filter out end-of-seq
>> 	terminators that do not terminate anything.
>>
>> gdb/testsuite/ChangeLog:
>>
>> 2021-01-04  Tom de Vries  <tdevries@suse.de>
>>
>> 	* gdb.dwarf2/dw2-out-of-range-end-of-seq.exp: New file.
>>
>> ---
>>  gdb/buildsym.c                                     |  8 ++
>>  .../gdb.dwarf2/dw2-out-of-range-end-of-seq.exp     | 94 ++++++++++++++++++++++
>>  2 files changed, 102 insertions(+)
>>
>> diff --git a/gdb/buildsym.c b/gdb/buildsym.c
>> index 1d8c579b7b8..245aecf0c38 100644
>> --- a/gdb/buildsym.c
>> +++ b/gdb/buildsym.c
>> @@ -714,6 +714,14 @@ buildsym_compunit::record_line (struct subfile *subfile, int line,
>>  	    break;
>>  	  subfile->line_vector->nitems--;
>>  	}
>> +
>> +      /* Ignore an end-of-sequence marker marking an empty sequence.  */
>> +      struct linetable_entry *last
>> +	= (subfile->line_vector->nitems == 0
>> +	   ? nullptr
>> +	   : &subfile->line_vector->item[subfile->line_vector->nitems - 1]);
>> +      if (last == nullptr || last->line == 0)
>> +	return;
>>      }
>>  
> 
> What you want to achieve here can probably be done more easily this way:
> 
> diff --git a/gdb/buildsym.c b/gdb/buildsym.c
> index 1d8c579..dda83f7 100644
> --- a/gdb/buildsym.c
> +++ b/gdb/buildsym.c
> @@ -710,6 +710,8 @@ struct blockvector *
>        while (subfile->line_vector->nitems > 0)
>         {
>           e = subfile->line_vector->item + subfile->line_vector->nitems - 1;
> +         if (e->line == 0)
> +           return;
>           if (e->pc != pc)
>             break;
>           subfile->line_vector->nitems--;
> 
> 

Hi Bernd,

Thanks for the review.

Indeed, that's less verbose than the current patch, but to me it feels
more convoluted.  I'd rather keep the additional test out of the loop.

I did realize because of your comment that in both cases we're dealing
with the last entry in the table, so I've rewritten the patch to use a
last variable in the loop, and to reuse it afterwards.

> I wonder however where the out-of-range line entries are filtered away.
> Would it be better to filter the end of sequence markers there?

I'm also curious about that, but I don't think it matters much.

Committed to trunk as attached.

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-symtab-Remove-superfluous-end-of-sequence-marker.patch --]
[-- Type: text/x-patch, Size: 5142 bytes --]

[gdb/symtab] Remove superfluous end-of-sequence marker

While working on PR26935 I noticed that when running test-case
gdb.base/morestack.exp with target board unix/-m32/-fPIE/-pie and ld linker,
I get this linetable fragment for morestack.S using readelf -wL:
...
CU: ../../../../libgcc/config/i386/morestack.S:
Line number    Starting address    View    Stmt
109               0xc9c               x
  ...
838               0xe03               x
  -               0xe04

636                   0               x
637                 0x3               x
  -                 0x4
...
but with "maint info line-table" I get:
...
INDEX  LINE   ADDRESS            IS-STMT
0      END    0x00000004         Y
1      109    0x00000c9c         Y
  ...
110    838    0x00000e03         Y
111    END    0x00000e04         Y
...

So, apparently the entries with addresses 0x0 and 0x3 are filtered out
because the addresses are out of range, but the same doesn't happen with the
end-of-seq terminator.

Fix this by filtering out end-of-seq terminators that do not actually
terminate anything.

Tested on x86_64-linux.

gdb/ChangeLog:

2021-01-04  Tom de Vries  <tdevries@suse.de>

	* buildsym.c (buildsym_compunit::record_line): Filter out end-of-seq
	terminators that do not terminate anything.

gdb/testsuite/ChangeLog:

2021-01-04  Tom de Vries  <tdevries@suse.de>

	* gdb.dwarf2/dw2-out-of-range-end-of-seq.exp: New file.

---
 gdb/buildsym.c                                     |  9 ++-
 .../gdb.dwarf2/dw2-out-of-range-end-of-seq.exp     | 94 ++++++++++++++++++++++
 2 files changed, 101 insertions(+), 2 deletions(-)

diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 1d8c579b7b8..0f7449fed97 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -707,13 +707,18 @@ buildsym_compunit::record_line (struct subfile *subfile, int line,
      anyway.  */
   if (line == 0)
     {
+      struct linetable_entry *last = nullptr;
       while (subfile->line_vector->nitems > 0)
 	{
-	  e = subfile->line_vector->item + subfile->line_vector->nitems - 1;
-	  if (e->pc != pc)
+	  last = subfile->line_vector->item + subfile->line_vector->nitems - 1;
+	  if (last->pc != pc)
 	    break;
 	  subfile->line_vector->nitems--;
 	}
+
+      /* Ignore an end-of-sequence marker marking an empty sequence.  */
+      if (last == nullptr || last->line == 0)
+	return;
     }
 
   e = subfile->line_vector->item + subfile->line_vector->nitems++;
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-out-of-range-end-of-seq.exp b/gdb/testsuite/gdb.dwarf2/dw2-out-of-range-end-of-seq.exp
new file mode 100644
index 00000000000..7a32a01389c
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-out-of-range-end-of-seq.exp
@@ -0,0 +1,94 @@
+# Copyright 2021 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/>.
+
+# Check that an out-of-range DW_LNE_end_sequence is removed from the
+# line table.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    verbose "Skipping $gdb_test_file_name."
+    return 0
+}
+
+# The .c files use __attribute__.
+if [get_compiler_info] {
+    return -1
+}
+if !$gcc_compiled {
+    verbose "Skipping $gdb_test_file_name."
+    return 0
+}
+
+standard_testfile main.c -dw.S
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    declare_labels Llines
+    global srcdir subdir srcfile
+
+    cu {} {
+	compile_unit {
+	    {language @DW_LANG_C}
+	    {name $srcfile}
+	    {stmt_list $Llines DW_FORM_sec_offset}
+	} {
+	    subprogram {
+		{external 1 flag}
+		{MACRO_AT_func {main}}
+	    }
+	}
+    }
+
+    lines {version 2} Llines {
+	include_dir "${srcdir}/${subdir}"
+	file_name "$srcfile" 1
+
+	program {
+	    {DW_LNE_set_address main_label}
+	    {line 1}
+	    {DW_LNS_copy}
+
+	    {DW_LNS_advance_pc 1}
+	    {DW_LNE_end_sequence}
+
+	    {DW_LNE_set_address 0}
+	    {line 2}
+	    {DW_LNS_copy}
+
+	    {DW_LNS_advance_pc 1}
+	    {DW_LNE_end_sequence}
+	}
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_test_multiple "maint info line-table" "END with address 1 eliminated" {
+    -re -wrap "END *0x0*1 Y \r\n.*" {
+	fail $gdb_test_name
+    }
+    -re -wrap "" {
+	pass $gdb_test_name
+    }
+}

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-01-04 18:44 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-04 13:59 [PATCH][gdb/symtab] Remove superfluous end-of-sequence marker Tom de Vries
2021-01-04 16:27 ` Bernd Edlinger
2021-01-04 18:44   ` Tom de Vries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).