[2/2] gdb: Add support for tracking the DWARF line table is-stmt field

Message ID 94e33268f64060fc887670f4ee5ed524050cbcc7.1580902412.git.andrew.burgess@embecosm.com
State New, archived
Headers

Commit Message

Andrew Burgess Feb. 5, 2020, 11:37 a.m. UTC
  This commit brings support for the DWARF line table is_stmt field to
GDB.  The is_stmt field is used by the compiler when a single source
line is split into multiple assembler instructions, especially if the
assembler instructions are interleaved with instruction from other
source lines.

The compiler will set the is_stmt flag false from some instructions
from the source lines, these instructions are not a good place to
insert a breakpoint in order to stop at the source line.
Instructions which are marked with the is_stmt flag true are a good
place to insert a breakpoint for that source line.

Currently GDB ignores all instructions for which is_stmt is false.
This is fine in a lot of cases, however, there are some cases where
this means the debug experience is not as good as it could be.

Consider stopping at a random instruction, currently this instruction
will be attributed to the last line table entry before this point for
which is_stmt was true - as these are the only line table entries that
GDB tracks.  This can easily be incorrect in code with even a low
level of optimisation.

With is_stmt tracking in place, when stopping at a random instruction
we now attribute the instruction back to the real source line, even
when is_stmt is false for that instruction in the line table.

When inserting breakpoints we still select line table entries for
which is_stmt is true, so the breakpoint placing behaviour should not
change.

When stepping though code (at the line level, not the instruction
level) we will still stop at instruction where is_stmt is true, I
think this is more likely to be the desired behaviour.

Instruction stepping is, of course, unchanged, stepping one
instruction at a time, but we should now report more accurate line
table information with each instruction step.

The original motivation for this work was a patch posted by Bernd
here:
  https://sourceware.org/ml/gdb-patches/2019-11/msg00792.html

As part of that thread it was suggested that many issues would be
resolved if GDB supported line table views, this isn't something I've
attempted in this patch, though reading the spec, it seems like this
would be a useful feature to support in GDB in the future.  The spec
is here:
  http://dwarfstd.org/ShowIssue.php?issue=170427.1

And Bernd gives a brief description of the benefits here:
  https://sourceware.org/ml/gdb-patches/2020-01/msg00147.html

With that all said, I think that there is benefit to having proper
is_stmt support regardless of whether we have views support, so I
think we should consider getting this (or something like this) in
first, and then building view support on top of this.

The gdb.cp/next-inline.exp test is based off a test proposed by Bernd
Edlinger in message:
  https://sourceware.org/ml/gdb-patches/2019-12/msg00842.html

gdb/ChangeLog:

	* buildsym-legacy.c (record_line): Pass extra parameter to
	record_line.
	* buildsym.c (buildsym_compunit::record_line): Take an extra
	parameter, reduce duplication in the line table, and record the
	is_stmt flag in the line table.
	* buildsym.h (buildsym_compunit::record_line): Add extra
	parameter.
	* disasm.c (do_mixed_source_and_assembly_deprecated): Ignore
	non-statement lines.
	* dwarf2read.c (dwarf_record_line_1): Add extra parameter, pass
	this to the symtab builder.
	(dwarf_finish_line): Pass extra parameter to dwarf_record_line_1.
	(lnp_state_machine::record_line): Pass a suitable is_stmt flag
	through to dwarf_record_line_1.
	* infrun.c (process_event_stop_test): When stepping, don't stop at
	a non-statement instruction, and only refresh the step info when
	we land in the middle of a line's range.
	* jit.c (jit_symtab_line_mapping_add_impl): Initialise is_stmt
	field.
	* record-btrace.c (btrace_find_line_range): Only record lines
	marked as is-statement.
	* stack.c (frame_show_address): Show the frame address if we are
	in a non-statement sal.
	* symmisc.c (dump_symtab_1): Print the is_stmt flag.
	(maintenance_print_one_line_table): Print a header for the is_stmt
	column, and include is_stmt information in the output.
	* symtab.c (find_pc_sect_line): Find lines marked as statements in
	preference to non-statements.
	(find_pcs_for_symtab_line): Prefer is-statement entries.
	(find_line_common): Likewise.
	* symtab.h (struct linetable_entry): Add is_stmt field.
	(struct symtab_and_line): Likewise.
	* xcoffread.c (arrange_linetable): Initialise is_stmt field when
	arranging the line table.

gdb/testsuite/ChangeLog:

	* gdb.cp/next-inline.cc: New file.
	* gdb.cp/next-inline.exp: New file.
	* gdb.cp/next-inline.h: New file.
	* gdb.dwarf2/dw2-is-stmt.c: New file.
	* gdb.dwarf2/dw2-is-stmt.exp: New file.
	* gdb.dwarf2/dw2-is-stmt-2.c: New file.
	* gdb.dwarf2/dw2-is-stmt-2.exp: New file.
	* gdb.dwarf2/dw2-ranges-base.exp: Update line table pattern.

Change-Id: Id81de4c7d7e83558abe050f37a5c5927c42967e3
---
 gdb/ChangeLog                                |  37 ++++
 gdb/buildsym-legacy.c                        |   4 +-
 gdb/buildsym.c                               |  14 +-
 gdb/buildsym.h                               |   3 +-
 gdb/disasm.c                                 |   6 +
 gdb/dwarf2read.c                             |  13 +-
 gdb/infrun.c                                 |  42 +++--
 gdb/jit.c                                    |   1 +
 gdb/record-btrace.c                          |  11 +-
 gdb/stack.c                                  |   3 +-
 gdb/symmisc.c                                |  10 +-
 gdb/symtab.c                                 |  25 ++-
 gdb/symtab.h                                 |  10 +
 gdb/testsuite/ChangeLog                      |  11 ++
 gdb/testsuite/gdb.cp/next-inline.cc          |  65 +++++++
 gdb/testsuite/gdb.cp/next-inline.exp         |  70 +++++++
 gdb/testsuite/gdb.cp/next-inline.h           |  38 ++++
 gdb/testsuite/gdb.dwarf2/dw2-is-stmt-2.c     |  99 ++++++++++
 gdb/testsuite/gdb.dwarf2/dw2-is-stmt-2.exp   | 265 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.dwarf2/dw2-is-stmt.c       |  61 ++++++
 gdb/testsuite/gdb.dwarf2/dw2-is-stmt.exp     | 267 +++++++++++++++++++++++++++
 gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp |   6 +-
 gdb/xcoffread.c                              |   4 +
 23 files changed, 1036 insertions(+), 29 deletions(-)
 create mode 100644 gdb/testsuite/gdb.cp/next-inline.cc
 create mode 100644 gdb/testsuite/gdb.cp/next-inline.exp
 create mode 100644 gdb/testsuite/gdb.cp/next-inline.h
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-is-stmt-2.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-is-stmt-2.exp
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-is-stmt.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-is-stmt.exp
  

Comments

