[PATCH 0/2] Handle "line 0" ranges (PR26243, PR15314, PR15668)

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

[PATCH 0/2] Handle "line 0" ranges (PR26243, PR15314, PR15668)

Pedro Alves-2
PR 26243 shows that Clang associates some instructions in the middle
of functions to line 0.  That is valid DWARF, but it wasn't noticed
until recently, when the line info reading code was tweaked.
Currently, "step" and "next" with Clang misbehave because these "line
0" instructions or instruction ranges aren't being handled.

This series fixes that in two parts:

#1 - By teaching infrun to step over such no-line-info instructions
     automatically, when "set step-mode" is "off" (which is the
     default).

#2 - By making "step" and "next" behave like "stepi" and "nexti"
     respectively when a step is started at an instruction with no
     line info.

I think that with the first patch, most users won't frequently notice
these "no line info" regions unless they use stepi to run to them, or
they set a breapoint by address in them.  But it can happen that you
stop in one of them, and I think that making "step" not step out of
the whole function is just a good idea if it does happen.  The second
patch also fixes the older PR15314 and PR15668, because the error in
question they are complaining about is removed by that patch.

I think we could also try to fix the issue addressed by #2 by making a
"step" started at an instruction with no line info step until it finds
an instruction with line info (maybe in the same function, maybe in a
different function), instead of stepping out of the current function.
I do think that the behavior I'm proposing is more intuitive, though.

Pedro Alves (2):
  Keep stepping over "line 0" ranges (PR 26243)
  Make step act as stepi if no line info (PR26243, PR15314, PR15668)

 gdb/doc/gdb.texinfo                               |  36 ++--
 gdb/NEWS                                          |   5 +
 gdb/infcmd.c                                      |  30 +--
 gdb/infrun.c                                      |  47 +++--
 gdb/testsuite/gdb.base/step-symless.exp           |  20 +-
 gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.c   |  61 ++++++
 gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.exp | 240 ++++++++++++++++++++++
 7 files changed, 383 insertions(+), 56 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.exp


base-commit: 6d3d6e4ba779dc08b134cd1a09b055dbd88dbf8a
--
2.14.5

Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] Keep stepping over "line 0" ranges (PR 26243)

Pedro Alves-2
The DWARF standard states for the line register in the line number
information state machine the following:

 "An unsigned integer indicating a source line number.  Lines are
 numbered beginning at 1.  The compiler may emit the value 0 in cases
 where an instruction cannot be attributed to any source line."

So, it's possible to have a zero line number in the DWARF line table.

For some reason, Clang associates some instructions in the middle of
functions to line 0 using that form.  See PR 26243.

This is currently not handled well by GDB when using the next/step
commands.  The problem was masked until the recent line info reading
changes.

Let's use some simple dummy instruction / line mapping as an example:

 insn1 <-> line 10
 insn2 <-> line 0
 insn3 <-> line 0
 insn4 <-> line 11

- If an instruction is mapped to line 0, we should not hide the fact
  that the instruction maps to no source line.  We should not merge it
  with the preceding instruction's line, or something like that.

  So if you stepi until insn2, or put a breakpoint on it and run, it
  should not appear as if you stopped at line 10.  It should appear
  that you stopped somewhere without source.  I.e., present to the
  user the truth as the compiler produced it.  If the instructions
  should really be associated with a line, then this GDB behavior
  should be a good motivation for compilers to emit better debug info,
  where all instructions are covered, where possible.

- Getting from a region with line info to a region without line info.
  You are on insn1 and you do "next".  The current behavior is: the
  program stops at insn2, where you have no source available.

On one hand, it's "correct" to stop, because we went from an
instruction belonging to line 10 to an instruction not belonging to
line 10.  That's the conservative thing to do.  But that's also very
confusing to users who want to debug at source level.

There's a quite unknown setting, "set step-mode" that I think is
relevant here:

  (gdb) help set step-mode
  Set mode of the step operation.
  When set, doing a step over a function without debug line information
  will stop at the first instruction of that function. Otherwise, the
  function is skipped and the step command stops at a different source line.

It talks about "stepping over a function", but I think it should apply
here as well.  Basically, if you have step-mode off, then you want to
skip undebuggable code.  If "on", you want to stop stepping in
undebuggable code.

If it did (apply here as well), then when "off" (the default), GDB
would behave as it did historically, skipping those regions.  When
"on", a step would stop at the instruction with line 0.

That's what this patch does -- it makes the step/next commands keep
stepping when they go from an instruction with line info to an
instruction without line info.  In our example, doing a "next" while
stopped at insn1 would step until stopped at insn4.


The testcase added by this commit uses the DWARF assembler to write a
line table with the instructions that should be assigned to line 31
assigned to line 0 instead.  I.e., there's no line 31 in the line
info.  Note that with Clang-produced binaries the "holes" are
seemingly smaller than the whole set of instructions for a given line,
so the user doesn't notice anything missing, similarly to the example
above with lines 10 and 11.

