[PATCH 0/2] Line table is_stmt support

classic Classic list List threaded Threaded
77 messages Options
1234
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/2] Line table is_stmt support

Andrew Burgess
This series adds support for the DWARF line table is_stmt property.
Patch #2 includes a fuller description of the background to this
series (patch #1 is a minor setup patch), but this was previously
discussed on the list as part of a different patch series, but I
wanted to start a new thread so this got some visibility.

All feedback welcome.

thanks,
Andrew

--

Andrew Burgess (2):
  gdb/testsuite: Add is-stmt support to the DWARF compiler
  gdb: Add support for tracking the DWARF line table is-stmt field

 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                      |  16 ++
 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/testsuite/lib/dwarf.exp                  |   8 +-
 gdb/xcoffread.c                              |   4 +
 24 files changed, 1048 insertions(+), 30 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

--
2.14.5

Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] gdb/testsuite: Add is-stmt support to the DWARF compiler

Andrew Burgess
This commit adds the ability to set and toggle the DWARF line table
is-stmt flag.

A DWARF line table can now be created with the attribute
'default_is_stmt' like this:

  lines {version 2 default_is_stmt 0} label {
    ...
  }

If 'default_is_stmt' is not specified then the current default is 1,
which matches the existing behaviour.

Inside the DWARF line table program you can now make use of
{DW_LNS_negate_stmt} to toggle the is-stmt flag, for example this
meaningless program:

  lines {version 2 default_is_stmt 0} label {
    include_dir "some_directory"
    file_name "some_filename" 1

    program {
      {DW_LNS_negate_stmt}
      {DW_LNE_end_sequence}
    }
  }

This new functionality will be used in a later commit.

gdb/testsuite/ChangeLog:

        * lib/dwarf.exp (Dwarf::lines) Add support for modifying the
        is-stmt flag in the line table.