Bernd Edlinger Feb. 5, 2020, 5:55 p.m. UTC | #1
On 2/5/20 12:37 PM, Andrew Burgess wrote:
> This commit brings support for the DWARF line table is_stmt field to
> GDB.  The is_stmt field is used by the compiler when a single source
> line is split into multiple assembler instructions, especially if the
> assembler instructions are interleaved with instruction from other
> source lines.
> 
> The compiler will set the is_stmt flag false from some instructions
> from the source lines, these instructions are not a good place to
> insert a breakpoint in order to stop at the source line.
> Instructions which are marked with the is_stmt flag true are a good
> place to insert a breakpoint for that source line.
> 
> Currently GDB ignores all instructions for which is_stmt is false.
> This is fine in a lot of cases, however, there are some cases where
> this means the debug experience is not as good as it could be.
> 
> Consider stopping at a random instruction, currently this instruction
> will be attributed to the last line table entry before this point for
> which is_stmt was true - as these are the only line table entries that
> GDB tracks.  This can easily be incorrect in code with even a low
> level of optimisation.
> 
> With is_stmt tracking in place, when stopping at a random instruction
> we now attribute the instruction back to the real source line, even
> when is_stmt is false for that instruction in the line table.
> 
> When inserting breakpoints we still select line table entries for
> which is_stmt is true, so the breakpoint placing behaviour should not
> change.
> 
> When stepping though code (at the line level, not the instruction
> level) we will still stop at instruction where is_stmt is true, I
> think this is more likely to be the desired behaviour.
> 
> Instruction stepping is, of course, unchanged, stepping one
> instruction at a time, but we should now report more accurate line
> table information with each instruction step.
> 
> The original motivation for this work was a patch posted by Bernd
> here:
>   https://sourceware.org/ml/gdb-patches/2019-11/msg00792.html
> 
> As part of that thread it was suggested that many issues would be
> resolved if GDB supported line table views, this isn't something I've
> attempted in this patch, though reading the spec, it seems like this
> would be a useful feature to support in GDB in the future.  The spec
> is here:
>   http://dwarfstd.org/ShowIssue.php?issue=170427.1
> 
> And Bernd gives a brief description of the benefits here:
>   https://sourceware.org/ml/gdb-patches/2020-01/msg00147.html
> 
> With that all said, I think that there is benefit to having proper
> is_stmt support regardless of whether we have views support, so I
> think we should consider getting this (or something like this) in
> first, and then building view support on top of this.
> 
> The gdb.cp/next-inline.exp test is based off a test proposed by Bernd
> Edlinger in message:
>   https://sourceware.org/ml/gdb-patches/2019-12/msg00842.html
> 

Thanks Andrew, I played a bit with debugging the gcc internals,
and I think the debug-experience is good, the problem with step
over are completely gone, also in the original context.

Just a few comments on the patch follow below.

> gdb/ChangeLog:
> 
> 	* buildsym-legacy.c (record_line): Pass extra parameter to
> 	record_line.
> 	* buildsym.c (buildsym_compunit::record_line): Take an extra
> 	parameter, reduce duplication in the line table, and record the
> 	is_stmt flag in the line table.
> 	* buildsym.h (buildsym_compunit::record_line): Add extra
> 	parameter.
> 	* disasm.c (do_mixed_source_and_assembly_deprecated): Ignore
> 	non-statement lines.
> 	* dwarf2read.c (dwarf_record_line_1): Add extra parameter, pass
> 	this to the symtab builder.
> 	(dwarf_finish_line): Pass extra parameter to dwarf_record_line_1.
> 	(lnp_state_machine::record_line): Pass a suitable is_stmt flag
> 	through to dwarf_record_line_1.
> 	* infrun.c (process_event_stop_test): When stepping, don't stop at
> 	a non-statement instruction, and only refresh the step info when
> 	we land in the middle of a line's range.

did you mean, when we don't land in the middle of a line ?

> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -330,7 +330,7 @@ frame_show_address (struct frame_info *frame,
>        return false;
>      }
>  
> -  return get_frame_pc (frame) != sal.pc;
> +  return get_frame_pc (frame) != sal.pc || !sal.is_stmt;
>  }
>  
>  /* See frame.h.  */
> @@ -1036,6 +1036,7 @@ print_frame_info (const frame_print_options &fp_opts,
>    int location_print;
>    struct ui_out *uiout = current_uiout;
>  
> +
>    if (!current_uiout->is_mi_like_p ()
>        && fp_opts.print_frame_info != print_frame_info_auto)
>      {

Is this white space change intentional?




Thanks
Bernd.
  
Luis Machado Feb. 6, 2020, 9 a.m. UTC | #2
Hi,

I still need to check the patch itself, but i had a question about one 
particular paragraph...

On 2/5/20 8:37 AM, Andrew Burgess wrote:
> This commit brings support for the DWARF line table is_stmt field to
> GDB.  The is_stmt field is used by the compiler when a single source
> line is split into multiple assembler instructions, especially if the
> assembler instructions are interleaved with instruction from other
> source lines.
> 
> The compiler will set the is_stmt flag false from some instructions
> from the source lines, these instructions are not a good place to
> insert a breakpoint in order to stop at the source line.
> Instructions which are marked with the is_stmt flag true are a good
> place to insert a breakpoint for that source line.
> 
> Currently GDB ignores all instructions for which is_stmt is false.
> This is fine in a lot of cases, however, there are some cases where
> this means the debug experience is not as good as it could be.
> 
> Consider stopping at a random instruction, currently this instruction
> will be attributed to the last line table entry before this point for
> which is_stmt was true - as these are the only line table entries that
> GDB tracks.  This can easily be incorrect in code with even a low
> level of optimisation.
> 
> With is_stmt tracking in place, when stopping at a random instruction
> we now attribute the instruction back to the real source line, even
> when is_stmt is false for that instruction in the line table.
> 
> When inserting breakpoints we still select line table entries for
> which is_stmt is true, so the breakpoint placing behaviour should not
> change.
> 
> When stepping though code (at the line level, not the instruction
> level) we will still stop at instruction where is_stmt is true, I
> think this is more likely to be the desired behaviour.

Considering a block of continuous instructions that map to the same 
source line, will line-stepping stop at each one of these instructions 
if is_stmt is true? As opposed to stepping over the the whole block 
until we see a line change?

I'm wondering if this would help this bug 
(https://sourceware.org/bugzilla/show_bug.cgi?id=21221) in any way.
  
Bernd Edlinger Feb. 10, 2020, 6:30 p.m. UTC | #3
Hi,

I just did a rebase of your patch, (and was surprised that it worked),
but got an interesting message from git:

First, rewinding head to replay your work on top of it...
Applying: - is-stmt patch
Using index info to reconstruct a base tree...
A	gdb/dwarf2read.c
<stdin>:874: space before tab in indent.
 		 [expr [gdb_get_line_number "main end"] \
<stdin>:1128: space before tab in indent.
 		 [expr [gdb_get_line_number "main, set var to 0"] \
<stdin>:1143: space before tab in indent.
 		 [expr [gdb_get_line_number "main end"] \
warning: 3 lines add whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging gdb/dwarf2/read.c


and indeed there are SPACE followed by TAB in
gdb/testsuite/gdb.dwarf2/dw2-is-stmt-2.exp

not sure if that is only because I copied these lines
out of the E-mail and messed it up by doing so, or if the whitespace
is wrong in the original patch as well.


Thanks
Bernd.
  
Andrew Burgess Feb. 11, 2020, 1:57 p.m. UTC | #4
* Bernd Edlinger <bernd.edlinger@hotmail.de> [2020-02-05 17:55:37 +0000]:

> On 2/5/20 12:37 PM, Andrew Burgess wrote:
> > This commit brings support for the DWARF line table is_stmt field to
> > GDB.  The is_stmt field is used by the compiler when a single source
> > line is split into multiple assembler instructions, especially if the
> > assembler instructions are interleaved with instruction from other
> > source lines.
> > 
> > The compiler will set the is_stmt flag false from some instructions
> > from the source lines, these instructions are not a good place to
> > insert a breakpoint in order to stop at the source line.
> > Instructions which are marked with the is_stmt flag true are a good
> > place to insert a breakpoint for that source line.
> > 
> > Currently GDB ignores all instructions for which is_stmt is false.
> > This is fine in a lot of cases, however, there are some cases where
> > this means the debug experience is not as good as it could be.
> > 
> > Consider stopping at a random instruction, currently this instruction
> > will be attributed to the last line table entry before this point for
> > which is_stmt was true - as these are the only line table entries that
> > GDB tracks.  This can easily be incorrect in code with even a low
> > level of optimisation.
> > 
> > With is_stmt tracking in place, when stopping at a random instruction
> > we now attribute the instruction back to the real source line, even
> > when is_stmt is false for that instruction in the line table.
> > 
> > When inserting breakpoints we still select line table entries for
> > which is_stmt is true, so the breakpoint placing behaviour should not
> > change.
> > 
> > When stepping though code (at the line level, not the instruction
> > level) we will still stop at instruction where is_stmt is true, I
> > think this is more likely to be the desired behaviour.
> > 
> > Instruction stepping is, of course, unchanged, stepping one
> > instruction at a time, but we should now report more accurate line
> > table information with each instruction step.
> > 
> > The original motivation for this work was a patch posted by Bernd
> > here:
> >   https://sourceware.org/ml/gdb-patches/2019-11/msg00792.html
> > 
> > As part of that thread it was suggested that many issues would be
> > resolved if GDB supported line table views, this isn't something I've
> > attempted in this patch, though reading the spec, it seems like this
> > would be a useful feature to support in GDB in the future.  The spec
> > is here:
> >   http://dwarfstd.org/ShowIssue.php?issue=170427.1
> > 
> > And Bernd gives a brief description of the benefits here:
> >   https://sourceware.org/ml/gdb-patches/2020-01/msg00147.html
> > 
> > With that all said, I think that there is benefit to having proper
> > is_stmt support regardless of whether we have views support, so I
> > think we should consider getting this (or something like this) in
> > first, and then building view support on top of this.
> > 
> > The gdb.cp/next-inline.exp test is based off a test proposed by Bernd
> > Edlinger in message:
> >   https://sourceware.org/ml/gdb-patches/2019-12/msg00842.html
> > 
> 
> Thanks Andrew, I played a bit with debugging the gcc internals,
> and I think the debug-experience is good, the problem with step
> over are completely gone, also in the original context.
> 
> Just a few comments on the patch follow below.
> 
> > gdb/ChangeLog:
> > 
> > 	* buildsym-legacy.c (record_line): Pass extra parameter to
> > 	record_line.
> > 	* buildsym.c (buildsym_compunit::record_line): Take an extra
> > 	parameter, reduce duplication in the line table, and record the
> > 	is_stmt flag in the line table.
> > 	* buildsym.h (buildsym_compunit::record_line): Add extra
> > 	parameter.
> > 	* disasm.c (do_mixed_source_and_assembly_deprecated): Ignore
> > 	non-statement lines.
> > 	* dwarf2read.c (dwarf_record_line_1): Add extra parameter, pass
> > 	this to the symtab builder.
> > 	(dwarf_finish_line): Pass extra parameter to dwarf_record_line_1.
> > 	(lnp_state_machine::record_line): Pass a suitable is_stmt flag
> > 	through to dwarf_record_line_1.
> > 	* infrun.c (process_event_stop_test): When stepping, don't stop at
> > 	a non-statement instruction, and only refresh the step info when
> > 	we land in the middle of a line's range.
> 
> did you mean, when we don't land in the middle of a line ?

No, in this case I did write the correct thing.

So GDB had/has some code designed to "improve" the handling of looping
constructs.  I think the idea is to handle cases like this:

  0x100      LN=5
  0x104 <-\
  0x108   |
  0x10c   |  LN=6
  0x110   |
  0x114 --/

So when line 6 branches back to the middle of line 5, we don't stop
(because we are in the middle of line 5), but we do want to stop at
the start of line 6, so we "reset" the starting point back to line 5.

This is done by calling set_step_info in process_event_stop_test, in
infrun.c.

What we actually did though was whenever we were at an address that
didn't exactly match a SAL (in the example above, marked LN=5, LN=6)
we called set_step_info.  This worked fine, at address 0x104 we reset
back to LN=5, same at address 0x108.

However, in the new world things can look different, for example, like
this:

  0x100      LN=5,STMT=1
  0x104
  0x108      LN=6,STMT=0
  0x10c      LN=6,STMT=1
  0x110
  0x114

In this world, when we move forward to 0x100 we don't have a matching
SAL, so we get the SAL for address 0x100.  We can then safely call
set_step_info like we did before, that's fine.  But when we step to
0x108 we do now have a matching SAL, but we don't want to stop yet as
this address is 'STMT=0'.  However, if we call set_step_info then GDB
is going to think we are stepping away from LN=6, and so will refuse
to stop at address 0x10c.

The proposal in this commit is that if we land at an address which
doesn't specifically match a SAL, 0x104, 0x110, 0x114 in the above
example, then we do call set_step_info, but otherwise we don't.

There are going to be situations where the behaviour of GDB changes
after this patch, but I don't think we can avoid that.  What we had
previously was just a heuristic.  I think enabling this new
information in GDB will allow us to do better overall, so I think we
should make this change and fix any issues as they show up.

> 
> > --- a/gdb/stack.c
> > +++ b/gdb/stack.c
> > @@ -330,7 +330,7 @@ frame_show_address (struct frame_info *frame,
> >        return false;
> >      }
> >  
> > -  return get_frame_pc (frame) != sal.pc;
> > +  return get_frame_pc (frame) != sal.pc || !sal.is_stmt;
> >  }
> >  
> >  /* See frame.h.  */
> > @@ -1036,6 +1036,7 @@ print_frame_info (const frame_print_options &fp_opts,
> >    int location_print;
> >    struct ui_out *uiout = current_uiout;
> >  
> > +
> >    if (!current_uiout->is_mi_like_p ()
> >        && fp_opts.print_frame_info != print_frame_info_auto)
> >      {
> 
> Is this white space change intentional?

Ooops.  Fixed.

I also fixed the space before tab issue you pointed out in another
mail.

I see you have a patch that depends on this one, but I'd like to leave
this on the mailing list for a little longer before I merge it to give
others a chance to comment.

I'll probably merge this over the weekend if there's no other
feedback.

Thanks,
Andrew
  

Patch

diff --git a/gdb/buildsym-legacy.c b/gdb/buildsym-legacy.c
index bf19d2d90a7..d9c27ebc956 100644
--- a/gdb/buildsym-legacy.c
+++ b/gdb/buildsym-legacy.c
@@ -252,7 +252,9 @@  void
 record_line (struct subfile *subfile, int line, CORE_ADDR pc)
 {
   gdb_assert (buildsym_compunit != nullptr);
-  buildsym_compunit->record_line (subfile, line, pc);
+  /* Assume every line entry is a statement start, that is a good place to
+     put a breakpoint for that line number.  */
+  buildsym_compunit->record_line (subfile, line, pc, true);
 }
 
 /* Start a new symtab for a new source file in OBJFILE.  Called, for example,
diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 4965b552b32..7fd256fecce 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -666,7 +666,7 @@  buildsym_compunit::pop_subfile ()
 
 void
 buildsym_compunit::record_line (struct subfile *subfile, int line,
-				CORE_ADDR pc)
+				CORE_ADDR pc, bool is_stmt)
 {
   struct linetable_entry *e;
 
@@ -681,6 +681,17 @@  buildsym_compunit::record_line (struct subfile *subfile, int line,
       m_have_line_numbers = true;
     }
 
+  /* If we have a duplicate for the previous entry then ignore the new
+     entry, except, if the new entry is setting the is_stmt flag, then
+     ensure the previous entry respects the new setting.  */
+  e = subfile->line_vector->item + subfile->line_vector->nitems - 1;
+  if (e->line == line && e->pc == pc)
+    {
+      if (is_stmt && !e->is_stmt)
+	e->is_stmt = 1;
+      return;
+    }
+
   if (subfile->line_vector->nitems + 1 >= subfile->line_vector_length)
     {
       subfile->line_vector_length *= 2;
@@ -716,6 +727,7 @@  buildsym_compunit::record_line (struct subfile *subfile, int line,
 
   e = subfile->line_vector->item + subfile->line_vector->nitems++;
   e->line = line;
+  e->is_stmt = is_stmt ? 1 : 0;
   e->pc = pc;
 }
 
diff --git a/gdb/buildsym.h b/gdb/buildsym.h
index c8e1bd0d5a7..c768a4c2dae 100644
--- a/gdb/buildsym.h
+++ b/gdb/buildsym.h
@@ -187,7 +187,8 @@  struct buildsym_compunit
 
   const char *pop_subfile ();
 
-  void record_line (struct subfile *subfile, int line, CORE_ADDR pc);
+  void record_line (struct subfile *subfile, int line, CORE_ADDR pc,
+		    bool is_stmt);
 
   struct compunit_symtab *get_compunit_symtab ()
   {
diff --git a/gdb/disasm.c b/gdb/disasm.c
index e45c8400689..143ba2f59b9 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -376,6 +376,12 @@  do_mixed_source_and_assembly_deprecated
       if (le[i].line == le[i + 1].line && le[i].pc == le[i + 1].pc)
 	continue;		/* Ignore duplicates.  */
 
+      /* Ignore non-statement line table entries.  This means we print the
+	 source line at the place where GDB would insert a breakpoint for
+	 that line, which seems more intuitive.  */
+      if (le[i].is_stmt == 0)
+	continue;
+
       /* Skip any end-of-function markers.  */
       if (le[i].line == 0)
 	continue;
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index dafe01d94a0..cbee4abcb74 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -21222,7 +21222,7 @@  dwarf_record_line_p (struct dwarf2_cu *cu,
 
 static void
 dwarf_record_line_1 (struct gdbarch *gdbarch, struct subfile *subfile,
-		     unsigned int line, CORE_ADDR address,
+		     unsigned int line, CORE_ADDR address, bool is_stmt,
 		     struct dwarf2_cu *cu)
 {
   CORE_ADDR addr = gdbarch_addr_bits_remove (gdbarch, address);
@@ -21236,7 +21236,7 @@  dwarf_record_line_1 (struct gdbarch *gdbarch, struct subfile *subfile,
     }
 
   if (cu != nullptr)
-    cu->get_builder ()->record_line (subfile, line, addr);
+    cu->get_builder ()->record_line (subfile, line, addr, is_stmt);
 }
 
 /* Subroutine of dwarf_decode_lines_1 to simplify it.
@@ -21259,7 +21259,7 @@  dwarf_finish_line (struct gdbarch *gdbarch, struct subfile *subfile,
 			  paddress (gdbarch, address));
     }
 
-  dwarf_record_line_1 (gdbarch, subfile, 0, address, cu);
+  dwarf_record_line_1 (gdbarch, subfile, 0, address, true, cu);
 }
 
 void
@@ -21286,8 +21286,7 @@  lnp_state_machine::record_line (bool end_sequence)
   else if (m_op_index == 0 || end_sequence)
     {
       fe->included_p = 1;
-      if (m_record_lines_p
-	  && (producer_is_codewarrior (m_cu) || m_is_stmt || end_sequence))
+      if (m_record_lines_p)
 	{
 	  if (m_last_subfile != m_cu->get_builder ()->get_current_subfile ()
 	      || end_sequence)
@@ -21298,6 +21297,8 @@  lnp_state_machine::record_line (bool end_sequence)
 
 	  if (!end_sequence)
 	    {
+	      bool is_stmt = producer_is_codewarrior (m_cu) || m_is_stmt;
+
 	      if (dwarf_record_line_p (m_cu, m_line, m_last_line,
 				       m_line_has_non_zero_discriminator,
 				       m_last_subfile))
@@ -21305,7 +21306,7 @@  lnp_state_machine::record_line (bool end_sequence)
 		  buildsym_compunit *builder = m_cu->get_builder ();
 		  dwarf_record_line_1 (m_gdbarch,
 				       builder->get_current_subfile (),
-				       m_line, m_address,
+				       m_line, m_address, is_stmt,
 				       m_currently_recording_lines ? m_cu : nullptr);
 		}
 	      m_last_subfile = m_cu->get_builder ()->get_current_subfile ();
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 3e846f8e680..3919de81f90 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7146,19 +7146,31 @@  process_event_stop_test (struct execution_control_state *ecs)
       return;
     }
 
+  bool refresh_step_info = true;
   if ((ecs->event_thread->suspend.stop_pc == stop_pc_sal.pc)
       && (ecs->event_thread->current_line != stop_pc_sal.line
  	  || ecs->event_thread->current_symtab != stop_pc_sal.symtab))
     {
-      /* We are at the start of a different line.  So stop.  Note that
-         we don't stop if we step into the middle of a different line.
-         That is said to make things like for (;;) statements work
-         better.  */
-      if (debug_infrun)
-	 fprintf_unfiltered (gdb_stdlog,
-			     "infrun: stepped to a different line\n");
-      end_stepping_range (ecs);
-      return;
+      if (stop_pc_sal.is_stmt)
+	{
+	  /* We are at the start of a different line.  So stop.  Note that
+	     we don't stop if we step into the middle of a different line.
+	     That is said to make things like for (;;) statements work
+	     better.  */
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"infrun: stepped to a different line\n");
+	  end_stepping_range (ecs);
+	  return;
+	}
+      else
+	{
+	  refresh_step_info = false;
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"infrun: stepped to a different line, but "
+				"it's not the start of a statement\n");
+	}
     }
 
   /* We aren't done stepping.
@@ -7166,12 +7178,20 @@  process_event_stop_test (struct execution_control_state *ecs)
      Optimize by setting the stepping range to the line.
      (We might not be in the original line, but if we entered a
      new line in mid-statement, we continue stepping.  This makes
-     things like for(;;) statements work better.)  */
+     things like for(;;) statements work better.)
+
+     If we entered a SAL that indicates a non-statement line table entry,
+     then we update the stepping range, but we don't update the step info,
+     which includes things like the line number we are stepping away from.
+     This means we will stop when we find a line table entry that is marked
+     as is-statement, even if it matches the non-statement one we just
+     stepped into.   */
 
   ecs->event_thread->control.step_range_start = stop_pc_sal.pc;
   ecs->event_thread->control.step_range_end = stop_pc_sal.end;
   ecs->event_thread->control.may_range_step = 1;
-  set_step_info (frame, stop_pc_sal);
+  if (refresh_step_info)
+    set_step_info (frame, stop_pc_sal);
 
   if (debug_infrun)
      fprintf_unfiltered (gdb_stdlog, "infrun: keep going\n");
diff --git a/gdb/jit.c b/gdb/jit.c
index eeaab70bfe0..0d81951e4fd 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -578,6 +578,7 @@  jit_symtab_line_mapping_add_impl (struct gdb_symbol_callbacks *cb,
     {
       stab->linetable->item[i].pc = (CORE_ADDR) map[i].pc;
       stab->linetable->item[i].line = map[i].line;
+      stab->linetable->item[i].is_stmt = 1;
     }
 }
 
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index ef23a0b7af7..d3da8527c5c 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -718,7 +718,16 @@  btrace_find_line_range (CORE_ADDR pc)
   range = btrace_mk_line_range (symtab, 0, 0);
   for (i = 0; i < nlines - 1; i++)
     {
-      if ((lines[i].pc == pc) && (lines[i].line != 0))
+      /* The test of is_stmt here was added when the is_stmt field was
+	 introduced to the 'struct linetable_entry' structure.  This
+	 ensured that this loop maintained the same behaviour as before we
+	 introduced is_stmt.  That said, it might be that we would be
+	 better off not checking is_stmt here, this would lead to us
+	 possibly adding more line numbers to the range.  At the time this
+	 change was made I was unsure how to test this so chose to go with
+	 maintaining the existing experience.  */
+      if ((lines[i].pc == pc) && (lines[i].line != 0)
+	  && (lines[i].is_stmt == 1))
 	range = btrace_line_range_add (range, lines[i].line);
     }
 
diff --git a/gdb/stack.c b/gdb/stack.c
index af30405f29e..6ecb878c476 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -330,7 +330,7 @@  frame_show_address (struct frame_info *frame,
       return false;
     }
 
-  return get_frame_pc (frame) != sal.pc;
+  return get_frame_pc (frame) != sal.pc || !sal.is_stmt;
 }
 
 /* See frame.h.  */
@@ -1036,6 +1036,7 @@  print_frame_info (const frame_print_options &fp_opts,
   int location_print;
   struct ui_out *uiout = current_uiout;
 
+
   if (!current_uiout->is_mi_like_p ()
       && fp_opts.print_frame_info != print_frame_info_auto)
     {
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index a6a7e728c4a..3011bdecf72 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -305,6 +305,8 @@  dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile)
 	{
 	  fprintf_filtered (outfile, " line %d at ", l->item[i].line);
 	  fputs_filtered (paddress (gdbarch, l->item[i].pc), outfile);
+	  if (l->item[i].is_stmt)
+	    fprintf_filtered (outfile, "\t(stmt)");
 	  fprintf_filtered (outfile, "\n");
 	}
     }
@@ -991,8 +993,8 @@  maintenance_print_one_line_table (struct symtab *symtab, void *data)
 
       /* Leave space for 6 digits of index and line number.  After that the
 	 tables will just not format as well.  */
-      printf_filtered (_("%-6s %6s %s\n"),
-		       _("INDEX"), _("LINE"), _("ADDRESS"));
+      printf_filtered (_("%-6s %6s %s %s\n"),
+		       _("INDEX"), _("LINE"), _("ADDRESS"), _("IS-STMT"));
 
       for (i = 0; i < linetable->nitems; ++i)
 	{
@@ -1004,7 +1006,9 @@  maintenance_print_one_line_table (struct symtab *symtab, void *data)
 	    printf_filtered ("%6d ", item->line);
 	  else
 	    printf_filtered ("%6s ", _("END"));
-	  printf_filtered ("%s\n", core_addr_to_string (item->pc));
+	  printf_filtered ("%s%s\n",
+			   core_addr_to_string (item->pc),
+			   (item->is_stmt ? " Y" : ""));
 	}
     }
 
diff --git a/gdb/symtab.c b/gdb/symtab.c
index f456f4d852d..d6a7271f636 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3244,6 +3244,23 @@  find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent)
 	  best = prev;
 	  best_symtab = iter_s;
 
+	  /* If during the binary search we land on a non-statement entry,
+	     scan backward through entries at the same address to see if
+	     there is an entry marked as is-statement.  In theory this
+	     duplication should have been removed from the line table
+	     during construction, this is just a double check.  If the line
+	     table has had the duplication removed then this should be
+	     pretty cheap.  */
+	  if (!best->is_stmt)
+	    {
+	      struct linetable_entry *tmp = best;
+	      while (tmp > first && (tmp - 1)->pc == tmp->pc
+		     && (tmp - 1)->line != 0 && !tmp->is_stmt)
+		--tmp;
+	      if (tmp->is_stmt)
+		best = tmp;
+	    }
+
 	  /* Discard BEST_END if it's before the PC of the current BEST.  */
 	  if (best_end <= best->pc)
 	    best_end = 0;
@@ -3274,6 +3291,7 @@  find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent)
     }
   else
     {
+      val.is_stmt = best->is_stmt;
       val.symtab = best_symtab;
       val.line = best->line;
       val.pc = best->pc;
@@ -3442,7 +3460,8 @@  find_pcs_for_symtab_line (struct symtab *symtab, int line,
 	{
 	  struct linetable_entry *item = &SYMTAB_LINETABLE (symtab)->item[idx];
 
-	  if (*best_item == NULL || item->line < (*best_item)->line)
+	  if (*best_item == NULL
+	      || (item->line < (*best_item)->line && item->is_stmt))
 	    *best_item = item;
 
 	  break;
@@ -3553,6 +3572,10 @@  find_line_common (struct linetable *l, int lineno,
     {
       struct linetable_entry *item = &(l->item[i]);
 
+      /* Ignore non-statements.  */
+      if (!item->is_stmt)
+	continue;
+
       if (item->line == lineno)
 	{
 	  /* Return the first (lowest address) entry which matches.  */
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 5fa067b5f48..771b5ec5bf7 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1277,7 +1277,13 @@  struct rust_vtable_symbol : public symbol
 
 struct linetable_entry
 {
+  /* The line number for this entry.  */
   int line;
+
+  /* True if this PC is a good location to place a breakpoint for LINE.  */
+  unsigned is_stmt : 1;
+
+  /* The address for this entry.  */
   CORE_ADDR pc;
 };
 
@@ -1853,6 +1859,10 @@  struct symtab_and_line
   bool explicit_pc = false;
   bool explicit_line = false;
 
+  /* If the line number information is valid, then this indicates if this
+     line table entry had the is-stmt flag set or not.  */
+  bool is_stmt = false;
+
   /* The probe associated with this symtab_and_line.  */
   probe *prob = NULL;
   /* If PROBE is not NULL, then this is the objfile in which the probe
diff --git a/gdb/testsuite/gdb.cp/next-inline.cc b/gdb/testsuite/gdb.cp/next-inline.cc
new file mode 100644
index 00000000000..24f5c6916b2
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/next-inline.cc
@@ -0,0 +1,65 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019-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/>.  */
+
+#ifdef USE_NEXT_INLINE_H
+
+#include "next-inline.h"
+
+#else	/* USE_NEXT_INLINE_H */
+
+/* The code below must remain identical to the code in next-inline.h.  */
+
+#include <stdlib.h>
+
+struct tree
+{
+  volatile int x;
+  volatile int z;
+};
+
+#define TREE_TYPE(NODE) (*tree_check (NODE, 0))
+
+inline tree *
+tree_check (tree *t, int i)
+{
+  if (t->x != i)
+    abort();
+  tree *x = t;
+  return x;
+}
+
+#endif	/* USE_NEXT_INLINE_H */
+
+int __attribute__((noinline, noclone))
+get_alias_set (tree *t)
+{
+  if (t != NULL
+      && TREE_TYPE (t).z != 1
+      && TREE_TYPE (t).z != 2
+      && TREE_TYPE (t).z != 3)
+    return 0;
+  return 1;
+}
+
+tree xx;
+
+int
+main()
+{
+  get_alias_set (&xx);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.cp/next-inline.exp b/gdb/testsuite/gdb.cp/next-inline.exp
new file mode 100644
index 00000000000..0b2b22d8894
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/next-inline.exp
@@ -0,0 +1,70 @@ 
+# Copyright 2019-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/>.
+
+standard_testfile .cc
+
+# Compile the test source with USE_NEXT_INLINE_H defined (when
+# use_header is true), or not defined.
+proc do_test { use_header } {
+    global srcfile testfile
+
+    set options {c++ debug nowarnings optimize=-O2}
+    if { $use_header } {
+	lappend options additional_flags=-DUSE_NEXT_INLINE_H
+	set executable "$testfile-with-header"
+    } else {
+	set executable "$testfile-no-header"
+    }
+
+    if { [prepare_for_testing "failed to prepare" $executable \
+	      $srcfile $options] } {
+	return -1
+    }
+
+    if ![runto_main] {
+	fail "can't run to main"
+	return
+    }
+
+    gdb_test "bt" "\\s*\\#0\\s+main.*" "in main"
+    gdb_test "step" ".*" "step into get_alias_set"
+    gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \
+	"not in inline 1"
+    # It's possible that this first failure (when not using a header
+    # file) is GCC's fault, though the remaining failures would best
+    # be fixed by adding location views support (though it could be
+    # that some easier heuristic could be figured out).  Still, it is
+    # not certain that the first failure wouldn't also be fixed by
+    # having location view support, so for now it is tagged as such.
+    if {!$use_header} { setup_kfail "*-*-*" symtab/25507 }
+    gdb_test "next" ".*TREE_TYPE.*" "next step 1"
+    gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \
+	"not in inline 2"
+    gdb_test "next" ".*TREE_TYPE.*" "next step 2"
+    gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \
+	"not in inline 3"
+    if {!$use_header} { setup_kfail "*-*-*" symtab/25507 }
+    gdb_test "next" ".*TREE_TYPE.*" "next step 3"
+    gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \
+	"not in inline 4"
+    if {!$use_header} { setup_kfail "*-*-*" symtab/25507 }
+    gdb_test "next" "return 0.*" "next step 4"
+    gdb_test "bt" \
+	"\\s*\\#0\\s+(main|get_alias_set)\[^\r\]*${srcfile}:.*" \
+	"not in inline 5"
+}
+
+do_test 0
+do_test 1
diff --git a/gdb/testsuite/gdb.cp/next-inline.h b/gdb/testsuite/gdb.cp/next-inline.h
new file mode 100644
index 00000000000..f6a3991ae7d
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/next-inline.h
@@ -0,0 +1,38 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019-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/>.  */
+
+/* The code below must remain identical to the block of code in
+   next-inline.cc.  */
+
+#include <stdlib.h>
+
+struct tree
+{
+  volatile int x;
+  volatile int z;
+};
+
+#define TREE_TYPE(NODE) (*tree_check (NODE, 0))
+
+inline tree *
+tree_check (tree *t, int i)
+{
+  if (t->x != i)
+    abort();
+  tree *x = t;
+  return x;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-is-stmt-2.c b/gdb/testsuite/gdb.dwarf2/dw2-is-stmt-2.c
new file mode 100644
index 00000000000..be3fa458cf0
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-is-stmt-2.c
@@ -0,0 +1,99 @@ 
+/* 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/>.  */
+
+/* This tests GDB's handling of the DWARF is-stmt field in the line table.
+
+   This field is used when many addresses all represent the same source
+   line.  The address(es) at which it is suitable to place a breakpoint for
+   a line are marked with is-stmt true, while address(es) that are not good
+   places to place a breakpoint are marked as is-stmt false.
+
+   In order to build a reproducible test and exercise GDB's is-stmt
+   support, we will be generating our own DWARF.  The test will contain a
+   series of C source lines, ensuring that we get a series of assembler
+   instructions.  Each C source line will be given an assembler label,
+   which we use to generate a fake line table.
+
+   In this fake line table each assembler block is claimed to represent a
+   single C source line, however, we will toggle the is-stmt flag.  We can
+   then debug this with GDB and test the handling of is-stmt.  */
+
+/* Used to insert labels with which we can build a fake line table.  */
+#define LL(N)						\
+  do							\
+    {							\
+      asm ("line_label_" #N ": .globl line_label_" #N); \
+      var = (N);					\
+      bar = ((N) + 1);					\
+    }							\
+  while (0)
+
+volatile int var;
+volatile int bar;
+
+int
+main ()
+{					/* main prologue */
+  asm ("main_label: .globl main_label");
+
+  /* main start */
+
+  /* Line 1.  */
+  /* Line 2.  */
+  /* Line 3.  */
+  /* Line 4.  */
+  /* Line 5.  */
+
+  LL (0);
+  LL (1);
+  LL (2);
+  LL (3);
+  LL (4);
+  LL (5);
+  LL (6);
+  LL (7);
+  LL (8);
+  LL (9);
+  LL (10);
+  LL (11);
+  LL (12);
+  LL (13);
+  LL (14);
+  LL (15);
+  LL (16);
+  return 0;				/* main end */
+}
+
+#if 0
+
+PROLOGUE*
+1: L1
+2: L1*
+3: L2
+4: L1
+L3
+L4
+L4*
+L2*
+L2
+L3
+L5
+L5*
+L3
+L4
+L5
+RETURN
+
+#endif
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-is-stmt-2.exp b/gdb/testsuite/gdb.dwarf2/dw2-is-stmt-2.exp
new file mode 100644
index 00000000000..860b13140c6
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-is-stmt-2.exp
@@ -0,0 +1,265 @@ 
+# 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/>.
+
+# This test shows the importance of not corrupting the order of line
+# table information.  When multiple lines are given for the same
+# address the compiler usually lists these in the order in which we
+# would expect to encounter them.  When stepping through nested inline
+# frames the last line given for an address is assumed by GDB to be
+# the most inner frame, and this is what GDB displays.
+#
+# If we corrupt the order of the line table entries then GDB will
+# display the wrong line as being the inner most frame.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+# The .c files use __attribute__.
+if [get_compiler_info] {
+    return -1
+}
+if !$gcc_compiled {
+    return 0
+}
+
+standard_testfile dw2-is-stmt-2.c dw2-is-stmt-2.S
+
+# Extract the start, length, and end for function called NAME and
+# create suitable variables in the callers scope.
+proc get_func_info { name } {
+    global srcdir subdir srcfile
+
+    upvar 1 "${name}_start" func_start
+    upvar 1 "${name}_len" func_len
+    upvar 1 "${name}_end" func_end
+
+    lassign [function_range ${name} [list ${srcdir}/${subdir}/$srcfile]] \
+	func_start func_len
+    set func_end "$func_start + $func_len"
+}
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    global srcdir subdir srcfile
+    declare_labels lines_label
+
+    get_func_info main
+
+    cu {} {
+	compile_unit {
+	    {language @DW_LANG_C}
+	    {name dw2-is-stmt.c}
+	    {low_pc 0 addr}
+	    {stmt_list ${lines_label} DW_FORM_sec_offset}
+	} {
+	    subprogram {
+		{external 1 flag}
+		{name main}
+		{low_pc $main_start addr}
+		{high_pc "$main_start + $main_len" addr}
+	    } {}
+	}
+    }
+
+    lines {version 2 default_is_stmt 1} lines_label {
+	include_dir "${srcdir}/${subdir}"
+	file_name "$srcfile" 1
+
+	program {
+	    {DW_LNE_set_address main}
+	    {DW_LNS_advance_line \
+		 [expr [gdb_get_line_number "main prologue"] - 1]}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_0}
+	    {DW_LNS_advance_line \
+		 [expr [gdb_get_line_number "main start"] \
+		      - [gdb_get_line_number "main prologue"]]}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_1}
+	    {DW_LNS_advance_line \
+		 [expr [gdb_get_line_number "Line 1"] \
+		      - [gdb_get_line_number "main start"]]}
+	    {DW_LNS_negate_stmt}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_2}
+	    {DW_LNS_negate_stmt}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_3}
+	    {DW_LNS_advance_line \
+		 [expr [gdb_get_line_number "Line 2"] \
+		      - [gdb_get_line_number "Line 1"]]}
+	    {DW_LNS_negate_stmt}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_4}
+	    {DW_LNS_advance_line \
+		 [expr [gdb_get_line_number "Line 1"] \
+		      - [gdb_get_line_number "Line 2"]]}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_5}
+	    {DW_LNS_advance_line \
+		 [expr [gdb_get_line_number "Line 3"] \
+		      - [gdb_get_line_number "Line 1"]]}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_6}
+	    {DW_LNS_advance_line \
+		 [expr [gdb_get_line_number "Line 4"] \
+		      - [gdb_get_line_number "Line 3"]]}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_7}
+	    {DW_LNS_negate_stmt}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_8}
+	    {DW_LNS_advance_line \
+		 [expr [gdb_get_line_number "Line 2"] \
+		      - [gdb_get_line_number "Line 4"]]}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_9}
+	    {DW_LNS_negate_stmt}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_10}
+	    {DW_LNS_advance_line \
+		 [expr [gdb_get_line_number "Line 3"] \
+		      - [gdb_get_line_number "Line 2"]]}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_11}
+	    {DW_LNS_advance_line \
+		 [expr [gdb_get_line_number "Line 5"] \
+		      - [gdb_get_line_number "Line 3"]]}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_12}
+	    {DW_LNS_negate_stmt}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_13}
+	    {DW_LNS_advance_line \
+		 [expr [gdb_get_line_number "Line 3"] \
+		      - [gdb_get_line_number "Line 5"]]}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_14}
+	    {DW_LNS_advance_line \
+		 [expr [gdb_get_line_number "Line 4"] \
+		      - [gdb_get_line_number "Line 3"]]}
+	    {DW_LNS_negate_stmt}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_15}
+	    {DW_LNS_advance_line \
+		 [expr [gdb_get_line_number "Line 5"] \
+		      - [gdb_get_line_number "Line 4"]]}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_16}
+	    {DW_LNS_advance_line \
+ 		 [expr [gdb_get_line_number "main end"] \
+		      - [gdb_get_line_number "Line 5"]]}
+	    {DW_LNS_negate_stmt}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address ${main_end}}
+	    {DW_LNE_end_sequence}
+	}
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+# Check stepping through the out of order lines gives the experience
+# we expect; lines in the correct order, and stop at the correct
+# labels.Q
+set locs { { "Line 1." "line_label_2" } \
+	       { "Line 4." "line_label_7" } \
+	       { "Line 2." "line_label_8" } \
+	       { "Line 5." "line_label_12" } \
+	       { "Line 3." "line_label_13" } }
+foreach entry $locs {
+    set pattern [lindex $entry 0]
+    set label  [lindex $entry 1]
+
+    set linum [gdb_get_line_number "$pattern"]
+    gdb_test "step" "\r\n$linum\[ \t\]+/\\* $pattern  \\*/" \
+	"step to $pattern"
+
+    set pc [get_hexadecimal_valueof "\$pc" "NO-PC" \
+	       "read \$pc at $pattern"]
+    set val [get_hexadecimal_valueof "&${label}" "INVALID"]
+    gdb_assert { $pc == $val } \
+	"check pc at $pattern"
+}
+
+# Now restart the test, and this time, single instruction step
+# through.  This is checking that the is-stmt marked lines are
+# displayed differently (without addresses) to addresses that are
+# mid-way through a line, or not marked as is-stmt.
+clean_restart $binfile
+runto_main
+
+foreach entry $locs {
+    set pattern [lindex $entry 0]
+    set label  [lindex $entry 1]
+
+    with_test_prefix "stepi to $label" {
+	# If can take many instruction steps to get to the next label.
+	set i 0
+	set pc [get_hexadecimal_valueof "\$pc" "NO-PC" ]
+	set val [get_hexadecimal_valueof "&${label}" "INVALID"]
+	while { $pc < $val } {
+	    incr i
+	    set line_changed -1
+	    gdb_test_multiple "stepi" "stepi $i" {
+		-re "\r\n$hex\[ \t\]+$decimal\[^\r\n\]+\r\n$gdb_prompt" {
+		    set line_changed 0
+		}
+		-re "\r\n$decimal\[ \t\]+/\\* $pattern  \\*/\r\n$gdb_prompt" {
+		    set line_changed 1
+		}
+	    }
+	    gdb_assert { $line_changed != -1 } \
+		"ensure we saw a valid pattern, $i"
+	    set pc [get_hexadecimal_valueof "\$pc" "NO-PC" \
+			"get pc inside stepi loop, $i"]
+	    if { $pc == $val } {
+		gdb_assert { $line_changed } \
+		    "line must change at $label"
+	    } else {
+		gdb_assert { !$line_changed } "same line $i"
+	    }
+	}
+    }
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-is-stmt.c b/gdb/testsuite/gdb.dwarf2/dw2-is-stmt.c
new file mode 100644
index 00000000000..7e70995d80f
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-is-stmt.c
@@ -0,0 +1,61 @@ 
+/* 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/>.  */
+
+/* This tests GDB's handling of the DWARF is-stmt field in the line table.
+
+   This field is used when many addresses all represent the same source
+   line.  The address(es) at which it is suitable to place a breakpoint for
+   a line are marked with is-stmt true, while address(es) that are not good
+   places to place a breakpoint are marked as is-stmt false.
+
+   In order to build a reproducible test and exercise GDB's is-stmt
+   support, we will be generating our own DWARF.  The test will contain a
+   series of C source lines, ensuring that we get a series of assembler
+   instructions.  Each C source line will be given an assembler label,
+   which we use to generate a fake line table.
+
+   In this fake line table each assembler block is claimed to represent a
+   single C source line, however, we will toggle the is-stmt flag.  We can
+   then debug this with GDB and test the handling of is-stmt.  */
+
+/* Used to insert labels with which we can build a fake line table.  */
+#define LL(N) asm ("line_label_" #N ": .globl line_label_" #N)
+
+volatile int var;
+volatile int bar;
+
+int
+main ()
+{					/* main prologue */
+  asm ("main_label: .globl main_label");
+  LL (1);
+  var = 99;				/* main, set var to 99 */
+  bar = 99;
+
+  LL (2);
+  var = 0;				/* main, set var to 0 */
+  bar = 0;
+
+  LL (3);
+  var = 1;				/* main, set var to 1 */
+  bar = 1;
+
+  LL (4);
+  var = 2;				/* main, set var to 2 */
+  bar = 2;
+
+  LL (5);
+  return 0;				/* main end */
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-is-stmt.exp b/gdb/testsuite/gdb.dwarf2/dw2-is-stmt.exp
new file mode 100644
index 00000000000..6e5f3e6bf07
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-is-stmt.exp
@@ -0,0 +1,267 @@ 
+# 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/>.
+
+# This test shows the importance of not corrupting the order of line
+# table information.  When multiple lines are given for the same
+# address the compiler usually lists these in the order in which we
+# would expect to encounter them.  When stepping through nested inline
+# frames the last line given for an address is assumed by GDB to be
+# the most inner frame, and this is what GDB displays.
+#
+# If we corrupt the order of the line table entries then GDB will
+# display the wrong line as being the inner most frame.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+# The .c files use __attribute__.
+if [get_compiler_info] {
+    return -1
+}
+if !$gcc_compiled {
+    return 0
+}
+
+standard_testfile dw2-is-stmt.c dw2-is-stmt.S
+
+# Extract the start, length, and end for function called NAME and
+# create suitable variables in the callers scope.
+proc get_func_info { name } {
+    global srcdir subdir srcfile
+
+    upvar 1 "${name}_start" func_start
+    upvar 1 "${name}_len" func_len
+    upvar 1 "${name}_end" func_end
+
+    lassign [function_range ${name} [list ${srcdir}/${subdir}/$srcfile]] \
+	func_start func_len
+    set func_end "$func_start + $func_len"
+}
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    global srcdir subdir srcfile
+    declare_labels lines_label
+
+    get_func_info main
+
+    cu {} {
+	compile_unit {
+	    {language @DW_LANG_C}
+	    {name dw2-is-stmt.c}
+	    {low_pc 0 addr}
+	    {stmt_list ${lines_label} DW_FORM_sec_offset}
+	} {
+	    subprogram {
+		{external 1 flag}
+		{name main}
+		{low_pc $main_start addr}
+		{high_pc "$main_start + $main_len" addr}
+	    } {}
+	}
+    }
+
+    lines {version 2 default_is_stmt 0} lines_label {
+	include_dir "${srcdir}/${subdir}"
+	file_name "$srcfile" 1
+
+	program {
+	    {DW_LNE_set_address main}
+	    {DW_LNS_advance_line \
+		 [expr [gdb_get_line_number "main prologue"] - 1]}
+	    {DW_LNS_negate_stmt}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_1}
+	    {DW_LNS_advance_line \
+		 [expr [gdb_get_line_number "main, set var to 99"] \
+		      - [gdb_get_line_number "main prologue"]]}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_2}
+	    {DW_LNS_advance_line \
+ 		 [expr [gdb_get_line_number "main, set var to 0"] \
+		      - [gdb_get_line_number "main, set var to 99"]]}
+	    {DW_LNS_negate_stmt}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_3}
+	    {DW_LNS_negate_stmt}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_4}
+	    {DW_LNS_negate_stmt}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address line_label_5}
+	    {DW_LNS_advance_line \
+ 		 [expr [gdb_get_line_number "main end"] \
+		      - [gdb_get_line_number "main, set var to 0"]]}
+	    {DW_LNS_negate_stmt}
+	    {DW_LNS_copy}
+
+	    {DW_LNE_set_address ${main_end}}
+	    {DW_LNE_end_sequence}
+	}
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+# First, break by address at a location we know is marked as not a
+# statement.  GDB should still correctly report the file and line
+# number.
+gdb_breakpoint "*line_label_2"
+gdb_continue_to_breakpoint "*line_label_2"
+
+# Now step by instruction.  We should skip over the is-stmt location
+# for this line, and land on the next source line.
+gdb_test "step" "/\\* main end \\*/" \
+    "step to end from line_label_2"
+
+# Restart the test.  This time, stop at a location we know is marked
+# as a statement.
+clean_restart ${binfile}
+runto_main
+
+gdb_breakpoint "*line_label_3"
+gdb_continue_to_breakpoint "*line_label_3"
+
+# Now step by instruction.  As you would expect we should leave this
+# line and stop at the next source line.
+gdb_test "step" "/\\* main end \\*/" \
+    "step to end from line_label_3"
+
+# Restart the test, this time, step through line by line, ensure we
+# only stop at the places where is-stmt is true.
+clean_restart ${binfile}
+runto_main
+
+# Get the values of the labels where we expect to stop.
+set ll1 [get_hexadecimal_valueof "&line_label_1" "INVALID"]
+set ll2 [get_hexadecimal_valueof "&line_label_2" "INVALID"]
+set ll3 [get_hexadecimal_valueof "&line_label_3" "INVALID"]
+set ll5 [get_hexadecimal_valueof "&line_label_5" "INVALID"]
+
+# The first stop should be at line_label_1
+with_test_prefix "check we're at line_label_1" {
+    set pc [get_hexadecimal_valueof "\$pc" "NO-PC"]
+    gdb_assert { $ll1 == $pc } "check initial \$pc is line_label_1"
+}
+
+# Now step, this should take us to line_label_3 which is the next
+# location marked as is-stmt.
+with_test_prefix "step to line_label_3" {
+    gdb_test "step" "/\\* main, set var to 0 \\*/"
+    set pc [get_hexadecimal_valueof "\$pc" "NO-PC"]
+    gdb_assert { $ll3 == $pc } "check initial \$pc is line_label_3"
+}
+
+# A final step should take us to line_label_5.
+with_test_prefix "step to line_label_5" {
+    gdb_test "step" "/\\* main end \\*/"
+    set pc [get_hexadecimal_valueof "\$pc" "NO-PC"]
+    gdb_assert { $ll5 == $pc } "check initial \$pc"
+}
+
+# Now restart the test, and place a breakpoint by line number.  GDB
+# should select the location that is marked as is-stmt.
+clean_restart ${binfile}
+runto_main
+set linum [gdb_get_line_number "main, set var to 0"]
+gdb_breakpoint "$srcfile:$linum"
+gdb_continue_to_breakpoint "Breakpoint on line, set var to 0"
+set pc [get_hexadecimal_valueof "\$pc" "NO-PC"]
+gdb_assert { $ll3 == $pc } "check initial \$pc"
+
+# Restart the test again, this time we will test stepping by
+# instruction.
+clean_restart ${binfile}
+runto_main
+
+# We will be at line_label_1 at this point - we already tested this
+# above.  Now single instruction step forward until we get to
+# line_label_2.  Every instruction before line_label_2 should be
+# attributed to the 'var = 99' line.  For most targets there will only
+# be a single instruction between line_label_1 and line_label_2, but
+# we allow for up to 25 (just a random number).
+
+set $i 0
+set pc [get_hexadecimal_valueof "\$pc" "NO-PC" \
+	   "get pc before stepi loop at line_label_1"]
+while { $pc < $ll2 } {
+    incr i
+    set line_changed -1
+    gdb_test_multiple "stepi" "stepi until line_label_2, $i" {
+	-re "main, set var to 99.*$gdb_prompt" {
+	    set line_changed 0
+	}
+	-re "main, set var to 0.*$gdb_prompt " {
+	    set line_changed 1
+	}
+    }
+    gdb_assert { $line_changed != -1 } \
+	"ensure we saw a valid line pattern, $i"
+    set pc [get_hexadecimal_valueof "\$pc" "NO-PC" \
+		"get pc inside stepi loop from line_label_1, $i"]
+    if { $ll2 == $pc } {
+	gdb_assert { $line_changed } \
+	    "line must change at line_label_2"
+    } else {
+	gdb_assert { !$line_changed } \
+	    "line should not change until line_label_2, $i"
+    }
+}
+
+# Now single instruction step forward until GDB reports a new source
+# line, at which point we should be at line_label_5.
+
+set $i 0
+set pc [get_hexadecimal_valueof "\$pc" "NO-PC" \
+	   "get pc before stepi loop at line_label_2"]
+while { $pc < $ll5 } {
+    incr i
+    set line_changed -1
+    gdb_test_multiple "stepi" "stepi until line_label_5, $i" {
+	-re "main, set var to 0.*$gdb_prompt" {
+	    set line_changed 0
+	}
+	-re "main end.*$gdb_prompt " {
+	    set line_changed 1
+	}
+    }
+    gdb_assert { $line_changed != -1 } \
+	"ensure we saw a valid line pattern, $i"
+    set pc [get_hexadecimal_valueof "\$pc" "NO-PC" \
+		"get pc inside stepi loop from line_label_2, $i"]
+    if { $ll5 == $pc } {
+	gdb_assert { $line_changed } \
+	    "line must change at line_label_5"
+    } else {
+	gdb_assert { !$line_changed } \
+	    "line should not change until line_label_5, $i"
+    }
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
index 33177336f58..7f7301502ab 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
@@ -146,10 +146,10 @@  gdb_test "info line frame3" \
 set end_seq_count 0
 gdb_test_multiple "maint info line-table" \
     "count END markers in line table" {
-	-re "^$decimal\[ \t\]+$decimal\[ \t\]+$hex\r\n" {
+	-re "^$decimal\[ \t\]+$decimal\[ \t\]+$hex\(\[ \t\]+Y\)?\r\n" {
 	    exp_continue
 	}
-	-re "^$decimal\[ \t\]+END\[ \t\]+$hex\r\n" {
+	-re "^$decimal\[ \t\]+END\[ \t\]+$hex\(\[ \t\]+Y\)?\r\n" {
 	    incr end_seq_count
 	    exp_continue
 	}
@@ -159,7 +159,7 @@  gdb_test_multiple "maint info line-table" \
 	-re ".*linetable: \\(\\(struct linetable \\*\\) 0x0\\):\r\nNo line table.\r\n" {
 	    exp_continue
 	}
-	-re ".*linetable: \\(\\(struct linetable \\*\\) $hex\\):\r\nINDEX\[ \t\]+LINE\[ \t\]+ADDRESS\r\n" {
+	-re ".*linetable: \\(\\(struct linetable \\*\\) $hex\\):\r\nINDEX\[ \t\]+LINE\[ \t\]+ADDRESS\[ \t\]+IS-STMT\r\n" {
 	    exp_continue
 	}
     }
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index b7da3f944c7..735f8b08825 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -432,6 +432,9 @@  arrange_linetable (struct linetable *oldLineTb)
 
   for (function_count = 0, ii = 0; ii < oldLineTb->nitems; ++ii)
     {
+      if (oldLineTb->item[ii].is_stmt == 0)
+	continue;
+
       if (oldLineTb->item[ii].line == 0)
 	{			/* Function entry found.  */
 	  if (function_count >= fentry_size)
@@ -442,6 +445,7 @@  arrange_linetable (struct linetable *oldLineTb)
 			  fentry_size * sizeof (struct linetable_entry));
 	    }
 	  fentry[function_count].line = ii;
+	  fentry[function_count].is_stmt = 1;
 	  fentry[function_count].pc = oldLineTb->item[ii].pc;
 	  ++function_count;