Re: [gdb/testsuite] Fix bad line table entry sequence

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Tom de Vries <tdevries@suse.de> [2020-06-08 16:40:40 +0200]:

> [ was: Re: [PATCH][gdb/symtab] Fix line-table end-of-sequence sorting ]
> 
> On 06-06-2020 11:25, Andrew Burgess wrote:
> > * Tom de Vries <tdevries@suse.de> [2020-06-06 01:44:42 +0200]:
> > 
> >> [ was: Re: [PATCH 2/3] gdb: Don't reorder line table entries too much
> >> when sorting. ]
> >>
> >> On 05-06-2020 18:00, Tom de Vries wrote:
> >>> On 05-06-2020 16:49, Tom de Vries wrote:
> >>>> On 23-12-2019 02:51, Andrew Burgess wrote:
> >>>>> I had to make a small adjustment in find_pc_sect_line in order to
> >>>>> correctly find the previous line in the line table.  In some line
> >>>>> tables I was seeing an actual line entry and an end of sequence marker
> >>>>> at the same address, before this commit these would reorder to move
> >>>>> the end of sequence marker before the line entry (end of sequence has
> >>>>> line number 0).  Now the end of sequence marker remains in its correct
> >>>>> location, and in order to find a previous line we should step backward
> >>>>> over any end of sequence markers.
> >>>>>
> >>>>> As an example, the binary:
> >>>>>   gdb/testsuite/outputs/gdb.dwarf2/dw2-ranges-func/dw2-ranges-func-lo-cold
> >>>>>
> >>>>> Has this line table before the patch:
> >>>>>
> >>>>>   INDEX    LINE ADDRESS
> >>>>>   0          48 0x0000000000400487
> >>>>>   1         END 0x000000000040048e
> >>>>>   2          52 0x000000000040048e
> >>>>>   3          54 0x0000000000400492
> >>>>>   4          56 0x0000000000400497
> >>>>>   5         END 0x000000000040049a
> >>>>>   6          62 0x000000000040049a
> >>>>>   7         END 0x00000000004004a1
> >>>>>   8          66 0x00000000004004a1
> >>>>>   9          68 0x00000000004004a5
> >>>>>   10         70 0x00000000004004aa
> >>>>>   11         72 0x00000000004004b9
> >>>>>   12        END 0x00000000004004bc
> >>>>>   13         76 0x00000000004004bc
> >>>>>   14         78 0x00000000004004c0
> >>>>>   15         80 0x00000000004004c5
> >>>>>   16        END 0x00000000004004cc
> >>>>>
> >>>>> And after this patch:
> >>>>>
> >>>>>   INDEX    LINE ADDRESS
> >>>>>   0          48 0x0000000000400487
> >>>>>   1          52 0x000000000040048e
> >>>>>   2         END 0x000000000040048e
> >>>>>   3          54 0x0000000000400492
> >>>>>   4          56 0x0000000000400497
> >>>>>   5         END 0x000000000040049a
> >>>>>   6          62 0x000000000040049a
> >>>>>   7          66 0x00000000004004a1
> >>>>>   8         END 0x00000000004004a1
> >>>>>   9          68 0x00000000004004a5
> >>>>>   10         70 0x00000000004004aa
> >>>>>   11         72 0x00000000004004b9
> >>>>>   12        END 0x00000000004004bc
> >>>>>   13         76 0x00000000004004bc
> >>>>>   14         78 0x00000000004004c0
> >>>>>   15         80 0x00000000004004c5
> >>>>>   16        END 0x00000000004004cc
> >>>>>
> >>>>> When calling find_pc_sect_line with the address 0x000000000040048e, in
> >>>>> both cases we find entry #3, we then try to find the previous entry,
> >>>>> which originally found this entry '2         52 0x000000000040048e',
> >>>>> after the patch it finds '2         END 0x000000000040048e', which
> >>>>> cases the lookup to fail.
> >>>>>
> >>>>> By skipping the END marker after this patch we get back to the correct
> >>>>> entry, which is now #1: '1          52 0x000000000040048e', and
> >>>>> everything works again.
> >>>>
> >>>> I start to suspect that you have been working around an incorrect line
> >>>> table.
> >>>>
> >>>> Consider this bit:
> >>>> ...
> >>>>    0          48 0x0000000000400487
> >>>>    1          52 0x000000000040048e
> >>>>    2         END 0x000000000040048e
> >>>> ...
> >>>>
> >>>> The end marker marks the address one past the end of the sequence.
> >>>> Therefore, it makes no sense to have an entry in the sequence with the
> >>>> same address as the end marker.
> >>>>
> >>>> [ dwarf doc:
> >>>>
> >>>> end_sequence:
> >>>>
> >>>> A boolean indicating that the current address is that of the first byte
> >>>> after the end of a sequence of target machine instructions. end_sequence
> >>>> terminates a sequence of lines; therefore other information in the same
> >>>> row is not meaningful.
> >>>>
> >>>> DW_LNE_end_sequence:
> >>>>
> >>>> The DW_LNE_end_sequence opcode takes no operands. It sets the
> >>>> end_sequence register of the state machine to “true” and appends a row
> >>>> to the matrix using the current values of the state-machine registers.
> >>>> Then it resets the registers to the initial values specified above (see
> >>>> Section 6.2.2). Every line number program sequence must end with a
> >>>> DW_LNE_end_sequence instruction which creates a row whose address is
> >>>> that of the byte after the last target machine instruction of the sequence.
> >>>>
> >>>> ]
> >>>>
> >>>> The incorrect entry is generated by this dwarf assembler sequence:
> >>>> ...
> >>>>                 {DW_LNS_copy}
> >>>>                 {DW_LNE_end_sequence}
> >>>> ...
> >>>>
> >>>> I think we should probably fix the dwarf assembly test-cases.
> >>>>
> >>>> If we want to handle this in gdb, the thing that seems most logical to
> >>>> me is to ignore this kind of entries.
> >>>
> >>> Hmm, that seems to be done already, in buildsym_compunit::record_line.
> >>>
> >>> Anyway, I was looking at the line table for
> >>> gdb.dwarf2/dw2-ranges-base.exp, and got a line table with subsequent end
> >>> markers:
> >>> ...
> >>> INDEX  LINE   ADDRESS            IS-STMT
> >>> 0      31     0x00000000004004a7 Y
> >>> 1      21     0x00000000004004ae Y
> >>> 2      END    0x00000000004004ae Y
> >>> 3      11     0x00000000004004ba Y
> >>> 4      END    0x00000000004004ba Y
> >>> 5      END    0x00000000004004c6 Y
> >>> ...
> >>>
> >>> By using this patch:
> >>> ...
> >>> diff --git a/gdb/buildsym.c b/gdb/buildsym.c
> >>> index 33bf6523e9..76f0b54ff6 100644
> >>> --- a/gdb/buildsym.c
> >>> +++ b/gdb/buildsym.c
> >>> @@ -943,6 +943,10 @@ buildsym_compunit::end_symtab_with_blockvector
> >>> (struct block *static_block,
> >>>             = [] (const linetable_entry &ln1,
> >>>                   const linetable_entry &ln2) -> bool
> >>>               {
> >>> +               if (ln1.pc == ln2.pc
> >>> +                   && ((ln1.line == 0) != (ln2.line == 0)))
> >>> +                 return ln1.line == 0 ? true : false;
> >>> +
> >>>                 return (ln1.pc < ln2.pc);
> >>>               };
> >>>
> >>> ...
> >>> I get the expected:
> >>> ...
> >>> INDEX  LINE   ADDRESS            IS-STMT
> >>> 0      31     0x00000000004004a7 Y
> >>> 1      END    0x00000000004004ae Y
> >>> 2      21     0x00000000004004ae Y
> >>> 3      END    0x00000000004004ba Y
> >>> 4      11     0x00000000004004ba Y
> >>> 5      END    0x00000000004004c6 Y
> >>> ...
> >>
> >> Any comments on patch below?
> > 
> > Yes! Thank you for working on this.
> > 
> > This should go in, however, I'd like to tweak the commit message a bit
> > please (see below).
> > 
> > Also, do you plan to include the revert of find_pc_sect_line from
> > 3d92a3e313 in this patch - I think you should.
> > 
> > Thanks,
> > Andrew
> > 
> >>
> >> Thanks,
> >> - Tom
> >>
> > 
> >> [gdb/symtab] Fix line-table end-of-sequence sorting
> >>
> >> Consider test-case gdb.dwarf2/dw2-ranges-base.exp.  It has a line-table for
> >> dw2-ranges-base.c like this:
> >> ...
> >>  Line Number Statements:
> >>   [0x0000014e]  Extended opcode 2: set Address to 0x4004ba
> >>   [0x00000159]  Advance Line by 10 to 11
> >>   [0x0000015b]  Copy
> >>   [0x0000015c]  Advance PC by 12 to 0x4004c6
> >>   [0x0000015e]  Advance Line by 19 to 30
> >>   [0x00000160]  Copy
> >>   [0x00000161]  Extended opcode 1: End of Sequence
> >>
> >>   [0x00000164]  Extended opcode 2: set Address to 0x4004ae
> >>   [0x0000016f]  Advance Line by 20 to 21
> >>   [0x00000171]  Copy
> >>   [0x00000172]  Advance PC by 12 to 0x4004ba
> >>   [0x00000174]  Advance Line by 29 to 50
> >>   [0x00000176]  Copy
> >>   [0x00000177]  Extended opcode 1: End of Sequence
> >>
> >>   [0x0000017a]  Extended opcode 2: set Address to 0x4004a7
> >>   [0x00000185]  Advance Line by 30 to 31
> >>   [0x00000187]  Copy
> >>   [0x00000188]  Advance PC by 7 to 0x4004ae
> >>   [0x0000018a]  Advance Line by 39 to 70
> >>   [0x0000018c]  Copy
> >>   [0x0000018d]  Extended opcode 1: End of Sequence
> >> ...
> >>
> >> The Copy followed by End-of-Sequence is as specified in the dwarf assembly,
> >> but incorrect.  F.i., consider:
> >> ...
> >>   [0x0000015c]  Advance PC by 12 to 0x4004c6
> >>   [0x0000015e]  Advance Line by 19 to 30
> >>   [0x00000160]  Copy
> >>   [0x00000161]  Extended opcode 1: End of Sequence
> >> ...
> >>
> >> Both the Copy and the End-of-Sequence append a row to the matrix using the
> >> same addres: 0x4004c6.  The Copy declares a target instruction at that
> >> address.  The End-of-Sequence declares that the sequence ends before that
> >> address.  It's a contradiction that the target instruction is both part of the
> >> sequence (according to Copy) and not part of the sequence (according to
> >> End-of-Sequence).
> > 
> > I don't want to argue about this specific test, but about the idea of
> > an empty line occurring at the same address as an end of sequence
> > marker.  This can happen due to compiler optimisation, though it is
> > perfectly reasonable to suggest that in this case the compiler should
> > be removing the line marker, this doesn't always happen.
> > 
> > I guess what I'm saying is that I think the case for this being
> > obviously wrong is not quite as clear cut as you seem to imply>, but
> > also, I don't think this is really relevant to this bug - so maybe we
> > could just drop this part?
> > 
> 
> To me it's obviously wrong, so it would help me if you explain your
> interpretation of the dwarf standard on this topic.

I'm not aware of any statement in DWARF that specifically mentions
this case, but not do I believe that this case is specifically pointed
out as being forbidden.

Then there's the existing code in GDB to deal with this case, in
buildsym_compunit::record_line, this code:

  /* Normally, we treat lines as unsorted.  But the end of sequence
     marker is special.  We sort line markers at the same PC by line
     number, so end of sequence markers (which have line == 0) appear
     first.  This is right if the marker ends the previous function,
     and there is no padding before the next function.  But it is
     wrong if the previous line was empty and we are now marking a
     switch to a different subfile.  We must leave the end of sequence
     marker at the end of this group of lines, not sort the empty line
     to after the marker.  The easiest way to accomplish this is to
     delete any empty lines from our table, if they are followed by
     end of sequence markers.  All we lose is the ability to set
     breakpoints at some lines which contain no instructions
     anyway.  */
  if (line == 0)
    {
      while (subfile->line_vector->nitems > 0)
	{
	  e = subfile->line_vector->item + subfile->line_vector->nitems - 1;
	  if (e->pc != pc)
	    break;
	  subfile->line_vector->nitems--;
	}
    }

Is specifically to deal with this case.

Finally, I feel that a compiler could easily get into the situation
where optimising out a line might result in an empty line.

So, I certainly agree with you that an empty line is pretty
meaningless (though maybe it does still tell us something), but I
don't think we can say its wrong.

> 
> I think though I may have indeed conflated two issues in my original
> patch, so I've now got one patch that deals with the incorrect dwarf in
> the dwarf test-cases (attached below), and I'll submit another dealing
> with the regression.  I hope that will make the conversation easier.

Sounds like a good plan.

> 
> Thanks,
> - Tom

> [gdb/testsuite] Fix bad line table entry sequence
> 
> Consider test-case gdb.dwarf2/dw2-ranges-base.exp.  It has a line-table for
> dw2-ranges-base.c like this:
> ...
>  Line Number Statements:
>   [0x0000014e]  Extended opcode 2: set Address to 0x4004ba
>   [0x00000159]  Advance Line by 10 to 11
>   [0x0000015b]  Copy
>   [0x0000015c]  Advance PC by 12 to 0x4004c6
>   [0x0000015e]  Advance Line by 19 to 30
>   [0x00000160]  Copy
>   [0x00000161]  Extended opcode 1: End of Sequence
> ...
> 
> The Copy followed by End-of-Sequence is as specified in the dwarf assembly,
> but incorrect.
> 
> Both the Copy and the End-of-Sequence append a row to the matrix, each using
> the same addres: 0x4004c6.  The Copy declares a target instruction at that
> address.  The End-of-Sequence declares that the sequence ends before that
> address.
> 
> It's a contradiction that the target instruction is both part of the sequence
> (according to Copy) and not part of the sequence (according to End-of-Sequence).
> 
> The offending Copy is silently skipped by buildsym_compunit::record_line.  A
> gdb PR has been filed to warn about this (PR26092).
> 
> Fix the dwarf assembly test-cases that contain this contradictory construct,
> to prevent people from:
> - coming across this while testing patches, and mistakenly getting the
>   impression that this is valid dwarf, or even
> - copying it to new test-cases.
> 
> Also, add a dwarf assembly test-case called dw2-bad-dw-lne-end-sequence.exp
> containing this construct, to verify that it's ignored.
> 
> Tested on x86_64-linux.
> 
> Also tested using a detection patch with assert, such that the only test-case
> triggering that assert is dw2-bad-dw-lne-end-sequence.exp.  The corresponding
> build had to be done without --with-separate-debug-dir=/usr/lib/debug",
> otherwise we'd trigger the assert in debug info generated with recent GCC (PR
> gcc/95574).
> 
> gdb/testsuite/ChangeLog:
> 
> 2020-06-08  Tom de Vries  <tdevries@suse.de>
> 
> 	* gdb.dwarf2/dw2-bad-elf.exp: Fix bad line table entry sequence.
> 	* gdb.dwarf2/dw2-dir-file-name.exp: Same.
> 	* gdb.dwarf2/dw2-inline-small-func.exp: Same.
> 	* gdb.dwarf2/dw2-ranges-base.exp: Same.
> 	* gdb.dwarf2/dw2-ranges-func.exp: Same.
> 	* gdb.dwarf2/dw2-bad-dw-lne-end-sequence.exp: New file.
> 
> ---
>  .../gdb.dwarf2/dw2-bad-dw-lne-end-sequence.exp     | 116 +++++++++++++++++++++
>  gdb/testsuite/gdb.dwarf2/dw2-bad-elf.exp           |   4 -
>  gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp     |   1 -
>  gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.exp |   1 -
>  gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp       |   6 --
>  gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp       |  10 --
>  6 files changed, 116 insertions(+), 22 deletions(-)