Change-Id: Ia3f61d523826382dd2333f65b9aae368ad29c4a5
---
 gdb/testsuite/ChangeLog     | 5 +++++
 gdb/testsuite/lib/dwarf.exp | 8 +++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index 6c6ffbe7c2f..417b22d2345 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -1311,12 +1311,14 @@ namespace eval Dwarf {
  set _unit_addr_size default
  set _line_saw_program 0
  set _line_saw_file 0
+ set _default_is_stmt 1
 
  foreach { name value } $options {
     switch -exact -- $name {
  is_64 { set is_64 $value }
  version { set _unit_version $value }
  addr_size { set _unit_addr_size $value }
+ default_is_stmt { set _default_is_stmt $value }
  default { error "unknown option $name" }
     }
  }
@@ -1363,7 +1365,7 @@ namespace eval Dwarf {
  define_label $header_len_label
 
  _op .byte 1 "minimum_instruction_length"
- _op .byte 1 "default_is_stmt"
+ _op .byte $_default_is_stmt "default_is_stmt"
  _op .byte 1 "line_base"
  _op .byte 1 "line_range"
  _op .byte 10 "opcode_base"
@@ -1438,6 +1440,10 @@ namespace eval Dwarf {
  _op .byte 1
     }
 
+    proc DW_LNS_negate_stmt {} {
+ _op .byte 6
+    }
+
     proc DW_LNS_advance_pc {offset} {
  _op .byte 2
  _op .uleb128 ${offset}
--
2.14.5

Reply | Threaded
Open this post in threaded view
|

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

Andrew Burgess
In reply to this post by Andrew Burgess
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

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;
 
--
2.14.5

Reply | Threaded
Open this post in threaded view
|

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

Bernd Edlinger-2
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.
Reply | Threaded
Open this post in threaded view
|

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

Luis Machado-2
In reply to this post by Andrew Burgess
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.
Reply | Threaded
Open this post in threaded view
|

[PATCH] Fix range end handling of inlined subroutines

Bernd Edlinger-2
In reply to this post by Andrew Burgess
Hi,

this is based on Andrew's patch here:

https://sourceware.org/ml/gdb-patches/2020-02/msg00092.html

This and adds a heuristic to fix the case where caller
and callee reside in the same subfile, it uses
the recorded is-stmt bits and locates end of
range infos, including the previously ignored empty
range, and adjusting is-stmt info at this
same pc, when the last item is not-is-stmt, the
previous line table entries are dubious and
we reset the is-stmt bit there.
This fixes the new test case in Andrew's patch.

It understood, that this is just a heuristic
approach, since we do not have exactly the data,
which is needed to determine at which of the identical
PCs in the line table the subroutine actually ends.
So, this tries to be as conservative as possible,
just changing what is absolutely necessary.

This patch itself is preliminary, since the is-stmt patch
needs to be rebased after the refactoring of
dwarf2read.c from yesterday, so I will have to rebase
this patch as well, but decided to wait for Andrew.


Thanks
Bernd.

0001-Fix-range-end-handling-of-inlined-subroutines.patch (9K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Bernd Edlinger-2
In reply to this post by Bernd Edlinger-2
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.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix range end handling of inlined subroutines

Andrew Burgess
In reply to this post by Bernd Edlinger-2
* Bernd Edlinger <[hidden email]> [2020-02-09 21:07:32 +0000]:

> This patch itself is preliminary, since the is-stmt patch
> needs to be rebased after the refactoring of
> dwarf2read.c from yesterday, so I will have to rebase
> this patch as well, but decided to wait for Andrew.

I've been busy with other projects for the last few days, but plan to
look at this series tomorrow.

Thanks,
Andrew


>
>
> Thanks
> Bernd.

> From d15f3346feb1ed5cbbc14e708f3f6b58d88bc0fe Mon Sep 17 00:00:00 2001
> From: Bernd Edlinger <[hidden email]>
> Date: Sun, 9 Feb 2020 21:13:17 +0100
> Subject: [PATCH] Fix range end handling of inlined subroutines
>
> Since the is_stmt is now handled, it became
> possible to locate dubious is_stmt line entries
> at the end of an inlined function, in the same
> subfile.
>
> If any is-stmt line is followed by a non-is-stmt,
> at the same PC as the inlined subroutine ends,
> clear the is-stmt there (except the line = 0
> end marker, of course).
>
> This is about the best we can do at the moment,
> unless location view information are added to the
> block ranges debug info structure, and location
> views are implemented in gdb in general.
>
> gdb:
> 2020-02-09  Bernd Edlinger  <[hidden email]>
>
> * buildsym.c (buildsym_compunit::record_inline_range_end):
> New function.
> * buildsym.h (buildsym_compunit::record_inline_range_end): Declare.
> * dwarf2read.c (dwarf2_rnglists_process,
> dwarf2_ranges_process): Don't ignore empty ranges here.
> (dwarf2_ranges_read): Ignore empty ranges here.
> (dwarf2_record_block_ranges): Pass end of range PC to
> record_inline_range_end for inline functions.
>
> gdb/testsuite:
> 2020-02-09  Bernd Edlinger  <[hidden email]>
>
> * gdb.cp/next-inline.exp: Adjust test.
> ---
>  gdb/buildsym.c                       | 23 +++++++++++++++++++++++
>  gdb/buildsym.h                       |  2 ++
>  gdb/dwarf2read.c                     | 22 +++++++++++++---------
>  gdb/testsuite/gdb.cp/next-inline.exp |  9 ---------
>  4 files changed, 38 insertions(+), 18 deletions(-)
>
> diff --git a/gdb/buildsym.c b/gdb/buildsym.c
> index 7fd256f..381a7c8 100644
> --- a/gdb/buildsym.c
> +++ b/gdb/buildsym.c
> @@ -732,6 +732,29 @@ buildsym_compunit::record_line (struct subfile *subfile, int line,
>  }
>  
>  
> +/* Record a PC where a inlined subroutine ends.  */
> +
> +void
> +buildsym_compunit::record_inline_range_end (CORE_ADDR end)
> +{
> +  struct subfile *subfile;
> +
> +  for (subfile = m_subfiles; subfile != NULL; subfile = subfile->next)
> +    {
> +      if (subfile->line_vector != NULL)
> + {
> +  struct linetable_entry *items = subfile->line_vector->item;
> +  int i;
> +
> +  for (i = subfile->line_vector->nitems - 1; i > 0; i--)
> +    if (items[i].pc == end && items[i - 1].pc == end
> + && !items[i].is_stmt && items[i - 1].line != 0)
> +      items[i - 1].is_stmt = 0;
> + }
> +    }
> +}
> +
> +
>  /* Subroutine of end_symtab to simplify it.  Look for a subfile that
>     matches the main source file's basename.  If there is only one, and
>     if the main source file doesn't have any symbol or line number
> diff --git a/gdb/buildsym.h b/gdb/buildsym.h
> index c768a4c..3cf0f8b 100644
> --- a/gdb/buildsym.h
> +++ b/gdb/buildsym.h
> @@ -190,6 +190,8 @@ struct buildsym_compunit
>    void record_line (struct subfile *subfile, int line, CORE_ADDR pc,
>      bool is_stmt);
>  
> +  void record_inline_range_end (CORE_ADDR end);
> +
>    struct compunit_symtab *get_compunit_symtab ()
>    {
>      return m_compunit_symtab;
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index cbee4ab..6919bf1 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -14407,10 +14407,6 @@ dwarf2_rnglists_process (unsigned offset, struct dwarf2_cu *cu,
>    return false;
>   }
>  
> -      /* Empty range entries have no effect.  */
> -      if (range_beginning == range_end)
> - continue;
> -
>        range_beginning += base;
>        range_end += base;
>  
> @@ -14521,10 +14517,6 @@ dwarf2_ranges_process (unsigned offset, struct dwarf2_cu *cu,
>    return 0;
>   }
>  
> -      /* Empty range entries have no effect.  */
> -      if (range_beginning == range_end)
> - continue;
> -
>        range_beginning += base;
>        range_end += base;
>  
> @@ -14564,6 +14556,10 @@ dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return,
>    retval = dwarf2_ranges_process (offset, cu,
>      [&] (CORE_ADDR range_beginning, CORE_ADDR range_end)
>      {
> +      /* Empty range entries have no effect.  */
> +      if (range_beginning == range_end)
> + return;
> +
>        if (ranges_pst != NULL)
>   {
>    CORE_ADDR lowpc;
> @@ -14801,6 +14797,7 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
>    struct gdbarch *gdbarch = get_objfile_arch (objfile);
>    struct attribute *attr;
>    struct attribute *attr_high;
> +  bool inlined_subroutine = (die->tag == DW_TAG_inlined_subroutine);
>  
>    attr_high = dwarf2_attr (die, DW_AT_high_pc, cu);
>    if (attr_high)
> @@ -14816,7 +14813,10 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
>  
>    low = gdbarch_adjust_dwarf2_addr (gdbarch, low + baseaddr);
>    high = gdbarch_adjust_dwarf2_addr (gdbarch, high + baseaddr);
> -  cu->get_builder ()->record_block_range (block, low, high - 1);
> +  if (inlined_subroutine)
> +    cu->get_builder ()->record_inline_range_end (high);
> +  if (low < high)
> +    cu->get_builder ()->record_block_range (block, low, high - 1);
>          }
>      }
>  
> @@ -14841,6 +14841,10 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
>    end += baseaddr;
>    start = gdbarch_adjust_dwarf2_addr (gdbarch, start);
>    end = gdbarch_adjust_dwarf2_addr (gdbarch, end);
> +  if (inlined_subroutine)
> +    cu->get_builder ()->record_inline_range_end (end);
> +  if (start == end)
> +    return;
>    cu->get_builder ()->record_block_range (block, start, end - 1);
>    blockvec.emplace_back (start, end);
>   });
> diff --git a/gdb/testsuite/gdb.cp/next-inline.exp b/gdb/testsuite/gdb.cp/next-inline.exp
> index 0b2b22d..11d9e2e 100644
> --- a/gdb/testsuite/gdb.cp/next-inline.exp
> +++ b/gdb/testsuite/gdb.cp/next-inline.exp
> @@ -42,24 +42,15 @@ proc do_test { use_header } {
>      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}:.*" \
> --
> 1.9.1
>

Reply | Threaded
Open this post in threaded view
|

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

Andrew Burgess
In reply to this post by Bernd Edlinger-2
* Bernd Edlinger <[hidden email]> [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
Reply | Threaded
Open this post in threaded view
|

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

Andrew Burgess
In reply to this post by Luis Machado-2
* Luis Machado <[hidden email]> [2020-02-06 06:00:29 -0300]:

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

No, a continuous block will be skipped as it is at the moment, even if
is_stmt is true for all entries.

>
> I'm wondering if this would help this bug
> (https://sourceware.org/bugzilla/show_bug.cgi?id=21221) in any way.

I took a look at this bug and even had a few ideas on how we might
improve GDB.  Below is one patch that I put together, though this only
fixes one of the example cases from that bug.  The problem this patch
runs into is that it regresses a few GDB tests
(gdb.base/gdb-sigterm.exp and
gdb.thread/step-bg-decr-pc-switch-thread.exp) in at least one of these
tests GDB makes the following assumption:  Single stepping on a one
line loop (e.g. 'for (;;);') will block forever.  The question then
is:  Does single step move us to the next source line to be executed
that is not the current line, or does single step move us to the next
source line to be executed, even if it is the same as the current
line?

I'm not sure what the answer is to this question...

Thanks,
Andrew

---

commit 35a72ddacf86c7e7f899c869558d1af3cfadb549
Author: Andrew Burgess <[hidden email]>
Date:   Tue Feb 11 15:08:10 2020 +0000

    WIP: Try to better handle small loops
   
    This handles this case:
   
      int main(void)
      {
        int var = 0;
   
        for (;;)
          {
            var++;
          }
   
        return 0;
      }
   
    Compiled as 'gcc -g -o test.x test.c'.
   
    Change-Id: I6f499181eca5fb51b6edb050cbce0bda15665d38

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 3919de81f90..03878025959 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -6674,11 +6674,14 @@ process_event_stop_test (struct execution_control_state *ecs)
 
       /* When stepping backward, stop at beginning of line range
  (unless it's the function entry point, in which case
- keep going back to the call point).  */
+ keep going back to the call point).  When stepping forward we
+ stop at the beginning of the range as this indicates we must be
+ looping.  */
       CORE_ADDR stop_pc = ecs->event_thread->suspend.stop_pc;
       if (stop_pc == ecs->event_thread->control.step_range_start
-  && stop_pc != ecs->stop_func_start
-  && execution_direction == EXEC_REVERSE)
+  && (execution_direction != EXEC_REVERSE
+      || (stop_pc != ecs->stop_func_start
+  && execution_direction == EXEC_REVERSE)))
  end_stepping_range (ecs);
       else
  keep_going (ecs);
@@ -7173,6 +7176,16 @@ process_event_stop_test (struct execution_control_state *ecs)
  }
     }
 
+  if (ecs->event_thread->suspend.stop_pc
+      == ecs->event_thread->control.step_range_start)
+    {
+      /* Very small loops might only contain a single line information
+ entry.  If this is the case then spot when we return to the start
+ of the line range and stop again.  */
+      end_stepping_range (ecs);
+      return;
+    }
+
   /* We aren't done stepping.
 
      Optimize by setting the stepping range to the line.
Reply | Threaded
Open this post in threaded view
|

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

Bernd Edlinger-2
In reply to this post by Andrew Burgess
On 2/11/20 2:57 PM, Andrew Burgess wrote:

> * Bernd Edlinger <[hidden email]> [2020-02-05 17:55:37 +0000]:
>
>> On 2/5/20 12:37 PM, Andrew Burgess wrote:
>>
>> 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.
>

I agree with that, thanks for this explanation.

However, I think I found a small problem in this patch.
You can see it with the next-inline test case.
When you use just step the execution does not stop
between the inline functions, because not calling

current behaviour is this:

Breakpoint 1, main () at next-inline.cc:63
63  get_alias_set (&xx);
(gdb) s
get_alias_set (t=0x404040 <xx>) at next-inline.cc:50
50  if (t != NULL
(gdb) s
51      && TREE_TYPE (t).z != 1
(gdb) s
0x0000000000401169 in tree_check (i=0, t=0x404040 <xx>) at next-inline.h:34
34  if (t->x != i)
(gdb) s
0x000000000040117d in tree_check (i=0, t=0x404040 <xx>) at next-inline.h:34
34  if (t->x != i)
(gdb) s
0x000000000040118f in tree_check (i=0, t=0x404040 <xx>) at next-inline.h:34
34  if (t->x != i)
(gdb) s
main () at next-inline.cc:64
64  return 0;
(gdb)

I debugged a bit because I was not sure why that happens, and it looks
like set_step_info does also set the current inline frame, but you
only want to suppress the line number not the currently executing
inline function.
With this small patch the step works as expected.

commit 193d81765d88b11e68111a8856a84cdf084d0b22
Author: Bernd Edlinger <[hidden email]>
Date:   Fri Feb 14 20:41:51 2020 +0100

    Fix next-inline test case with step
   
    Should stop between inline function invocations.

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 3919de8..89e6654 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7190,6 +7190,8 @@ process_event_stop_test (struct execution_control_state *e
   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;
+  ecs->event_thread->control.step_frame_id = get_frame_id (frame);
+  ecs->event_thread->control.step_stack_frame_id = get_stack_frame_id (frame);
   if (refresh_step_info)
     set_step_info (frame, stop_pc_sal);
 

I'm not sure if that is the cleanest solution, but it works.
With that adjustment, the stepping out of the inline and back into
makes the step call stop, which is the correct behavior:

Breakpoint 1, main () at next-inline.cc:63
63  get_alias_set (&xx);
(gdb) s
get_alias_set (t=0x404040 <xx>) at next-inline.cc:50
50  if (t != NULL
(gdb) s
51      && TREE_TYPE (t).z != 1
(gdb) s
0x0000000000401169 in tree_check (i=0, t=0x404040 <xx>) at next-inline.h:34
34  if (t->x != i)
(gdb) s
52      && TREE_TYPE (t).z != 2
(gdb) s
tree_check (i=0, t=0x404040 <xx>) at next-inline.h:34
34  if (t->x != i)
(gdb) s
53      && TREE_TYPE (t).z != 3)
(gdb) s
tree_check (i=0, t=0x404040 <xx>) at next-inline.h:34
34  if (t->x != i)
(gdb) s
main () at next-inline.cc:64
64  return 0;
(gdb)

If you like you can integrate that into your patch.
You will probably want to add that to the test case.

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

Thanks, regarding my other patch...

I used gprof to get performance numbers, because I was not sure if that
simple approach adds too much execution time, and it turns out that it
spent 30% of the complete execution time in the record_inline_range_end
function, when I was debugging GCC's cc1 executable.
So I decided that this needs to be optimized, I will send a new
version that does the line table patching with a binary search,
after the weekend.

Thanks
Bernd.


> I'll probably merge this over the weekend if there's no other
> feedback.
>
> Thanks,
> Andrew
>
Reply | Threaded
Open this post in threaded view
|

[PATCHv2] Fix range end handling of inlined subroutines

Bernd Edlinger-2
In reply to this post by Bernd Edlinger-2
On 2/9/20 10:07 PM, Bernd Edlinger wrote:

> Hi,
>
> this is based on Andrew's patch here:
>
> https://sourceware.org/ml/gdb-patches/2020-02/msg00092.html
>
> This and adds a heuristic to fix the case where caller
> and callee reside in the same subfile, it uses
> the recorded is-stmt bits and locates end of
> range infos, including the previously ignored empty
> range, and adjusting is-stmt info at this
> same pc, when the last item is not-is-stmt, the
> previous line table entries are dubious and
> we reset the is-stmt bit there.
> This fixes the new test case in Andrew's patch.
>
> It understood, that this is just a heuristic
> approach, since we do not have exactly the data,
> which is needed to determine at which of the identical
> PCs in the line table the subroutine actually ends.
> So, this tries to be as conservative as possible,
> just changing what is absolutely necessary.
>
> This patch itself is preliminary, since the is-stmt patch
> needs to be rebased after the refactoring of
> dwarf2read.c from yesterday, so I will have to rebase
> this patch as well, but decided to wait for Andrew.
>
>
So, this is an update to my previous patch above:
https://sourceware.org/ml/gdb-patches/2020-02/msg00262.html

It improves performance on big data, by using binary
search to locate the bogus line table entries.
Otherwise it behaves identical to the previous version,
and is still waiting for Andrew's patch before it can
be applied.


Thanks
Bernd.

0001-Fix-range-end-handling-of-inlined-subroutines.patch (14K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Bernd Edlinger-2
In reply to this post by Bernd Edlinger-2
On 2/14/20 9:05 PM, Bernd Edlinger wrote:

> On 2/11/20 2:57 PM, Andrew Burgess wrote:
>> * Bernd Edlinger <[hidden email]> [2020-02-05 17:55:37 +0000]:
>>
>>> On 2/5/20 12:37 PM, Andrew Burgess wrote:
>>>
>>> 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.
>>
>
> I agree with that, thanks for this explanation.
>
> However, I think I found a small problem in this patch.
> You can see it with the next-inline test case.
> When you use just step the execution does not stop
> between the inline functions, because not calling
>
> current behaviour is this:
>
> Breakpoint 1, main () at next-inline.cc:63
> 63  get_alias_set (&xx);
> (gdb) s
> get_alias_set (t=0x404040 <xx>) at next-inline.cc:50
> 50  if (t != NULL
> (gdb) s
> 51      && TREE_TYPE (t).z != 1
> (gdb) s
> 0x0000000000401169 in tree_check (i=0, t=0x404040 <xx>) at next-inline.h:34
> 34  if (t->x != i)
> (gdb) s
> 0x000000000040117d in tree_check (i=0, t=0x404040 <xx>) at next-inline.h:34
> 34  if (t->x != i)
> (gdb) s
> 0x000000000040118f in tree_check (i=0, t=0x404040 <xx>) at next-inline.h:34
> 34  if (t->x != i)
> (gdb) s
> main () at next-inline.cc:64
> 64  return 0;
> (gdb)
>
> I debugged a bit because I was not sure why that happens, and it looks
> like set_step_info does also set the current inline frame, but you
> only want to suppress the line number not the currently executing
> inline function.
> With this small patch the step works as expected.
>
Hmm, I think this got stuck, so I just worked a follow-up patch out for you.

I also noticed that the test case cannot work with gcc before gcc-8,
since the is_stmt feature is not implemented earlier, at least with
gcc-4.8 and gcc-5.4 the test case fails.

I thought it would be good to detect that by adding the -gstatement-frontiers
option, so that the gcc driver complains when it is not able to generate
sufficient debug info for that test.

Thoughts?


Bernd.

0001-Fix-next-inline-test-case-with-step.patch (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Andrew Burgess
* Bernd Edlinger <[hidden email]> [2020-03-05 18:01:45 +0000]:

> On 2/14/20 9:05 PM, Bernd Edlinger wrote:
> > On 2/11/20 2:57 PM, Andrew Burgess wrote:
> >> * Bernd Edlinger <[hidden email]> [2020-02-05 17:55:37 +0000]:
> >>
> >>> On 2/5/20 12:37 PM, Andrew Burgess wrote:
> >>>
> >>> 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.
> >>
> >
> > I agree with that, thanks for this explanation.
> >
> > However, I think I found a small problem in this patch.
> > You can see it with the next-inline test case.
> > When you use just step the execution does not stop
> > between the inline functions, because not calling
> >
> > current behaviour is this:
> >
> > Breakpoint 1, main () at next-inline.cc:63
> > 63  get_alias_set (&xx);
> > (gdb) s
> > get_alias_set (t=0x404040 <xx>) at next-inline.cc:50
> > 50  if (t != NULL
> > (gdb) s
> > 51      && TREE_TYPE (t).z != 1
> > (gdb) s
> > 0x0000000000401169 in tree_check (i=0, t=0x404040 <xx>) at next-inline.h:34
> > 34  if (t->x != i)
> > (gdb) s
> > 0x000000000040117d in tree_check (i=0, t=0x404040 <xx>) at next-inline.h:34
> > 34  if (t->x != i)
> > (gdb) s
> > 0x000000000040118f in tree_check (i=0, t=0x404040 <xx>) at next-inline.h:34
> > 34  if (t->x != i)
> > (gdb) s
> > main () at next-inline.cc:64
> > 64  return 0;
> > (gdb)
> >
> > I debugged a bit because I was not sure why that happens, and it looks
> > like set_step_info does also set the current inline frame, but you
> > only want to suppress the line number not the currently executing
> > inline function.
> > With this small patch the step works as expected.
> >
>
> Hmm, I think this got stuck, so I just worked a follow-up patch out
> for you.

Thank for this.  Just wanted to let you know that I saw this mail and
am currently integrating your fix into the original patch.  I have a
second fix I need to merge in and write a test for, but I hope to have
something together soon for you to take a look at.

Thanks,
Andrew



>
> I also noticed that the test case cannot work with gcc before gcc-8,
> since the is_stmt feature is not implemented earlier, at least with
> gcc-4.8 and gcc-5.4 the test case fails.
>
> I thought it would be good to detect that by adding the -gstatement-frontiers
> option, so that the gcc driver complains when it is not able to generate
> sufficient debug info for that test.
>
> Thoughts?
>
>
> Bernd.

> From 3f39d068e3386ba204a598fa3d2946d357b1d7b2 Mon Sep 17 00:00:00 2001
> From: Bernd Edlinger <[hidden email]>
> Date: Fri, 14 Feb 2020 20:41:51 +0100
> Subject: [PATCH] Fix next-inline test case with step
>
> Should stop between inline function invocations.
>
> Add -gstatement-frontiers compile option to avoid
> running test with gcc version that don't support
> this feature, which would fail the test unnecessarily.
> Instead make the compile step fail which is less noisy.
>
> Add a prefix use_header / no_header to all test cases.
>
> gdb:
> 2020-03-05  Bernd Edlinger  <[hidden email]>
>
> * infrun.c (process_event_stop_test): Set step_frame_id.
>
> gdb/testsuite:
> 2020-03-05  Bernd Edlinger  <[hidden email]>
>
> * gdb.cp/next-inline.exp: Extend test case.
> ---
>  gdb/infrun.c                         |  2 ++
>  gdb/testsuite/gdb.cp/next-inline.exp | 45 +++++++++++++++++++++++++++++++++++-
>  2 files changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 6c35805..e8221ba 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -7219,6 +7219,8 @@ process_event_stop_test (struct execution_control_state *ecs)
>    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;
> +  ecs->event_thread->control.step_frame_id = get_frame_id (frame);
> +  ecs->event_thread->control.step_stack_frame_id = get_stack_frame_id (frame);
>    if (refresh_step_info)
>      set_step_info (frame, stop_pc_sal);
>  
> diff --git a/gdb/testsuite/gdb.cp/next-inline.exp b/gdb/testsuite/gdb.cp/next-inline.exp
> index 0b2b22d..ec04694 100644
> --- a/gdb/testsuite/gdb.cp/next-inline.exp
> +++ b/gdb/testsuite/gdb.cp/next-inline.exp
> @@ -20,12 +20,16 @@ standard_testfile .cc
>  proc do_test { use_header } {
>      global srcfile testfile
>  
> -    set options {c++ debug nowarnings optimize=-O2}
> +    set options {c++ debug nowarnings optimize=-O2\ -gstatement-frontiers}
>      if { $use_header } {
>   lappend options additional_flags=-DUSE_NEXT_INLINE_H
>   set executable "$testfile-with-header"
> + set hdrfile "next-inline.h"
> + set prefix "use_header"
>      } else {
>   set executable "$testfile-no-header"
> + set hdrfile "$srcfile"
> + set prefix "no_header"
>      }
>  
>      if { [prepare_for_testing "failed to prepare" $executable \
> @@ -33,6 +37,8 @@ proc do_test { use_header } {
>   return -1
>      }
>  
> +    with_test_prefix $prefix {
> +
>      if ![runto_main] {
>   fail "can't run to main"
>   return
> @@ -64,6 +70,43 @@ proc do_test { use_header } {
>      gdb_test "bt" \
>   "\\s*\\#0\\s+(main|get_alias_set)\[^\r\]*${srcfile}:.*" \
>   "not in inline 5"
> +
> +    if {!$use_header} {
> +        return #until that is fixed
> +    }
> +
> +    if ![runto_main] {
> + fail "can't run to main pass 2"
> + return
> +    }
> +
> +    gdb_test "bt" "\\s*\\#0\\s+main.*" "in main pass 2"
> +    gdb_test "step" ".*" "step into get_alias_set pass 2"
> +    gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \
> + "in get_alias_set pass 2"
> +    gdb_test "step" ".*TREE_TYPE.*" "step 1"
> +    gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \
> + "not in inline 1 pass 2"
> +    gdb_test "step" ".*if \\(t->x != i\\).*" "step 2"
> +    gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \
> + "in inline 1 pass 2"
> +    gdb_test "step" ".*TREE_TYPE.*" "step 3"
> +    gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \
> + "not in inline 2 pass 2"
> +    gdb_test "step" ".*if \\(t->x != i\\).*" "step 4"
> +    gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \
> + "in inline 2 pass 2"
> +    gdb_test "step" ".*TREE_TYPE.*" "step 5"
> +    gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \
> + "not in inline 3 pass 2"
> +    gdb_test "step" ".*if \\(t->x != i\\).*" "step 6"
> +    gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \
> + "in inline 3 pass 2"
> +    gdb_test "step" "return 0.*" "step 7"
> +    gdb_test "bt" \
> + "\\s*\\#0\\s+(main|get_alias_set)\[^\r\]*${srcfile}:.*" \
> + "not in inline 4 pass 2"
> +    }
>  }
>  
>  do_test 0
> --
> 1.9.1
>

Reply | Threaded
Open this post in threaded view
|

[PATCHv2 0/2] Line table is_stmt support

Andrew Burgess
In reply to this post by Bernd Edlinger-2
Patch #1 is unchanged.

Patch #2 includes additional changes in infrun.c based on Bernd's
suggested fix, as well as his additional tests.

Bernd,

If you are happy with this version of the patch that I'll merge this
in the next few days.

Thanks,
Andrew

--

Andrew Burgess (2):
  gdb/testsuite: Add is-stmt support to the DWARF compiler
  gdb: Add support for tracking the DWARF line table is-stmt field

 gdb/ChangeLog                                 |  38 ++++
 gdb/buildsym-legacy.c                         |   4 +-
 gdb/buildsym.c                                |  14 +-
 gdb/buildsym.h                                |   3 +-
 gdb/disasm.c                                  |   6 +
 gdb/dwarf2/read.c                             |  13 +-
 gdb/infrun.c                                  |  51 +++--
 gdb/jit.c                                     |   1 +
 gdb/record-btrace.c                           |  11 +-
 gdb/stack.c                                   |   2 +-
 gdb/symmisc.c                                 |  10 +-
 gdb/symtab.c                                  |  25 ++-
 gdb/symtab.h                                  |  10 +
 gdb/testsuite/ChangeLog                       |  16 ++
 gdb/testsuite/gdb.cp/step-and-next-inline.cc  |  66 +++++++
 gdb/testsuite/gdb.cp/step-and-next-inline.exp | 119 ++++++++++++
 gdb/testsuite/gdb.cp/step-and-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/testsuite/lib/dwarf.exp                   |   8 +-
 gdb/xcoffread.c                               |   4 +
 24 files changed, 1107 insertions(+), 30 deletions(-)
 create mode 100644 gdb/testsuite/gdb.cp/step-and-next-inline.cc
 create mode 100644 gdb/testsuite/gdb.cp/step-and-next-inline.exp
 create mode 100644 gdb/testsuite/gdb.cp/step-and-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

--
2.14.5

Reply | Threaded
Open this post in threaded view
|

[PATCHv2 1/2] gdb/testsuite: Add is-stmt support to the DWARF compiler

Andrew Burgess
This commit adds the ability to set and toggle the DWARF line table
is-stmt flag.

A DWARF line table can now be created with the attribute
'default_is_stmt' like this:

  lines {version 2 default_is_stmt 0} label {
    ...
  }

If 'default_is_stmt' is not specified then the current default is 1,
which matches the existing behaviour.

Inside the DWARF line table program you can now make use of
{DW_LNS_negate_stmt} to toggle the is-stmt flag, for example this
meaningless program:

  lines {version 2 default_is_stmt 0} label {
    include_dir "some_directory"
    file_name "some_filename" 1

    program {
      {DW_LNS_negate_stmt}
      {DW_LNE_end_sequence}
    }
  }

This new functionality will be used in a later commit.

gdb/testsuite/ChangeLog:

        * lib/dwarf.exp (Dwarf::lines) Add support for modifying the
        is-stmt flag in the line table.

Change-Id: Ia3f61d523826382dd2333f65b9aae368ad29c4a5
---
 gdb/testsuite/ChangeLog     | 5 +++++
 gdb/testsuite/lib/dwarf.exp | 8 +++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index 6c6ffbe7c2f..417b22d2345 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -1311,12 +1311,14 @@ namespace eval Dwarf {
  set _unit_addr_size default
  set _line_saw_program 0
  set _line_saw_file 0
+ set _default_is_stmt 1
 
  foreach { name value } $options {
     switch -exact -- $name {
  is_64 { set is_64 $value }
  version { set _unit_version $value }
  addr_size { set _unit_addr_size $value }
+ default_is_stmt { set _default_is_stmt $value }
  default { error "unknown option $name" }
     }
  }
@@ -1363,7 +1365,7 @@ namespace eval Dwarf {
  define_label $header_len_label
 
  _op .byte 1 "minimum_instruction_length"
- _op .byte 1 "default_is_stmt"
+ _op .byte $_default_is_stmt "default_is_stmt"
  _op .byte 1 "line_base"
  _op .byte 1 "line_range"
  _op .byte 10 "opcode_base"
@@ -1438,6 +1440,10 @@ namespace eval Dwarf {
  _op .byte 1
     }
 
+    proc DW_LNS_negate_stmt {} {
+ _op .byte 6
+    }
+
     proc DW_LNS_advance_pc {offset} {
  _op .byte 2
  _op .uleb128 ${offset}
--
2.14.5

Reply | Threaded
Open this post in threaded view
|

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

Andrew Burgess
In reply to this post by Andrew Burgess
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 in first, and then building view
support on top of this.

The gdb.cp/step-and-next-inline.exp test is based off a test proposed
by Bernd Edlinger in this 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.
        * dwarf2/read.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.  Also add an extra
        comment.
        * 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/step-and-next-inline.cc: New file.
        * gdb.cp/step-and-next-inline.exp: New file.
        * gdb.cp/step-and-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.
---
 gdb/ChangeLog                                 |  38 ++++
 gdb/buildsym-legacy.c                         |   4 +-
 gdb/buildsym.c                                |  14 +-
 gdb/buildsym.h                                |   3 +-
 gdb/disasm.c                                  |   6 +
 gdb/dwarf2/read.c                             |  13 +-
 gdb/infrun.c                                  |  51 +++--
 gdb/jit.c                                     |   1 +
 gdb/record-btrace.c                           |  11 +-
 gdb/stack.c                                   |   2 +-
 gdb/symmisc.c                                 |  10 +-
 gdb/symtab.c                                  |  25 ++-
 gdb/symtab.h                                  |  10 +
 gdb/testsuite/ChangeLog                       |  11 ++
 gdb/testsuite/gdb.cp/step-and-next-inline.cc  |  66 +++++++
 gdb/testsuite/gdb.cp/step-and-next-inline.exp | 119 ++++++++++++
 gdb/testsuite/gdb.cp/step-and-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, 1095 insertions(+), 29 deletions(-)
 create mode 100644 gdb/testsuite/gdb.cp/step-and-next-inline.cc
 create mode 100644 gdb/testsuite/gdb.cp/step-and-next-inline.exp
 create mode 100644 gdb/testsuite/gdb.cp/step-and-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

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 84cb44277a4..24aeba8e252 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/dwarf2/read.c b/gdb/dwarf2/read.c
index 1d4397dfabc..1706b96cc04 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -19939,7 +19939,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);
@@ -19953,7 +19953,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.
@@ -19976,7 +19976,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
@@ -20003,8 +20003,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)
@@ -20015,6 +20014,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))
@@ -20022,7 +20023,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 2a319295d36..d672d1a1609 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7045,6 +7045,10 @@ process_event_stop_test (struct execution_control_state *ecs)
  }
     }
 
+  /* This always returns the sal for the inner-most frame when we are in a
+     stack of inlined frames, even if GDB actually believes that it is in a
+     more outer frame.  This is checked for below by calls to
+     inline_skipped_frames.  */
   stop_pc_sal = find_pc_line (ecs->event_thread->suspend.stop_pc, 0);
 
   /* NOTE: tausq/2004-05-24: This if block used to be done before all
@@ -7179,19 +7183,36 @@ 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 if (frame_id_eq (get_frame_id (get_current_frame ()),
+    ecs->event_thread->control.step_frame_id))
+ {
+  /* We are at the start of a different line, however, this line is
+     not marked as a statement, and we have not changed frame.  We
+     ignore this line table entry, and continue stepping forward,
+     looking for a better place to stop.  */
+  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.
@@ -7199,12 +7220,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 (ecs->event_thread, frame, stop_pc_sal);
+  if (refresh_step_info)
+    set_step_info (ecs->event_thread, 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 35b7167270d..8eb98f4a38a 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -579,6 +579,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 266d771e35f..024ead0b611 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.  */
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index 1d7c3816670..4bf1f08849f 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -301,6 +301,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");
  }
     }