With the testcase, and before the GDB fix, we see:

 (gdb) b bar1
 Breakpoint 2 at 0x55555555513f: file /net/cascais.nfs/gdb/binutils-gdb/src/gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.c, line 27.
 (gdb) c
 Continuing.

 Breakpoint 2, bar1 () at /net/cascais.nfs/gdb/binutils-gdb/src/gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.c:27
 27        foo (1);
 (gdb) list
 22
 23      void
 24      bar1 (void)
 25      {
 26        asm ("bar1_label: .globl bar1_label");
 27        foo (1);
 28        asm ("bar1_label_2: .globl bar1_label_2");
 29        foo (2);
 30        asm ("bar1_label_3: .globl bar1_label_3");
 31        foo (3);
 32        asm ("bar1_label_4: .globl bar1_label_4");
 33        foo (4);
 34        asm ("bar1_label_5: .globl bar1_label_5");
 35      }
 (gdb) n
 29        foo (2);
 (gdb) n
 0x0000555555555153 in bar1 ()
 (gdb) n
 Single stepping until exit from function bar1,
 which has no line number information.
 0x00005555555551aa in main ()
 (gdb)

Note how the step started at line 29 stopped at an address without
showing a line number.

If we instead kept single-stepping at line 29, we would eventually
reach line 33.

 (gdb) n
 29        foo (2);
 (gdb) ni
 0x000055555555514e      29        foo (2);
 (gdb)
 0x0000555555555153 in bar1 ()
 (gdb)
 0x0000555555555158 in bar1 ()
 (gdb)
 33        foo (4);

That is what this patch does.  With it, we get instead:

 27        foo (1);
 (gdb) n
 29        foo (2);
 (gdb) n
 33        foo (4);
 (gdb)

The testcase exercises various interesting aspects:

 1. Next through a zero-line instruction, is_stmt == 1
 2. Next through a zero-line instruction, is_stmt == 0
 3. Step through a zero-line instruction, is_stmt == 1
 4. Step through a zero-line instruction, is_stmt == 0
 5. Show source location at zero-line instruction, is_stmt == 1
 6. Show source location at zero-line instruction, is_stmt == 0

All the above with "set step-mode" either "on" or "off".

gdb/doc/ChangeLog:
2020-07-16  Pedro Alves  <[hidden email]>

        PR symtab/26243
        * gdb.texinfo (Continuing and Stepping) <set step-mode>: Describe
        behavior in function address ranges with no line info.

gdb/ChangeLog:
2020-07-16  Pedro Alves  <[hidden email]>

        PR symtab/26243
        * infrun.c (process_event_stop_test): If we have no line number
        information for the current PC, but have debug info for the
        function we started stepping, and are still in the same function,
        then continue stepping.

gdb/testsuite/ChangeLog:
2020-07-20  Tom de Vries  <[hidden email]>
            Pedro Alves  <[hidden email]>

        PR symtab/26243
        * gdb.dwarf2/dw2-line-number-zero.c: New test.
        * gdb.dwarf2/dw2-line-number-zero.exp: New file.
---
 gdb/doc/gdb.texinfo                               |  13 +-
 gdb/infrun.c                                      |  47 +++--
 gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.c   |  61 +++++++
 gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.exp | 210 ++++++++++++++++++++++
 4 files changed, 314 insertions(+), 17 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.exp

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a002084d5b9..99d5383f009 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -6037,14 +6037,19 @@ source line.  This prevents multiple stops that could otherwise occur in
 The @code{set step-mode on} command causes the @code{step} command to
 stop at the first instruction of a function which contains no debug line
 information rather than stepping over it.
+Similarly, causes the @code{step} and @code{next} commands to stop at
+the first instruction of a function address range if it contains no
+debug line information rather than stepping over it.
 
 This is useful in cases where you may be interested in inspecting the
-machine instructions of a function which has no symbolic info and do not
-want @value{GDBN} to automatically skip over this function.
+machine instructions of a function or function address range which has
+no symbolic info and do not want @value{GDBN} to automatically skip
+over this function or range.
 
 @item set step-mode off
-Causes the @code{step} command to step over any functions which contains no
-debug information.  This is the default.
+Causes the @code{step} and @code{next} commands to automatically step
+over any functions or function address ranges that have no line number
+information.  This is the default.
 
 @item show step-mode
 Show whether @value{GDBN} will stop in or step over functions without
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 31266109a6d..28a42676e07 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -127,9 +127,8 @@ mark_infrun_async_event_handler (void)
   mark_async_event_handler (infrun_async_inferior_event_token);
 }
 
