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

Message ID 20200721153737.11262-3-pedro@palves.net
State New
Headers show
Series
  • Handle "line 0" ranges (PR26243, PR15314, PR15668)
Related show

Commit Message

Pedro Alves July 21, 2020, 3:37 p.m.
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(-)

-- 
2.14.5

Comments

Eli Zaretskii July 21, 2020, 4:40 p.m. | #1
> From: Pedro Alves <pedro@palves.net>

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

Patch

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
+}