@@ -987,8 +989,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)
  {
@@ -1000,7 +1002,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 a80b80db5af..44b711397d5 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3236,6 +3236,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;
@@ -3266,6 +3283,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;
@@ -3434,7 +3452,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;
@@ -3545,6 +3564,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/step-and-next-inline.cc b/gdb/testsuite/gdb.cp/step-and-next-inline.cc
new file mode 100644
index 00000000000..d923cc53413
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/step-and-next-inline.cc
@@ -0,0 +1,66 @@
+/* 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 "step-and-next-inline.h"
+
+#else /* USE_NEXT_INLINE_H */
+
+/* The code below must remain identical to the code in
+   step-and-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/step-and-next-inline.exp b/gdb/testsuite/gdb.cp/step-and-next-inline.exp
new file mode 100644
index 00000000000..acec48ba81d
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/step-and-next-inline.exp
@@ -0,0 +1,119 @@
+# 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\ -gstatement-frontiers}
+    if { $use_header } {
+ lappend options additional_flags=-DUSE_NEXT_INLINE_H
+ set executable "$testfile-with-header"
+ set hdrfile "step-and-next-inline.h"
+ set prefix "use_header"
+    } else {
+ set executable "$testfile-no-header"
+ set hdrfile "$srcfile"
+ set prefix "no_header"
+    }
+
+    if { [prepare_for_testing "failed to prepare" $executable \
+      $srcfile $options] } {
+ return -1
+    }
+
+    with_test_prefix $prefix {
+
+    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"
+
+    if {!$use_header} {
+ # With the debug from GCC 10.x (and earlier) GDB is currently
+ # unable to successfully complete the following tests when we
+ # are not using a header file.
+ kfail symtab/25507 "stepping tests"
+ return
+    }
+
+    clean_restart ${executable}
+
+    if ![runto_main] {
+ fail "can't run to main pass 2"
+ return
+    }
+
+    gdb_test "bt" "\\s*\\#0\\s+main.*" "in main pass 2"
+    gdb_test "step" ".*" "step into get_alias_set pass 2"
+    gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \
+ "in get_alias_set pass 2"
+    gdb_test "step" ".*TREE_TYPE.*" "step 1"
+    gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \
+ "not in inline 1 pass 2"
+    gdb_test "step" ".*if \\(t->x != i\\).*" "step 2"
+    gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \
+ "in inline 1 pass 2"
+    gdb_test "step" ".*TREE_TYPE.*" "step 3"
+    gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \
+ "not in inline 2 pass 2"
+    gdb_test "step" ".*if \\(t->x != i\\).*" "step 4"
+    gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \
+ "in inline 2 pass 2"
+    gdb_test "step" ".*TREE_TYPE.*" "step 5"
+    gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \
+ "not in inline 3 pass 2"
+    gdb_test "step" ".*if \\(t->x != i\\).*" "step 6"
+    gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \
+ "in inline 3 pass 2"
+    gdb_test "step" "return 0.*" "step 7"
+    gdb_test "bt" \
+ "\\s*\\#0\\s+(main|get_alias_set)\[^\r\]*${srcfile}:.*" \
+ "not in inline 4 pass 2"
+    }
+}
+
+do_test 0
+do_test 1
diff --git a/gdb/testsuite/gdb.cp/step-and-next-inline.h b/gdb/testsuite/gdb.cp/step-and-next-inline.h
new file mode 100644
index 00000000000..6c6d90a37ff
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/step-and-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
+   step-and-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..436c4d01024
--- /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..1bcf5b0c698
--- /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;
 
--
2.14.5

Reply | Threaded
Open this post in threaded view
|

Re: [PATCHv2 0/2] Line table is_stmt support

Bernd Edlinger-2
In reply to this post by Andrew Burgess
On 3/8/20 1:50 PM, Andrew Burgess wrote:

> Patch #1 is unchanged.
>
> Patch #2 includes additional changes in infrun.c based on Bernd's
> suggested fix, as well as his additional tests.
>
> Bernd,
>
> If you are happy with this version of the patch that I'll merge this
> in the next few days.
>

Sure, a quick smoke test shows this is still on the right track.

I will post a re-based version of my follow-up patch in a moment.


Thanks
Bernd.

> Thanks,
> Andrew
>
> --
>
> Andrew Burgess (2):
>   gdb/testsuite: Add is-stmt support to the DWARF compiler
>   gdb: Add support for tracking the DWARF line table is-stmt field
>
>  gdb/ChangeLog                                 |  38 ++++
>  gdb/buildsym-legacy.c                         |   4 +-
>  gdb/buildsym.c                                |  14 +-
>  gdb/buildsym.h                                |   3 +-
>  gdb/disasm.c                                  |   6 +
>  gdb/dwarf2/read.c                             |  13 +-
>  gdb/infrun.c                                  |  51 +++--
>  gdb/jit.c                                     |   1 +
>  gdb/record-btrace.c                           |  11 +-
>  gdb/stack.c                                   |   2 +-
>  gdb/symmisc.c                                 |  10 +-
>  gdb/symtab.c                                  |  25 ++-
>  gdb/symtab.h                                  |  10 +
>  gdb/testsuite/ChangeLog                       |  16 ++
>  gdb/testsuite/gdb.cp/step-and-next-inline.cc  |  66 +++++++
>  gdb/testsuite/gdb.cp/step-and-next-inline.exp | 119 ++++++++++++
>  gdb/testsuite/gdb.cp/step-and-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/testsuite/lib/dwarf.exp                   |   8 +-
>  gdb/xcoffread.c                               |   4 +
>  24 files changed, 1107 insertions(+), 30 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.cp/step-and-next-inline.cc
>  create mode 100644 gdb/testsuite/gdb.cp/step-and-next-inline.exp
>  create mode 100644 gdb/testsuite/gdb.cp/step-and-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
>
Reply | Threaded
Open this post in threaded view
|

[PATCHv3] Fix range end handling of inlined subroutines

Bernd Edlinger-2
In reply to this post by Bernd Edlinger-2
On 2/22/20 7:38 AM, Bernd Edlinger wrote:

> On 2/9/20 10:07 PM, Bernd Edlinger wrote:
>> Hi,
>>
>> this is based on Andrew's patch here:
>>
>> https://sourceware.org/ml/gdb-patches/2020-02/msg00092.html
>>
>> This and adds a heuristic to fix the case where caller
>> and callee reside in the same subfile, it uses
>> the recorded is-stmt bits and locates end of
>> range infos, including the previously ignored empty
>> range, and adjusting is-stmt info at this
>> same pc, when the last item is not-is-stmt, the
>> previous line table entries are dubious and
>> we reset the is-stmt bit there.
>> This fixes the new test case in Andrew's patch.
>>
>> It understood, that this is just a heuristic
>> approach, since we do not have exactly the data,
>> which is needed to determine at which of the identical
>> PCs in the line table the subroutine actually ends.
>> So, this tries to be as conservative as possible,
>> just changing what is absolutely necessary.
>>
>> This patch itself is preliminary, since the is-stmt patch
>> needs to be rebased after the refactoring of
>> dwarf2read.c from yesterday, so I will have to rebase
>> this patch as well, but decided to wait for Andrew.
>>
>>
>
> So, this is an update to my previous patch above:
> https://sourceware.org/ml/gdb-patches/2020-02/msg00262.html
>
> It improves performance on big data, by using binary
> search to locate the bogus line table entries.
> Otherwise it behaves identical to the previous version,
> and is still waiting for Andrew's patch before it can
> be applied.
>
>

Rebased to match Andrew's updated patch of today.


Thanks
Bernd.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCHv2 0/2] Line table is_stmt support

Andrew Burgess
In reply to this post by Bernd Edlinger-2
* Bernd Edlinger <[hidden email]> [2020-03-08 14:39:44 +0000]:

> On 3/8/20 1:50 PM, Andrew Burgess wrote:
> > Patch #1 is unchanged.
> >
> > Patch #2 includes additional changes in infrun.c based on Bernd's
> > suggested fix, as well as his additional tests.
> >
> > Bernd,
> >
> > If you are happy with this version of the patch that I'll merge this
> > in the next few days.
> >
>
> Sure, a quick smoke test shows this is still on the right track.
>
> I will post a re-based version of my follow-up patch in a moment.

I have now pushed this series to master.  I will review your follow up
patch in more detail tomorrow.

Thanks,
Andrew



>
>
> Thanks
> Bernd.
>
> > Thanks,
> > Andrew
> >
> > --
> >
> > Andrew Burgess (2):
> >   gdb/testsuite: Add is-stmt support to the DWARF compiler
> >   gdb: Add support for tracking the DWARF line table is-stmt field
> >
> >  gdb/ChangeLog                                 |  38 ++++
> >  gdb/buildsym-legacy.c                         |   4 +-
> >  gdb/buildsym.c                                |  14 +-
> >  gdb/buildsym.h                                |   3 +-
> >  gdb/disasm.c                                  |   6 +
> >  gdb/dwarf2/read.c                             |  13 +-
> >  gdb/infrun.c                                  |  51 +++--
> >  gdb/jit.c                                     |   1 +
> >  gdb/record-btrace.c                           |  11 +-
> >  gdb/stack.c                                   |   2 +-
> >  gdb/symmisc.c                                 |  10 +-
> >  gdb/symtab.c                                  |  25 ++-
> >  gdb/symtab.h                                  |  10 +
> >  gdb/testsuite/ChangeLog                       |  16 ++
> >  gdb/testsuite/gdb.cp/step-and-next-inline.cc  |  66 +++++++
> >  gdb/testsuite/gdb.cp/step-and-next-inline.exp | 119 ++++++++++++
> >  gdb/testsuite/gdb.cp/step-and-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/testsuite/lib/dwarf.exp                   |   8 +-
> >  gdb/xcoffread.c                               |   4 +
> >  24 files changed, 1107 insertions(+), 30 deletions(-)
> >  create mode 100644 gdb/testsuite/gdb.cp/step-and-next-inline.cc
> >  create mode 100644 gdb/testsuite/gdb.cp/step-and-next-inline.exp
> >  create mode 100644 gdb/testsuite/gdb.cp/step-and-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
> >
1234