-/* When set, stop the 'step' command if we enter a function which has
-   no line number information.  The normal behavior is that we step
-   over such function.  */
+/* For "set step-mode".  */
+
 bool step_stop_if_no_debug = false;
 static void
 show_step_stop_if_no_debug (struct ui_file *file, int from_tty,
@@ -7193,13 +7192,31 @@ process_event_stop_test (struct execution_control_state *ecs)
 
   if (stop_pc_sal.line == 0)
     {
-      /* We have no line number information.  That means to stop
-         stepping (does this always happen right after one instruction,
-         when we do "s" in a function with no line numbers,
-         or can this happen as a result of a return or longjmp?).  */
-      if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "infrun: no line number info\n");
-      end_stepping_range (ecs);
+      /* We have no line number information.  */
+
+      if (!step_stop_if_no_debug
+  && ecs->event_thread->control.step_start_function != nullptr
+  && (ecs->event_thread->control.step_start_function
+      == find_pc_function (ecs->event_thread->suspend.stop_pc)))
+ {
+  /* "set step-mode" is "off", and we're still in the same
+     function.  Continue stepping until we're out of the
+     no-line-info range.  */
+  if (debug_infrun)
+    fprintf_unfiltered (gdb_stdlog,
+ "infrun: no line number info, "
+ "but still in same function\n");
+  ecs->event_thread->control.may_range_step = 0;
+  keep_going (ecs);
+ }
+      else
+ {
+  /* Otherwise, stop stepping and let the user decide how to
+     proceed.  */
+  if (debug_infrun)
+    fprintf_unfiltered (gdb_stdlog, "infrun: no line number info\n");
+  end_stepping_range (ecs);
+ }
       return;
     }
 