I have no problem with this patch going in.  I 100% agree we need to
make sure we do have a test specifically to cover the functionality in
buildsym_compunit::record_line (which your new test does), so that's
good.

My only slight complaint would be that this patch is still phrased
around an empty line being somehow prohibited, and I'm still not
convinced that's the case (unless there is part of DWARF I'm
missing that says "a line shall not be empty".

I'd name the test something like "dw2-empty-line-table-entry" or
similar, then make the following change:

> 
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-dw-lne-end-sequence.exp b/gdb/testsuite/gdb.dwarf2/dw2-bad-dw-lne-end-sequence.exp
> new file mode 100644
> index 0000000000..b01f82ce85
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-dw-lne-end-sequence.exp
> @@ -0,0 +1,116 @@
> +# Copyright 2020 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/>.
> +
> +# Test that line table entries with the same address as an DW_LNE_end_sequence
> +# are ignored.
> +
> +load_lib dwarf.exp
> +
> +# This test can only be run on targets which support DWARF-2 and use gas.
> +if {![dwarf2_support]} {
> +    verbose "Skipping bad DW_LNE_end_sequence test."
> +    return 0
> +}
> +
> +standard_testfile main.c .dwarf.S
> +
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> +    global srcdir subdir srcfile srcfile2
> +    declare_labels L
> +
> +    # Find start address and length for our functions.
> +    set main_func \
> +	[function_range main [list ${srcdir}/${subdir}/$srcfile]]
> +
> +    cu {} {
> +	compile_unit {
> +	    {language @DW_LANG_C}
> +	    {name main.c}
> +	    {stmt_list $L DW_FORM_sec_offset}
> +	} {
> +	    subprogram {
> +		{external 1 flag}
> +		{name main}
> +	    }
> +	}
> +    }
> +
> +    lines {version 2} L {
> +	include_dir "${srcdir}/${subdir}"
> +	file_name "$srcfile" 1
> +
> +	# Generate simple line table program.  The last DW_LNS_copy is
> +	# problematic because it has the same address as the
> +	# DW_LNE_end_sequence.  We'll test that it's ignored.

    # Generate simple line table program.  The last DW_LNS_copy
    # results in an empty line as the following DW_LNE_end_sequence is
    # at the same address.  We'll test that the empty line is ignored.

Thanks,
Andrew

> +	program {
> +	    {DW_LNE_set_address [lindex $main_func 0]}
> +	    {DW_LNS_advance_line 10}
> +	    {DW_LNS_copy}
> +	    {DW_LNS_advance_pc [lindex $main_func 1]}
> +	    {DW_LNS_advance_line 19}
> +	    {DW_LNS_copy}
> +	    {DW_LNE_end_sequence}
> +	}
> +    }
> +}
> +
> +if { [prepare_for_testing "failed to prepare" ${testfile} \
> +	  [list $srcfile $asm_file] {nodebug}] } {
> +    return -1
> +}
> +
> +gdb_test_no_output "set auto-solib-add off"
> +
> +if ![runto_main] {
> +    return -1
> +}
> +
> +gdb_test_no_output "maint expand-symtabs gdb.dwarf2/main.c" \
> +    "maint expand-symtabs"
> +
> +set header_re \
> +    [multi_line \
> +	 ".*linetable: \\(\\(struct linetable \\*\\) $hex\\):" \
> +	 "INDEX\[ \t\]+LINE\[ \t\]+ADDRESS\[ \t\]+IS-STMT *" \
> +	 ""]
> +
> +set end_of_sequence_re \
> +    "^$decimal\[ \t\]+END\[ \t\]+$hex\(\[ \t\]+Y\)? *\r\n"
> +
> +set ok 1
> +set cmd "maint info line-table gdb.dwarf2/main.c"
> +gdb_test_multiple $cmd "bad ops ignored" {
> +    -re "^$decimal\[ \t\]+($decimal)\[ \t\]+$hex\(\[ \t\]+Y\)? *\r\n" {
> +	set line $expect_out(1,string)
> +	if { $line != 11 } {
> +	    set ok 0
> +	}
> +	exp_continue
> +    }
> +    -re "$end_of_sequence_re" {
> +	exp_continue
> +    }
> +    -re "^$gdb_prompt $" {
> +	if { $ok } {
> +	    pass $gdb_test_name
> +	} else {
> +	    fail $gdb_test_name
> +	}
> +    }
> +    -re $header_re {
> +	exp_continue
> +    }
> +}
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-elf.exp b/gdb/testsuite/gdb.dwarf2/dw2-bad-elf.exp
> index f6fe54a634..88a522ad63 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-bad-elf.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-elf.exp
> @@ -126,8 +126,6 @@ Dwarf::assemble $asm_file {
>  	    {DW_LNS_advance_line 10}
>  	    {DW_LNS_copy}
>  	    {DW_LNS_advance_pc [lindex $main_result 1]}
> -	    {DW_LNS_advance_line 19}
> -	    {DW_LNS_copy}
>  	    {DW_LNE_end_sequence}
>  	}
>      }
> @@ -142,8 +140,6 @@ Dwarf::assemble $asm_file {
>  	    {DW_LNS_advance_line 5}
>  	    {DW_LNS_copy}
>  	    {DW_LNS_advance_pc 64}
> -	    {DW_LNS_advance_line 8}
> -	    {DW_LNS_copy}
>  	    {DW_LNE_end_sequence}
>  	}
>      }
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp b/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp
> index 0de71f29d5..3fc142661b 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp
> @@ -146,7 +146,6 @@ proc out_line { name cu_dir cu_name line_dir line_name } {
>  	.uleb128	${addr_len}+1
>  	.byte		2
>  	.${addr_len}byte ${name}_end
> -	.byte		1	/* DW_LNS_copy */
>  	.byte		0	/* DW_LNE_end_of_sequence */
>  	.uleb128	1
>  	.byte		1
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.exp b/gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.exp
> index 4fcc3cfeac..1efe49d009 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.exp
> @@ -114,7 +114,6 @@ Dwarf::assemble $asm_file {
>  	    {DW_LNS_copy}
>  
>  	    {DW_LNE_set_address line_label_3}
> -	    {DW_LNS_copy}
>  	    {DW_LNE_end_sequence}
>  	}
>      }
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
> index 92f8f6cecb..9e4ebf7f3c 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
> @@ -88,24 +88,18 @@ Dwarf::assemble $asm_file {
>  	    {DW_LNS_advance_line 10}
>  	    {DW_LNS_copy}
>  	    {DW_LNS_advance_pc [lindex $main_func 1]}
> -	    {DW_LNS_advance_line 19}
> -	    {DW_LNS_copy}
>  	    {DW_LNE_end_sequence}
>  
>  	    {DW_LNE_set_address [lindex $frame2_func 0]}
>  	    {DW_LNS_advance_line 20}
>  	    {DW_LNS_copy}
>  	    {DW_LNS_advance_pc [lindex $frame2_func 1]}
> -	    {DW_LNS_advance_line 29}
> -	    {DW_LNS_copy}
>  	    {DW_LNE_end_sequence}
>  
>  	    {DW_LNE_set_address [lindex $frame3_func 0]}
>  	    {DW_LNS_advance_line 30}
>  	    {DW_LNS_copy}
>  	    {DW_LNS_advance_pc [lindex $frame3_func 1]}
> -	    {DW_LNS_advance_line 39}
> -	    {DW_LNS_copy}
>  	    {DW_LNE_end_sequence}
>  	}
>      }
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp
> index 0a95adc5bc..eafd9f65fb 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp
> @@ -143,8 +143,6 @@ proc do_test {suffix} {
>  		{DW_LNS_advance_line [expr [gdb_get_line_number "main return"] - [gdb_get_line_number "main foo call"]]}
>  		{DW_LNS_copy}
>  		{DW_LNE_set_address $main_end}
> -		{DW_LNS_advance_line [expr [gdb_get_line_number "main end"] - [gdb_get_line_number "main return"] + 1]}
> -		{DW_LNS_copy}
>  		{DW_LNE_end_sequence}
>  
>  		{DW_LNE_set_address $foo_start}
> @@ -160,24 +158,18 @@ proc do_test {suffix} {
>  		{DW_LNS_advance_line [expr [gdb_get_line_number "foo end"] - [gdb_get_line_number "foo foo_cold call"]]}
>  		{DW_LNS_copy}
>  		{DW_LNE_set_address $foo_end}
> -		{DW_LNS_advance_line 1}
> -		{DW_LNS_copy}
>  		{DW_LNE_end_sequence}
>  
>  		{DW_LNE_set_address $bar_start}
>  		{DW_LNS_advance_line [expr [gdb_get_line_number "bar end"] - 1]}
>  		{DW_LNS_copy}
>  		{DW_LNS_advance_pc $bar_len}
> -		{DW_LNS_advance_line 1}
> -		{DW_LNS_copy}
>  		{DW_LNE_end_sequence}
>  
>  		{DW_LNE_set_address $baz_start}
>  		{DW_LNS_advance_line [expr [gdb_get_line_number "baz end"] - 1]}
>  		{DW_LNS_copy}
>  		{DW_LNS_advance_pc $baz_len}
> -		{DW_LNS_advance_line 1}
> -		{DW_LNS_copy}
>  		{DW_LNE_end_sequence}
>  
>  		{DW_LNE_set_address $foo_cold_start}
> @@ -190,8 +182,6 @@ proc do_test {suffix} {
>  		{DW_LNS_advance_line [expr [gdb_get_line_number "foo_cold end"] - [gdb_get_line_number "foo_cold baz call"]]}
>  		{DW_LNS_copy}
>  		{DW_LNE_set_address $foo_cold_end}
> -		{DW_LNS_advance_line 1}
> -		{DW_LNS_copy}
>  		{DW_LNE_end_sequence}
>  	    }
>  	}