@@ -9691,9 +9708,13 @@ mode (see help set scheduler-locking)."),
   add_setshow_boolean_cmd ("step-mode", class_run, &step_stop_if_no_debug, _("\
 Set mode of the step operation."), _("\
 Show mode of the step operation."), _("\
-When set, doing a step over a function without debug line information\n\
-will stop at the first instruction of that function. Otherwise, the\n\
-function is skipped and the step command stops at a different source line."),
+When on, stop the 'step' command if we enter a function that has no\n\
+line number information, and stop the 'step' and 'next' commands if\n\
+stepping within a function steps into an address range with no line\n\
+number information.\n\
+\n\
+When off (the default), stepping automatically skips functions and\n\
+function address ranges with no line number information."),
    NULL,
    show_step_stop_if_no_debug,
    &setlist, &showlist);
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.c b/gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.c
new file mode 100644
index 00000000000..e6f99ad7e0c
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.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/>.  */
+
+void
+foo (int x)
+{
+
+}
+
+void
+bar1 (void)
+{
+  asm ("bar1_label: .globl bar1_label");
+  foo (1);
+  asm ("bar1_label_2: .globl bar1_label_2");
+  foo (2);
+  asm ("bar1_label_3: .globl bar1_label_3");
+  foo (3);
+  asm ("bar1_label_4: .globl bar1_label_4");
+  foo (4);
+  asm ("bar1_label_5: .globl bar1_label_5");
+}
+
+void
+bar2 (void)
+{
+  asm ("bar2_label: .globl bar2_label");
+  foo (1);
+  asm ("bar2_label_2: .globl bar2_label_2");
+  foo (2);
+  asm ("bar2_label_3: .globl bar2_label_3");
+  foo (3);
+  asm ("bar2_label_4: .globl bar2_label_4");
+  foo (4);
+  asm ("bar2_label_5: .globl bar2_label_5");
+}
+
+int
+main (void)
+{
+  asm ("main_label: .globl main_label");
+
+  bar1 ();
+
+  bar2 ();
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.exp b/gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.exp
new file mode 100644
index 00000000000..91a23237759
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.exp
@@ -0,0 +1,210 @@
+# Copyright 2020 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test GDB's support for line 0 entries in the line table.  Per the
+# DWARF standard (v2 onward), "State Machine Registers" / "line": "The
+# compiler may emit the value 0 in cases where an instruction cannot
+# be attributed to any source line.".
+
+# With "set step-mode off" (which is the default), stepping does not
+# stop in such undebuggable code regions.  With "set step-mode on", it
+# does stop.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    verbose "Skipping dw2-line-number-zero test."
+    return 0
+}
+
+# The .c files use __attribute__.
+if [get_compiler_info] {
+    return -1
+}
+if !$gcc_compiled {
+    verbose "Skipping dw2-line-number-zero test."
+    return 0
+}
+
+standard_testfile .c dw2-line-number-zero-dw.S
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    global srcdir subdir srcfile
+    declare_labels Llines
+
+    cu {} {
+ compile_unit {
+    {language @DW_LANG_C}
+    {name dw2-line-number-zero.c}
+    {stmt_list $Llines DW_FORM_sec_offset}
+ } {
+    subprogram {
+ {external 1 flag}
+ {MACRO_AT_func {main}}
+    }
+    subprogram {
+ {external 1 flag}
+ {MACRO_AT_func {bar1}}
+    }
+    subprogram {
+ {external 1 flag}
+ {MACRO_AT_func {bar2}}
+    }
+ }
+    }
+
+    lines {version 2} Llines {
+ include_dir "${srcdir}/${subdir}"
+ file_name "$srcfile" 1
+
+ program {
+    {DW_LNE_set_address bar1_label}
+    {line 27}
+    {DW_LNS_copy}
+
+    {DW_LNE_set_address bar1_label_2}
+    {line 29}
+    {DW_LNS_copy}
+
+    {DW_LNE_set_address bar1_label_3}
+    {line 0}
+    {DW_LNS_copy}
+
+    {DW_LNE_set_address bar1_label_4}
+    {line 33}
+    {DW_LNS_copy}
+
+    {DW_LNE_set_address bar1_label_5}
+    {DW_LNE_end_sequence}
+
+
+    {DW_LNE_set_address bar2_label}
+    {line 41}
+    {DW_LNS_copy}
+
+    {DW_LNE_set_address bar2_label_2}
+    {line 43}
+    {DW_LNS_copy}
+
+    {DW_LNE_set_address bar2_label_3}
+    {line 0}
+    {DW_LNS_negate_stmt}
+    {DW_LNS_copy}
+    {DW_LNS_negate_stmt}
+
+    {DW_LNE_set_address bar2_label_4}
+    {line 47}
+    {DW_LNS_copy}
+
+    {DW_LNE_set_address bar2_label_5}
+    {DW_LNE_end_sequence}
+ }
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+# Test that "next" automatically steps over the line 0 instructions,
+# stopping at the next source line.
+
+proc_with_prefix test_next_step_mode_off {} {
+    if ![runto_main] {
+ return -1
+    }
+
+    foreach {function start_line} {
+ "bar1" 27
+ "bar2" 41
+    } {
+ global hex gdb_prompt
+
+ with_test_prefix $function {
+    gdb_breakpoint $function
+    gdb_continue_to_breakpoint $function "\[^\r\n\]*:$start_line\r\n.*"
+
+    gdb_test "n" "foo \\(2\\);" "1st next"
+    gdb_test "n" "foo \\(4\\);" "2nd next"
+ }
+    }
+}
+
+# Test that with "set step-mode on", "next" stops at line 0
+# instructions.
+
+proc_with_prefix test_next_step_mode_on {} {
+    global hex gdb_prompt
+
+    if ![runto_main] {
+ return -1
+    }
+
+    gdb_test "set step-mode on"
+
+    foreach {function start_line} {
+ "bar1" 27
+ "bar2" 41
+    } {
+ global hex gdb_prompt
+
+ with_test_prefix $function {
+    gdb_breakpoint $function
+    gdb_continue_to_breakpoint $function "\[^\r\n\]*:$start_line\r\n.*"
+
+    gdb_test "n" "foo \\(2\\);" "next over normal line region"
+
+    set saw_no_line 0
+    gdb_test_multiple "n" "next over line0 region" {
+ -re "$hex in $function \\(\\)\r\n$gdb_prompt $" {
+    incr saw_no_line
+
+    # Avoid nexting forever if the testcase goes wild.
+    if {$saw_no_line > 100} {
+ fail $gdb_test_name
+    }
+
+    send_gdb "n\n"
+    exp_continue
+ }
+ -re "foo \\(4\\);\r\n$gdb_prompt $" {
+    gdb_assert $saw_no_line $gdb_test_name
+ }
+    }
+ }
+    }
+}
+
+# Test running to a breakpoint at an address associated with line 0.
+# GDB should not show any line info for the stop location.
+
+proc_with_prefix test_bkpt {} {
+    if ![runto_main] {
+ return -1
+    }
+
+    gdb_breakpoint "bar1_label_3"
+    gdb_continue_to_breakpoint "bar1_label_3" "bar1 \\(\\)"
+
+    gdb_breakpoint "bar2_label_3"
+    gdb_continue_to_breakpoint "bar2_label_3" "bar2 \\(\\)"
+}
+
+test_next_step_mode_off
+test_next_step_mode_on
+test_bkpt
--
2.14.5

Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] Make step act as stepi if no line info (PR26243, PR15314, PR15668)

Pedro Alves-2
In reply to this post by Pedro Alves-2
Currently, by default, if you do step/next when stopped at an
instruction with no line info, GDB steps until out of the function:

 Breakpoint 5, 0x0000555555555153 in bar1 ()
 (gdb) s
 Single stepping until exit from function bar1,
 which has no line number information.
 0x00005555555551aa in main ()
 (gdb)

That happens with "set step-mode off", which is the default.

If you enable "set step-mode on", then "step" behaves like "stepi"
instead:

 Breakpoint 5, 0x0000555555555153 in bar1 ()
 (gdb) s
 0x0000555555555158 in bar1 ()
 (gdb)
 0x0000555555555129 in foo ()
 (gdb)
 0x000055555555512d in foo ()
 (gdb)
 0x000055555555512e in foo ()
 (gdb)
 0x0000555555555131 in foo ()
 (gdb)
 0x0000555555555134 in foo ()
 (gdb)
 0x0000555555555135 in foo ()
 (gdb)
 0x0000555555555136 in foo ()
 (gdb)
 bar1 () at src/gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.c:33
 33        foo (4);
 (gdb)

I don't think the "set step-mode off" behavior is very useful.  I find
it very surprising, even.  "If I wanted to step out, I would have used
the finish command!" is what crosses my mind.  The implementation also
stops at a callee of the current function, but that's also
unintuitive, IHMO.

When "set step-mode" is off, I think it makes sense to step over
functions with no line info _iff you started stepping from an address
with line info_.  I.e., if you started a step at the source level.

But, if you're stopped at an instruction with no line info, you're
really debugging at the assembly level.  When you're stopped at such
an instruction with no source mapping, it is reasonable for IDEs to
show you a disassemble view around the current instruction.  That is
e.g., what cgdb does, and I think the TUI should be tweaked to do the
same as well.  In this scenario, stepping a line can be reasonably
explained as stepping one line of assembly.

Changing GDB's behavior here gets particularly important when we have
instructions with lines 0 in the middle of functions, such as like
Clang outputs.  If you stop in one of those instructions (e.g., with a
breakpoint), and issue a "step", GDB steps out of the function, even
though there is still more debuggable code with line info in the
function.  Note how in the "set step-mode on" example above, we ended
up at line 33 still within bar1.  While in the "set step-mode off"
case, we stepped all the way out of bar1.

I think it's a lot more intuitive to have a "step" behave like "stepi"
when stopped somewhere with no line info (i.e., with no mapping from
the current instruction to a high-level source).

So that's what this patches does.  If we have no line info for the
current instruction when we start a step or a next, then step acts as
a stepi, and next acts as a nexti, regardless of "set step-mode".

The gdb.dwarf2/dw2-line-number-zero.exp testcase is extended to
exercise this new behavior, with both "set step-mode on" and "set
step-mode off".  The gdb.base/step-symless.exp testcase was expecting the
previous behavior, so it got adjusted.

This ends up fixing two older PRs:

 Bug 15314 - Why does this error exist? "Cannot find bounds of current function"
 https://sourceware.org/bugzilla/show_bug.cgi?id=15314

 Bug 15668 - Cannot find bounds of current function
 https://sourceware.org/bugzilla/show_bug.cgi?id=15668

In PR 15668, the user gets confused with "step" not working.

Funnily, in PR 15314, I had already suggested switching to stepi years
ago...

The documentation is adjusted to match the new behavior, and cleaned
up a little along the way:

  - The reference to some old MIPS toolchain and to what GDB used to
    be is more distracting than helpful.

  - We already talk about line info throughout the manual, I see no
    point in trying to avoid it.

  - We no longer to use a "warning" to talk about the behavior without
    line info, since the new behavior isn't as "destructive".

gdb/doc/ChangeLog:

        * gdb.texinfo (Continuing and Stepping): Describe that "step"
        behaves like "stepi" if there's no line info.

gdb/ChangeLog:

        * NEWS: Mention new "step" and "next" behavior.
        * infcmd.c (prepare_one_step): If we have no line info, switch to
        "stepi/nexti" mode, regardless of "set step-mode".

gdb/testsuite/ChangeLog:

        * gdb.base/step-symless.exp: Step until the breakpoint is reached.
        * gdb.dwarf2/dw2-line-number-zero.exp (test_bkpt): Rename to ...
        (test_bkpt_step): ... this.  Add "step_mode" parameter.  Test
        stepping after reaching the no-line-info breakpoint.
---
 gdb/doc/gdb.texinfo                               | 23 ++++--------
 gdb/NEWS                                          |  5 +++
 gdb/infcmd.c                                      | 30 +++++-----------
 gdb/testsuite/gdb.base/step-symless.exp           | 20 ++++++++++-
 gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.exp | 44 +++++++++++++++++++----
 5 files changed, 76 insertions(+), 46 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 99d5383f009..ef14ba805bb 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -5973,20 +5973,6 @@ Continue running your program until control reaches a different source
 line, then stop it and return control to @value{GDBN}.  This command is
 abbreviated @code{s}.
 
-@quotation
-@c "without debugging information" is imprecise; actually "without line
-@c numbers in the debugging information".  (gcc -g1 has debugging info but
-@c not line numbers).  But it seems complex to try to make that
-@c distinction here.
-@emph{Warning:} If you use the @code{step} command while control is
-within a function that was compiled without debugging information,
-execution proceeds until control reaches a function that does have
-debugging information.  Likewise, it will not step into a function which
-is compiled without debugging information.  To step through functions
-without debugging information, use the @code{stepi} command, described
-below.
-@end quotation
-
 The @code{step} command only stops at the first instruction of a source
 line.  This prevents the multiple stops that could otherwise occur in
 @code{switch} statements, @code{for} loops, etc.  @code{step} continues
@@ -5996,9 +5982,12 @@ called within the line.
 
 Also, the @code{step} command only enters a function if there is line
 number information for the function.  Otherwise it acts like the
-@code{next} command.  This avoids problems when using @code{cc -gl}
-on @acronym{MIPS} machines.  Previously, @code{step} entered subroutines if there
-was any debugging information about the routine.
+@code{next} command.
+
+If, when you enter the @code{step} command, control is already within
+a function that was compiled without line information, the command
+behaves like the @code{stepi} command.  I.e., it executes one machine
+instruction, then stops and returns to the debugger.
 
 @item step @var{count}
 Continue running as in @code{step}, but do so @var{count} times.  If a
diff --git a/gdb/NEWS b/gdb/NEWS
index 001dc5e4683..875fb3c4852 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -100,6 +100,11 @@ alias [-a] [--] ALIAS = COMMAND [DEFAULT-ARGS...]
   defines the alias pp10 that will pretty print a maximum of 10 elements
   of the given expression (if the expression is an array).
 
+step / next
+  The 'step' and 'next' commands no longer single step until exit from
+  the current function when there's no line info for the function.
+  Instead, they behave like "stepi" and "nexti" respectively.
+
 * New targets
 
 GNU/Linux/RISC-V (gdbserver) riscv*-*-linux*
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index cfc31699925..c3b18c3593a 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -970,6 +970,8 @@ prepare_one_step (thread_info *tp, struct step_command_fsm *sm)
 
       set_step_frame (tp);
 
+      bool single_inst_once = false;
+
       if (!sm->single_inst)
  {
   CORE_ADDR pc;
@@ -1009,30 +1011,16 @@ prepare_one_step (thread_info *tp, struct step_command_fsm *sm)
  &tp->control.step_range_start,
  &tp->control.step_range_end);
 
-  tp->control.may_range_step = 1;
-
   /* If we have no line info, switch to stepi mode.  */
-  if (tp->control.step_range_end == 0 && step_stop_if_no_debug)
-    {
-      tp->control.step_range_start = tp->control.step_range_end = 1;
-      tp->control.may_range_step = 0;
-    }
-  else if (tp->control.step_range_end == 0)
-    {
-      const char *name;
-
-      if (find_pc_partial_function (pc, &name,
-    &tp->control.step_range_start,
-    &tp->control.step_range_end) == 0)
- error (_("Cannot find bounds of current function"));
-
-      target_terminal::ours_for_output ();
-      printf_filtered (_("Single stepping until exit from function %s,"
- "\nwhich has no line number information.\n"),
-       name);
-    }
+  if (tp->control.step_range_end == 0)
+    single_inst_once = true;
+  else
+    tp->control.may_range_step = 1;
  }
       else
+ single_inst_once = true;
+
+      if (single_inst_once)
  {
   /* Say we are stepping, but stop after one insn whatever it does.  */
   tp->control.step_range_start = tp->control.step_range_end = 1;
diff --git a/gdb/testsuite/gdb.base/step-symless.exp b/gdb/testsuite/gdb.base/step-symless.exp
index adff2b3e1ee..be806344561 100644
--- a/gdb/testsuite/gdb.base/step-symless.exp
+++ b/gdb/testsuite/gdb.base/step-symless.exp
@@ -38,4 +38,22 @@ if ![runto_main] {
 
 gdb_breakpoint symful
 
-gdb_test "step" "Single stepping until exit.*no line number information.*\r\nBreakpoint \[^\r\n\]* in \\.?symful \\(\\)"
+# With no line info, "step" behaves like "stepi".  Step until we reach
+# the breakpoint at "symful".
+set steps 0
+gdb_test_multiple "step" "" {
+    -re "$hex in main \\(\\)\r\n$gdb_prompt $" {
+ incr steps
+
+ # Avoid stepping forever if the testcase goes wild.
+ if {$steps > 100} {
+    fail $gdb_test_name
+ } else {
+    send_gdb "step\n"
+    exp_continue
+ }
+    }
+    -re "Breakpoint \[^\r\n\]* in \\.?symful \\(\\)\r\n$gdb_prompt $" {
+ pass $gdb_test_name
+    }
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.exp b/gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.exp
index 91a23237759..4de4a83ff85 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.exp
@@ -191,20 +191,50 @@ proc_with_prefix test_next_step_mode_on {} {
 }
 
 # Test running to a breakpoint at an address associated with line 0.
-# GDB should not show any line info for the stop location.
+# GDB should not show any line info for the stop location.  Then,
+# check that the "step" command behaves as "stepi" when there's no
+# line info, regardless of "set step-mode".
 
-proc_with_prefix test_bkpt {} {
+proc_with_prefix test_bkpt_step {step_mode} {
     if ![runto_main] {
  return -1
     }
 
-    gdb_breakpoint "bar1_label_3"
-    gdb_continue_to_breakpoint "bar1_label_3" "bar1 \\(\\)"
+    gdb_test_no_output "set step-mode $step_mode"
 
-    gdb_breakpoint "bar2_label_3"
-    gdb_continue_to_breakpoint "bar2_label_3" "bar2 \\(\\)"
+    foreach {function label} {
+ "bar1" bar1_label_3
+ "bar2" bar2_label_3
+    } {
+ global hex gdb_prompt
+
+ with_test_prefix $function {
+    gdb_breakpoint $label
+    gdb_continue_to_breakpoint $label "$function \\(\\)"
+
+    set steps 0
+    gdb_test_multiple "s" "step at line0 region" {
+ -re "$hex in .* \\(\\)\r\n$gdb_prompt $" {
+    incr steps
+
+    # Avoid stepping forever if the testcase goes wild.
+    if {$steps > 100} {
+ fail $gdb_test_name
+    } else {
+ send_gdb "s\n"
+ exp_continue
+    }
+ }
+ -re "foo \\(4\\);\r\n$gdb_prompt $" {
+    pass $gdb_test_name
+ }
+    }
+ }
+    }
 }
 
 test_next_step_mode_off
 test_next_step_mode_on
-test_bkpt
+foreach_with_prefix step_mode {"on" "off"} {
+    test_bkpt_step $step_mode
+}
--
2.14.5

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Handle "line 0" ranges (PR26243, PR15314, PR15668)

Simon Marchi-4
In reply to this post by Pedro Alves-2
On 2020-07-21 11:37 a.m., Pedro Alves wrote:

> PR 26243 shows that Clang associates some instructions in the middle
> of functions to line 0.  That is valid DWARF, but it wasn't noticed
> until recently, when the line info reading code was tweaked.
> Currently, "step" and "next" with Clang misbehave because these "line
> 0" instructions or instruction ranges aren't being handled.
>
> This series fixes that in two parts:
>
> #1 - By teaching infrun to step over such no-line-info instructions
>      automatically, when "set step-mode" is "off" (which is the
>      default).
>
> #2 - By making "step" and "next" behave like "stepi" and "nexti"
>      respectively when a step is started at an instruction with no
>      line info.
>
> I think that with the first patch, most users won't frequently notice
> these "no line info" regions unless they use stepi to run to them, or
> they set a breapoint by address in them.  But it can happen that you
> stop in one of them, and I think that making "step" not step out of
> the whole function is just a good idea if it does happen.  The second
> patch also fixes the older PR15314 and PR15668, because the error in
> question they are complaining about is removed by that patch.
>
> I think we could also try to fix the issue addressed by #2 by making a
> "step" started at an instruction with no line info step until it finds
> an instruction with line info (maybe in the same function, maybe in a
> different function), instead of stepping out of the current function.
> I do think that the behavior I'm proposing is more intuitive, though.
>
> Pedro Alves (2):
>   Keep stepping over "line 0" ranges (PR 26243)
>   Make step act as stepi if no line info (PR26243, PR15314, PR15668)
>
>  gdb/doc/gdb.texinfo                               |  36 ++--
>  gdb/NEWS                                          |   5 +
>  gdb/infcmd.c                                      |  30 +--
>  gdb/infrun.c                                      |  47 +++--
>  gdb/testsuite/gdb.base/step-symless.exp           |  20 +-
>  gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.c   |  61 ++++++
>  gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.exp | 240 ++++++++++++++++++++++
>  7 files changed, 383 insertions(+), 56 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.c
>  create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.exp
>
>
> base-commit: 6d3d6e4ba779dc08b134cd1a09b055dbd88dbf8a
> --
> 2.14.5
>

Because it can affect what kind of other bugfixes and mitigation we do
and how this patchset is discussed, my first question is: do you suggest
merging these patches before the GDB 10 branch is created or waiting
after the branch?

Simon
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Keep stepping over "line 0" ranges (PR 26243)

Eli Zaretskii
In reply to this post by Pedro Alves-2
> From: Pedro Alves <[hidden email]>
> Date: Tue, 21 Jul 2020 16:37:36 +0100
>
>  The @code{set step-mode on} command causes the @code{step} command to
>  stop at the first instruction of a function which contains no debug line
>  information rather than stepping over it.
> +Similarly, causes the @code{step} and @code{next} commands to stop at
   ^^^^^^^^^^^^^^^^^
I guess you meant "Similarly, it causes ..."?

>  This is useful in cases where you may be interested in inspecting the
> -machine instructions of a function which has no symbolic info and do not
> -want @value{GDBN} to automatically skip over this function.
> +machine instructions of a function or function address range which has
> +no symbolic info and do not want @value{GDBN} to automatically skip
                   ^
A comma here would disambiguate the sentence.

> +When on, stop the 'step' command if we enter a function that has no\n\
            ^^^^^^^^^^^^^^^^^^^^^^^
How do you "stop the 'step' command"?  It sounds awkward.

> +line number information, and stop the 'step' and 'next' commands if\n\
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Same here.

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

Re: [PATCH 2/2] Make step act as stepi if no line info (PR26243, PR15314, PR15668)

Eli Zaretskii
In reply to this post by Pedro Alves-2
> From: Pedro Alves <[hidden email]>
> Date: Tue, 21 Jul 2020 16:37:37 +0100
>
> gdb/doc/ChangeLog:
>
> * gdb.texinfo (Continuing and Stepping): Describe that "step"
> behaves like "stepi" if there's no line info.
>
> gdb/ChangeLog:
>
> * NEWS: Mention new "step" and "next" behavior.
> * infcmd.c (prepare_one_step): If we have no line info, switch to
> "stepi/nexti" mode, regardless of "set step-mode".
>
> gdb/testsuite/ChangeLog:
>
> * gdb.base/step-symless.exp: Step until the breakpoint is reached.
> * gdb.dwarf2/dw2-line-number-zero.exp (test_bkpt): Rename to ...
> (test_bkpt_step): ... this.  Add "step_mode" parameter.  Test
> stepping after reaching the no-line-info breakpoint.

OK for the documentation parts.

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

Re: [PATCH 0/2] Handle "line 0" ranges (PR26243, PR15314, PR15668)

Pedro Alves-2
In reply to this post by Simon Marchi-4
On 7/21/20 4:48 PM, Simon Marchi wrote:

> On 2020-07-21 11:37 a.m., Pedro Alves wrote:
>> PR 26243 shows that Clang associates some instructions in the middle
>> of functions to line 0.  That is valid DWARF, but it wasn't noticed
>> until recently, when the line info reading code was tweaked.
>> Currently, "step" and "next" with Clang misbehave because these "line
>> 0" instructions or instruction ranges aren't being handled.
>>
>> This series fixes that in two parts:
>>
>> #1 - By teaching infrun to step over such no-line-info instructions
>>      automatically, when "set step-mode" is "off" (which is the
>>      default).
>>
>> #2 - By making "step" and "next" behave like "stepi" and "nexti"
>>      respectively when a step is started at an instruction with no
>>      line info.
>>
>> I think that with the first patch, most users won't frequently notice
>> these "no line info" regions unless they use stepi to run to them, or
>> they set a breapoint by address in them.  But it can happen that you
>> stop in one of them, and I think that making "step" not step out of
>> the whole function is just a good idea if it does happen.  The second
>> patch also fixes the older PR15314 and PR15668, because the error in
>> question they are complaining about is removed by that patch.
>>
>> I think we could also try to fix the issue addressed by #2 by making a
>> "step" started at an instruction with no line info step until it finds
>> an instruction with line info (maybe in the same function, maybe in a
>> different function), instead of stepping out of the current function.
>> I do think that the behavior I'm proposing is more intuitive, though.
>>
>> Pedro Alves (2):
>>   Keep stepping over "line 0" ranges (PR 26243)
>>   Make step act as stepi if no line info (PR26243, PR15314, PR15668)
>>
>>  gdb/doc/gdb.texinfo                               |  36 ++--
>>  gdb/NEWS                                          |   5 +
>>  gdb/infcmd.c                                      |  30 +--
>>  gdb/infrun.c                                      |  47 +++--
>>  gdb/testsuite/gdb.base/step-symless.exp           |  20 +-
>>  gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.c   |  61 ++++++
>>  gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.exp | 240 ++++++++++++++++++++++
>>  7 files changed, 383 insertions(+), 56 deletions(-)
>>  create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.c
>>  create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.exp
>>
>>
>> base-commit: 6d3d6e4ba779dc08b134cd1a09b055dbd88dbf8a
>> --
>> 2.14.5
>>
>
> Because it can affect what kind of other bugfixes and mitigation we do
> and how this patchset is discussed, my first question is: do you suggest
> merging these patches before the GDB 10 branch is created or waiting
> after the branch?

At this point, I think it would be safer to go with Vries' approach
for GDB 10.  Also, I will be out of office the whole of next week,
so I wouldn't be around to handle any fallout.  

Also, I'm going to use a GDB with patch #2 from this series for all
my debugging for a while, see how will it holds up in practice.

Thanks,
Pedro Alves