public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Fix inline function stepping
@ 2024-02-01 15:21 Toby Lloyd Davies
  2024-02-01 15:21 ` [PATCH 1/3] gdb: Fix missed inline callsite when the frame has changed Toby Lloyd Davies
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Toby Lloyd Davies @ 2024-02-01 15:21 UTC (permalink / raw)
  To: gdb-patches; +Cc: Toby Lloyd Davies

These patches fix various bugs with gdb's handling when stepping into
inline functions.

Toby Lloyd Davies (3):
  gdb: Fix missed inline callsite when the frame has changed
  gdb: Don't stop at non-statement line after stepping into inline
    function
  gdb: Fix 'next' skipping over remainder of inline function

 gdb/gdbthread.h                               |   6 +
 gdb/infcmd.c                                  |   3 +-
 gdb/infrun.c                                  | 121 +++++++------
 .../gdb.dwarf2/dw2-inline-stepping-2.c        |  49 +++++
 .../gdb.dwarf2/dw2-inline-stepping-2.exp      | 127 +++++++++++++
 .../gdb.dwarf2/dw2-inline-stepping-3.c        |  50 ++++++
 .../gdb.dwarf2/dw2-inline-stepping-3.exp      | 153 ++++++++++++++++
 .../gdb.dwarf2/dw2-inline-stepping-4.c        |  59 +++++++
 .../gdb.dwarf2/dw2-inline-stepping-4.exp      | 167 ++++++++++++++++++
 9 files changed, 679 insertions(+), 56 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-2.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-2.exp
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.exp
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-4.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-4.exp

-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] gdb: Fix missed inline callsite when the frame has changed
  2024-02-01 15:21 [PATCH 0/3] Fix inline function stepping Toby Lloyd Davies
@ 2024-02-01 15:21 ` Toby Lloyd Davies
  2024-02-01 15:21 ` [PATCH 2/3] gdb: Don't stop at non-statement line after stepping into inline function Toby Lloyd Davies
  2024-02-01 15:21 ` [PATCH 3/3] gdb: Fix 'next' skipping over remainder of " Toby Lloyd Davies
  2 siblings, 0 replies; 7+ messages in thread
From: Toby Lloyd Davies @ 2024-02-01 15:21 UTC (permalink / raw)
  To: gdb-patches; +Cc: Toby Lloyd Davies

When the program has just returned from a function, gdb
could incorrectly step over an inline callsite. This would occur when
the line from the innermost inline function did not have is_stmt set.

e.g. with the following assembly:

foo ();
    call foo
bar ();
    global_var++; /* start of bar. is_stmt false */

When stepping out of foo, gdb will not stop the program at the callsite
of bar.

This problem was occurring because the first part of logic that checked
for "calls" to inlined functions was guarded by a check that the we were
still stepping through the same function. This meant that it wasn't used
when the frame had changed due to returning from a function. In this
case we would incorrectly use the symtab_and_line from the innermost
inline function which would mean we wouldn't stop if this line didn't
have is_stmt set.

This guard was necessary with the logic for detecting if we had stepped
into another inline function being done later than the check for inline
callsites. This is because otherwise we could decide to stop at an
inline callsite before we detect that we have also stepped into a
different inlined function we should step over. So fix the bug by
swapping the order of the checks.
---
 gdb/infrun.c                                  |  50 +++----
 .../gdb.dwarf2/dw2-inline-stepping-2.c        |  49 +++++++
 .../gdb.dwarf2/dw2-inline-stepping-2.exp      | 127 ++++++++++++++++++
 3 files changed, 202 insertions(+), 24 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-2.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-2.exp

diff --git a/gdb/infrun.c b/gdb/infrun.c
index e99a1a83070..a782c115cad 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8090,12 +8090,31 @@ process_event_stop_test (struct execution_control_state *ecs)
       return;
     }
 
-  /* Look for "calls" to inlined functions, part one.  If the inline
+  /* Look for "calls" to inlined functions, part one.  If we are still
+     in the same real function we were stepping through, but we have
+     to go further up to find the exact frame ID, we are stepping
+     through a more inlined call beyond its call site.  */
+
+  if (get_frame_type (get_current_frame ()) == INLINE_FRAME
+      && (*curr_frame_id != original_frame_id)
+      && stepped_in_from (get_current_frame (),
+			  ecs->event_thread->control.step_frame_id))
+    {
+      infrun_debug_printf ("stepping through inlined function");
+
+      if (ecs->event_thread->control.step_over_calls == STEP_OVER_ALL
+	  || inline_frame_is_marked_for_skip (false, ecs->event_thread))
+	keep_going (ecs);
+      else
+	end_stepping_range (ecs);
+      return;
+    }
+
+  /* Look for "calls" to inlined functions, part two.  If the inline
      frame machinery detected some skipped call sites, we have entered
      a new inline function.  */
 
-  if ((*curr_frame_id == original_frame_id)
-      && inline_skipped_frames (ecs->event_thread))
+  if (inline_skipped_frames (ecs->event_thread))
     {
       infrun_debug_printf ("stepped into inlined function");
 
@@ -8108,7 +8127,8 @@ process_event_stop_test (struct execution_control_state *ecs)
 	     we were previously stepping, go down into the function
 	     first.  Otherwise stop at the call site.  */
 
-	  if (call_sal.line == ecs->event_thread->current_line
+	  if (*curr_frame_id == original_frame_id
+	      && call_sal.line == ecs->event_thread->current_line
 	      && call_sal.symtab == ecs->event_thread->current_symtab)
 	    {
 	      step_into_inline_frame (ecs->event_thread);
@@ -8127,7 +8147,8 @@ process_event_stop_test (struct execution_control_state *ecs)
 	  /* For "next", we should stop at the call site if it is on a
 	     different source line.  Otherwise continue through the
 	     inlined function.  */
-	  if (call_sal.line == ecs->event_thread->current_line
+	  if (*curr_frame_id == original_frame_id
+	      && call_sal.line == ecs->event_thread->current_line
 	      && call_sal.symtab == ecs->event_thread->current_symtab)
 	    keep_going (ecs);
 	  else
@@ -8136,25 +8157,6 @@ process_event_stop_test (struct execution_control_state *ecs)
 	}
     }
 
-  /* Look for "calls" to inlined functions, part two.  If we are still
-     in the same real function we were stepping through, but we have
-     to go further up to find the exact frame ID, we are stepping
-     through a more inlined call beyond its call site.  */
-
-  if (get_frame_type (get_current_frame ()) == INLINE_FRAME
-      && (*curr_frame_id != original_frame_id)
-      && stepped_in_from (get_current_frame (), original_frame_id))
-    {
-      infrun_debug_printf ("stepping through inlined function");
-
-      if (ecs->event_thread->control.step_over_calls == STEP_OVER_ALL
-	  || inline_frame_is_marked_for_skip (false, ecs->event_thread))
-	keep_going (ecs);
-      else
-	end_stepping_range (ecs);
-      return;
-    }
-
   bool refresh_step_info = true;
   if ((ecs->event_thread->stop_pc () == stop_pc_sal.pc)
       && (ecs->event_thread->current_line != stop_pc_sal.line
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-2.c b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-2.c
new file mode 100644
index 00000000000..9c2b3f95a26
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-2.c
@@ -0,0 +1,49 @@
+/* Copyright 2019-2024 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 relies on bar being inlined into main and foo not being
+   inlined. */
+
+volatile int global_var;
+
+static inline int  __attribute__ ((always_inline))
+bar ()
+{
+  asm ("bar_label: .globl bar_label");
+  global_var++;					/* bar inc global_var*/
+  asm ("bar_label2: .globl bar_label2");
+  return global_var;				/* bar return global_var */
+}						/* bar end */
+
+void
+foo (void)
+{						/* foo prologue */
+  asm ("foo_label: .globl foo_label");
+  global_var++;					/* foo inc global_var */
+}						/* foo end */
+
+int
+main ()
+{						/* main prologue */
+  int ans;
+  asm ("main_label: .globl main_label");
+  global_var = 0;				/* main set global_var */
+  asm ("main_label2: .globl main_label2");
+  foo ();                                        /* main call foo */
+  asm ("main_label3: .globl main_label3");
+  ans = bar ();					/* main call bar */
+  asm ("main_label4: .globl main_label4");
+  return ans;
+}						/* main end */
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-2.exp b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-2.exp
new file mode 100644
index 00000000000..a97a1bfc44b
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-2.exp
@@ -0,0 +1,127 @@
+# Copyright 2019-2024 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 checks that we stop at an inline callsite when returning
+# from a non-inlined function. Specifically when the first line of the
+# inline function does not have is_stmt set.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+require dwarf2_support
+
+# The .c files use __attribute__.
+require is_c_compiler_gcc
+
+standard_testfile .c .S
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    global srcdir subdir srcfile srcfile2
+    declare_labels ranges_label lines_label bar_prog
+
+    lassign [function_range main [list ${srcdir}/${subdir}/$srcfile]] \
+	main_start main_len
+    set main_end "$main_start + $main_len"
+    lassign [function_range foo [list ${srcdir}/${subdir}/$srcfile]] \
+	foo_start foo_len
+    set foo_end "$foo_start + $foo_len"
+
+    set bar_call_line [gdb_get_line_number "main call bar"]
+
+    cu {} {
+	compile_unit {
+	    {language @DW_LANG_C}
+	    {name dw2-inline-stepping-2.c}
+	    {low_pc 0 addr}
+	    {stmt_list ${lines_label} DW_FORM_sec_offset}
+	    {ranges ${ranges_label} DW_FORM_sec_offset}
+	} {
+	    subprogram {
+		{external 1 flag}
+		{MACRO_AT_func {foo}}
+	    }
+	    bar_prog: subprogram {
+		{name bar}
+		{inline 3 data1}
+	    }
+	    subprogram {
+		{external 1 flag}
+		{name main}
+		{low_pc $main_start addr}
+		{high_pc "$main_start + $main_len" addr}
+	    } {
+		inlined_subroutine {
+		    {abstract_origin %$bar_prog}
+		    {low_pc main_label3 addr}
+		    {high_pc main_label4 addr}
+		    {call_file 1 data1}
+		    {call_line $bar_call_line data1}
+		}
+	    }
+	}
+    }
+
+    lines {version 2} lines_label {
+	include_dir "${srcdir}/${subdir}"
+	file_name "$srcfile" 1
+
+	program {
+	    DW_LNE_set_address foo_label
+	    line [gdb_get_line_number "foo inc global_var"]
+	    DW_LNS_copy
+	    DW_LNE_set_address $foo_end
+	    DW_LNE_end_sequence
+
+	    DW_LNE_set_address main_label3
+	    line [gdb_get_line_number "bar inc global_var"]
+	    DW_LNS_negate_stmt
+	    DW_LNS_copy
+
+	    DW_LNS_negate_stmt
+	    DW_LNE_set_address bar_label2
+	    line [gdb_get_line_number "bar return global_var"]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address main_label4
+	    line [gdb_get_line_number "return ans"]
+	    DW_LNS_copy
+	    DW_LNE_set_address $main_end
+	    DW_LNE_end_sequence
+	}
+    }
+
+    ranges {is_64 [is_64_target]} {
+	ranges_label: sequence {
+	    range ${main_start} ${main_end}
+	    range ${foo_start} ${foo_end}
+	}
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_breakpoint "foo"
+gdb_continue_to_breakpoint "foo"
+
+# We should stop at bar callsite when returning from foo.
+gdb_test "step" ".*main call bar.*" "step to bar callsite"
-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/3] gdb: Don't stop at non-statement line after stepping into inline function
  2024-02-01 15:21 [PATCH 0/3] Fix inline function stepping Toby Lloyd Davies
  2024-02-01 15:21 ` [PATCH 1/3] gdb: Fix missed inline callsite when the frame has changed Toby Lloyd Davies
@ 2024-02-01 15:21 ` Toby Lloyd Davies
  2024-02-01 15:43   ` [PATCH v2 " Toby Lloyd Davies
  2024-02-01 20:45   ` [PATCH " Carl Love
  2024-02-01 15:21 ` [PATCH 3/3] gdb: Fix 'next' skipping over remainder of " Toby Lloyd Davies
  2 siblings, 2 replies; 7+ messages in thread
From: Toby Lloyd Davies @ 2024-02-01 15:21 UTC (permalink / raw)
  To: gdb-patches; +Cc: Toby Lloyd Davies

Generally when stepping we only want to stop at lines that mark the
beginning of a statement i.e. have is_stmt set. However, when we stepped
into an inline function we would always stop regardless of whether
is_stmt was set on first line of the inline function. Fix this in
infrun.c:process_event_stop_test by not immediately stopping when
entering an inline function. Then code later in the function will decide
whether to stop based on whether is_stmt is set on the line.
Additionally, check if is_stmt is set on the line after stepping from an
inline callsite in infcmd.c:prepare_one_step. This fixes the bug when
the step starts directly at the inline function callsite.
---
 gdb/infcmd.c                                  |   3 +-
 gdb/infrun.c                                  |  32 ++--
 .../gdb.dwarf2/dw2-inline-stepping-3.c        |  50 ++++++
 .../gdb.dwarf2/dw2-inline-stepping-3.exp      | 153 ++++++++++++++++++
 4 files changed, 226 insertions(+), 12 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.exp

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 5e5f75021f2..22ab4dbebbc 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -964,7 +964,8 @@ prepare_one_step (thread_info *tp, struct step_command_fsm *sm)
 		fn = sym->print_name ();
 
 	      if (sal.line == 0
-		  || !function_name_is_marked_for_skip (fn, sal))
+		  || ((inline_skipped_frames (tp) || sal.is_stmt)
+		      && !function_name_is_marked_for_skip (fn, sal)))
 		{
 		  sm->count--;
 		  return prepare_one_step (tp, sm);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index a782c115cad..927e464e479 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8104,10 +8104,10 @@ process_event_stop_test (struct execution_control_state *ecs)
 
       if (ecs->event_thread->control.step_over_calls == STEP_OVER_ALL
 	  || inline_frame_is_marked_for_skip (false, ecs->event_thread))
-	keep_going (ecs);
-      else
-	end_stepping_range (ecs);
-      return;
+	{
+	  keep_going (ecs);
+	  return;
+	}
     }
 
   /* Look for "calls" to inlined functions, part two.  If the inline
@@ -8122,25 +8122,35 @@ process_event_stop_test (struct execution_control_state *ecs)
 
       if (ecs->event_thread->control.step_over_calls != STEP_OVER_ALL)
 	{
-	  /* For "step", we're going to stop.  But if the call site
-	     for this inlined function is on the same source line as
-	     we were previously stepping, go down into the function
-	     first.  Otherwise stop at the call site.  */
+	  /* For "step", if the call site for this inlined function is on the
+	     same source line as we were previously stepping, go down into the
+	     function first before deciding whether to stop.  Otherwise stop at
+	     the call site.  */
 
 	  if (*curr_frame_id == original_frame_id
 	      && call_sal.line == ecs->event_thread->current_line
 	      && call_sal.symtab == ecs->event_thread->current_symtab)
 	    {
 	      step_into_inline_frame (ecs->event_thread);
+	      frame = get_current_frame ();
 	      if (inline_frame_is_marked_for_skip (false, ecs->event_thread))
 		{
 		  keep_going (ecs);
 		  return;
 		}
+	      else if (inline_skipped_frames (ecs->event_thread))
+		{
+		  end_stepping_range (ecs);
+		  return;
+		}
+	      /* We are no longer at an inline function callsite.
+		 We use the checks further down to determine whether to stop. */
+	    }
+	  else
+	    {
+	      end_stepping_range (ecs);
+	      return;
 	    }
-
-	  end_stepping_range (ecs);
-	  return;
 	}
       else
 	{
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c
new file mode 100644
index 00000000000..3b6bc84fb05
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c
@@ -0,0 +1,50 @@
+/* Copyright 2019-2024 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 relies on foo and bar being inlined into main. */
+
+volatile int global_var;
+
+static inline int  __attribute__ ((always_inline))
+foo ()
+{
+  asm ("foo_label: .globl foo_label");
+  global_var++;					/* foo inc global_var*/
+  asm ("foo_label2: .globl foo_label2");
+  return global_var;				/* foo return global_var */
+}						/* foo end */
+
+static inline int  __attribute__ ((always_inline))
+bar ()
+{
+  asm ("bar_label: .globl bar_label");
+  global_var++;					/* bar inc global_var*/
+  asm ("bar_label2: .globl bar_label2");
+  return global_var;				/* bar return global_var */
+}						/* bar end */
+
+int
+main ()
+{						/* main prologue */
+  int ans;
+  asm ("main_label: .globl main_label");
+  global_var = 0;				/* main set global_var */
+  asm ("main_label2: .globl main_label2");
+  ans = foo ();                                 /* main call foo */
+  asm ("main_label3: .globl main_label3");
+  asm ("nop"); ans = bar ();		        /* main call bar */
+  asm ("main_label4: .globl main_label4");
+  return ans;
+}						/* main end */
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.exp b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.exp
new file mode 100644
index 00000000000..ef864d943b3
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.exp
@@ -0,0 +1,153 @@
+# Copyright 2019-2024 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 checks that we stop at an inline callsite when returning
+# from a non-inlined function. Specifically when the first line of the
+# inline function does not have is_stmt set.
+#
+# Additionally, we check that when stepping into the inline function we
+# step past this first line without is_stmt set. We check two cases. One
+# where the inline callsite line contains no instructions (i.e. it
+# begins at the same address that the inline function begins). This
+# exercices the codepath in infcmd.c:prepare_one_step.  The other is
+# when the inline callsite line contains one instruction. This exercises
+# the codepath in infrun.c:prepare_one_step.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+require dwarf2_support
+
+# The .c files use __attribute__.
+require is_c_compiler_gcc
+
+standard_testfile .c .S
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    global srcdir subdir srcfile srcfile2
+    declare_labels ranges_label lines_label foo_prog bar_prog
+
+    lassign [function_range main [list ${srcdir}/${subdir}/$srcfile]] \
+	main_start main_len
+    set main_end "$main_start + $main_len"
+
+    set foo_call_line [gdb_get_line_number "main call foo"]
+    set bar_call_line [gdb_get_line_number "main call bar"]
+
+    cu {} {
+	compile_unit {
+	    {language @DW_LANG_C}
+	    {name dw2-inline-stepping-2.c}
+	    {low_pc 0 addr}
+	    {stmt_list ${lines_label} DW_FORM_sec_offset}
+	    {ranges ${ranges_label} DW_FORM_sec_offset}
+	} {
+	    bar_prog: subprogram {
+		{name bar}
+		{inline 3 data1}
+	    }
+	    foo_prog: subprogram {
+		{name foo}
+		{inline 3 data1}
+	    }
+	    subprogram {
+		{external 1 flag}
+		{name main}
+		{low_pc $main_start addr}
+		{high_pc "$main_start + $main_len" addr}
+	    } {
+		inlined_subroutine {
+		    {abstract_origin %$foo_prog}
+		    {low_pc main_label2 addr}
+		    {high_pc main_label3 addr}
+		    {call_file 1 data1}
+		    {call_line $foo_call_line data1}
+		}
+		inlined_subroutine {
+		    {abstract_origin %$bar_prog}
+		    {low_pc bar_label addr}
+		    {high_pc main_label4 addr}
+		    {call_file 1 data1}
+		    {call_line $bar_call_line data1}
+		}
+	    }
+	}
+    }
+
+    lines {version 2} lines_label {
+	include_dir "${srcdir}/${subdir}"
+	file_name "$srcfile" 1
+
+	program {
+
+	    DW_LNE_set_address main_label
+	    line [gdb_get_line_number "main set global_var"]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address main_label2
+	    DW_LNS_negate_stmt
+	    line [gdb_get_line_number "foo inc global_var"]
+	    DW_LNS_copy
+	    DW_LNS_negate_stmt
+
+	    DW_LNE_set_address foo_label2
+	    line [gdb_get_line_number "foo return global_var"]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address main_label3
+	    line [gdb_get_line_number "main call bar"]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address bar_label
+	    DW_LNS_negate_stmt
+	    line [gdb_get_line_number "bar inc global_var"]
+	    DW_LNS_copy
+	    DW_LNS_negate_stmt
+
+	    DW_LNE_set_address bar_label2
+	    line [gdb_get_line_number "bar return global_var"]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address main_label4
+	    line [gdb_get_line_number "return ans"]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address $main_end
+	    DW_LNE_end_sequence
+	}
+    }
+
+    ranges {is_64 [is_64_target]} {
+	ranges_label: sequence {
+	    range ${main_start} ${main_end}
+	}
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_test "next" ".* main call foo.*" "step to foo callsite"
+gdb_test "step" ".* foo return global_var.*" "step to foo inc global_var"
+
+gdb_test "next" ".*main call bar.*" "step to bar callsite"
+gdb_test "step" ".* bar return global_var.*" "step to bar inc global_var"
-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/3] gdb: Fix 'next' skipping over remainder of inline function
  2024-02-01 15:21 [PATCH 0/3] Fix inline function stepping Toby Lloyd Davies
  2024-02-01 15:21 ` [PATCH 1/3] gdb: Fix missed inline callsite when the frame has changed Toby Lloyd Davies
  2024-02-01 15:21 ` [PATCH 2/3] gdb: Don't stop at non-statement line after stepping into inline function Toby Lloyd Davies
@ 2024-02-01 15:21 ` Toby Lloyd Davies
  2 siblings, 0 replies; 7+ messages in thread
From: Toby Lloyd Davies @ 2024-02-01 15:21 UTC (permalink / raw)
  To: gdb-patches; +Cc: Toby Lloyd Davies

An inline function's instructions are often interleaved with it's
callers. When stepping out of the inline function and then back into it
again we would treat this as stepping into a new inline function. Thus a
'next' command from within a inline function could skip over the rest of
the inline function.

This occurred because the step_frame_id was refreshed at the end of
process_event_stop_test when it detected that we had changed frame. Fix
this by refreshing step info only when the stack frame has changed.
However, not refreshing the step info when we step out of an inline
frame poses a problem if we subsequently step into a different inline
frame. The code wouldn't be able to determine whether we had stepped
into or out to this inline function, and thus whether a 'next' command
should skip it. If that inline function was in the backtrace when we
started stepping then we don't want to skip it but if it was not in the
backtrace then we want to skip it.

Therefore, to correctly determine when we have stepped into an inline frame
requires knowledge of every inline frame that was being stepped through
when the command was issued. So store these inline frame ids in a
vector. Then we can treat us as having stepped into an inline function
if we can't find the current frame in the vector and we haven't stepped
into a different stack frame.

Additionally, this fixes another bug. When stepping out of an inline
function and immediately entering into another inline function we would
not detect this as stepping into an inline function and thus a 'next'
command wouldn't step over the new inline function. Note that in this
case we would normally detect this as the entry point to the inline
function and create a inline callsite here, but if we didn't detect this
as an entry point then we would incorrectly step into the function.
---
 gdb/gdbthread.h                               |   6 +
 gdb/infrun.c                                  |  57 +++---
 .../gdb.dwarf2/dw2-inline-stepping-4.c        |  59 +++++++
 .../gdb.dwarf2/dw2-inline-stepping-4.exp      | 167 ++++++++++++++++++
 4 files changed, 260 insertions(+), 29 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-4.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-4.exp

diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index e7035d40ad4..ddfd4ecc965 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -146,6 +146,12 @@ struct thread_control_state
      any inlined frames).  */
   struct frame_id step_stack_frame_id {};
 
+  /* Frame IDs of inline frames as of when stepping command was issued.
+     From newest inline frame until newest non-inline frame
+     (exclusive). This is how we know when we step into an inlined
+     subroutine call.  */
+  std::vector<struct frame_id> step_inline_frame_ids {};
+
   /* True if the the thread is presently stepping over a breakpoint or
      a watchpoint, either with an inline step over or a displaced (out
      of line) step, and we're now expecting it to report a trap for
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 927e464e479..845341b8fff 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4780,6 +4780,13 @@ set_step_info (thread_info *tp, frame_info_ptr frame,
 
   tp->control.step_frame_id = get_frame_id (frame);
   tp->control.step_stack_frame_id = get_stack_frame_id (frame);
+  tp->control.step_inline_frame_ids.clear ();
+  for (frame_info_ptr f = frame;
+       f && get_frame_type (f) == INLINE_FRAME;
+       f = get_prev_frame (f))
+    {
+      tp->control.step_inline_frame_ids.push_back (get_frame_id (f));
+    }
 
   tp->current_symtab = sal.symtab;
   tp->current_line = sal.line;
@@ -5002,23 +5009,6 @@ adjust_pc_after_break (struct thread_info *thread,
     }
 }
 
-static bool
-stepped_in_from (frame_info_ptr frame, struct frame_id step_frame_id)
-{
-  for (frame = get_prev_frame (frame);
-       frame != nullptr;
-       frame = get_prev_frame (frame))
-    {
-      if (get_frame_id (frame) == step_frame_id)
-	return true;
-
-      if (get_frame_type (frame) != INLINE_FRAME)
-	break;
-    }
-
-  return false;
-}
-
 /* Look for an inline frame that is marked for skip.
    If PREV_FRAME is TRUE start at the previous frame,
    otherwise start at the current frame.  Stop at the
@@ -5039,11 +5029,16 @@ inline_frame_is_marked_for_skip (bool prev_frame, struct thread_info *tp)
       symtab_and_line sal;
       struct symbol *sym;
 
-      if (get_frame_id (frame) == tp->control.step_frame_id)
-	break;
       if (get_frame_type (frame) != INLINE_FRAME)
 	break;
 
+      /* Don't skip frames we were already stepping through. */
+      if (std::find (tp->control.step_inline_frame_ids.begin (),
+		     tp->control.step_inline_frame_ids.end (),
+		     get_frame_id (frame))
+	  != tp->control.step_inline_frame_ids.end ())
+	break;
+
       sal = find_frame_sal (frame);
       sym = get_frame_function (frame);
 
@@ -8091,14 +8086,17 @@ process_event_stop_test (struct execution_control_state *ecs)
     }
 
   /* Look for "calls" to inlined functions, part one.  If we are still
-     in the same real function we were stepping through, but we have
-     to go further up to find the exact frame ID, we are stepping
+     in the same real function we were stepping through, but we were not
+     previously stepping through this inline function, we are stepping
      through a more inlined call beyond its call site.  */
 
   if (get_frame_type (get_current_frame ()) == INLINE_FRAME
-      && (*curr_frame_id != original_frame_id)
-      && stepped_in_from (get_current_frame (),
-			  ecs->event_thread->control.step_frame_id))
+      && (std::find (ecs->event_thread->control.step_inline_frame_ids.begin (),
+		    ecs->event_thread->control.step_inline_frame_ids.end (),
+		    *curr_frame_id)
+	   == ecs->event_thread->control.step_inline_frame_ids.end ())
+      && (get_stack_frame_id (get_current_frame ())
+	  == ecs->event_thread->control.step_stack_frame_id))
     {
       infrun_debug_printf ("stepping through inlined function");
 
@@ -8208,10 +8206,11 @@ process_event_stop_test (struct execution_control_state *ecs)
 	  end_stepping_range (ecs);
 	  return;
 	}
-      else if (*curr_frame_id == original_frame_id)
+      else if (get_stack_frame_id (get_current_frame ())
+	       == ecs->event_thread->control.step_stack_frame_id)
 	{
 	  /* We are not at the start of a statement, and we have not changed
-	     frame.
+	     stack frame.
 
 	     We ignore this line table entry, and continue stepping forward,
 	     looking for a better place to stop.  */
@@ -8221,15 +8220,15 @@ process_event_stop_test (struct execution_control_state *ecs)
 	}
       else
 	{
-	  /* We are not the start of a statement, and we have changed frame.
+	  /* We are not the start of a statement, and we have changed stack frame.
 
 	     We ignore this line table entry, and continue stepping forward,
 	     looking for a better place to stop.  Keep refresh_step_info at
-	     true to note that the frame has changed, but ignore the line
+	     true to note that the stack frame has changed, but ignore the line
 	     number to make sure we don't ignore a subsequent entry with the
 	     same line number.  */
 	  stop_pc_sal.line = 0;
-	  infrun_debug_printf ("stepped to a different frame, but "
+	  infrun_debug_printf ("stepped to a different stack frame, but "
 			       "it's not the start of a statement");
 	}
     }
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-4.c b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-4.c
new file mode 100644
index 00000000000..3097fed5131
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-4.c
@@ -0,0 +1,59 @@
+/* Copyright 2019-2024 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the FOOU 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
+   FOOU General Public License for more details.
+
+   You should have received a copy of the FOOU General Public License
+   along with this program.  If not, see <http://www.foou.org/licenses/>.  */
+
+/* This test relies on foo_a, foo_b and bar being inlined into main */
+
+volatile int global_var;
+
+static inline void  __attribute__ ((always_inline))
+foo_a ()
+{
+  asm ("foo_label: .globl foo_label");
+  global_var++;					/* foo inc global_var */
+}
+
+static inline void  __attribute__ ((always_inline))
+foo_b ()
+{
+  asm ("foo_label2: .globl foo_label2");
+  asm ("nop");                                  /* foo nop */
+  asm ("foo_label3: .globl foo_label3");
+  asm ("jmp bar_label2");                       /* foo jmp bar */
+}						/* foo end */
+
+static inline void  __attribute__ ((always_inline))
+bar ()
+{
+  asm ("bar_label: .globl bar_label");
+  asm ("nop");                                  /* bar nop */
+  asm ("bar_label2: .globl bar_label2");
+  global_var++;					/* bar inc global_var */
+}						/* bar end */
+
+int
+main ()
+{						/* main prologue */
+  asm ("main_label: .globl main_label");
+  global_var = 0;				/* main set global_var */
+  asm ("main_label2: .globl main_label2");
+  foo_a ();			                /* main call foo */
+  asm ("main_label3: .globl main_label3");
+  asm ("jmp foo_label3");                       /* main jmp foo */
+  foo_b ();
+  asm ("main_label4: .globl main_label4");
+  bar();                                        /* main call bar */
+  asm ("main_label5: .globl main_label5");
+  return global_var;                            /* main return global_var */
+}						/* main end */
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-4.exp b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-4.exp
new file mode 100644
index 00000000000..6abba731c88
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-4.exp
@@ -0,0 +1,167 @@
+# Copyright 2019-2024 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/>.
+#
+# Inline functions often have instructions interleaved with their
+# callers. Test that when the 'next' command steps out of an inline
+# function and then back into it again we do not skip the rest of the
+# inline function.
+#
+# Additionally, test that when the 'next' command steps out of one
+# inline function and immediately enters another inline function
+# (without going to the callsite), we skip over the inline function.
+#
+# The testfile uses jumps into inline functions so that we do not stop
+# at the inline callsite lines.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+require dwarf2_support
+
+# The .c files use __attribute__.
+require is_c_compiler_gcc
+
+standard_testfile .c .S
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    global srcdir subdir srcfile srcfile2
+    declare_labels cu_ranges_label foo_ranges_label lines_label bar_prog foo_prog
+
+    lassign [function_range main [list ${srcdir}/${subdir}/$srcfile]] \
+	main_start main_len
+    set main_end "$main_start + $main_len"
+
+    set bar_call_line [gdb_get_line_number "main call bar"]
+    set foo_call_line [gdb_get_line_number "main call foo"]
+
+    cu {} {
+	compile_unit {
+	    {language @DW_LANG_C}
+	    {name dw2-inline-stepping-2.c}
+	    {low_pc 0 addr}
+	    {stmt_list ${lines_label} DW_FORM_sec_offset}
+	    {ranges ${cu_ranges_label} DW_FORM_sec_offset}
+	} {
+	    bar_prog: subprogram {
+		{name bar}
+		{inline 3 data1}
+	    }
+	    foo_prog: subprogram {
+		{name bar}
+		{inline 3 data1}
+	    }
+	    subprogram {
+		{external 1 flag}
+		{name main}
+		{low_pc $main_start addr}
+		{high_pc "$main_start + $main_len" addr}
+	    } {
+		inlined_subroutine {
+		    {abstract_origin %$foo_prog}
+			{ranges ${foo_ranges_label} DW_FORM_sec_offset}
+		    {call_file 1 data1}
+		    {call_line $foo_call_line data1}
+		}
+		inlined_subroutine {
+		    {abstract_origin %$bar_prog}
+		    {low_pc bar_label addr}
+		    {high_pc main_label5 addr}
+		    {call_file 1 data1}
+		    {call_line $bar_call_line data1}
+		}
+	    }
+	}
+    }
+
+    lines {version 2} lines_label {
+	include_dir "${srcdir}/${subdir}"
+	file_name "$srcfile" 1
+
+	program {
+
+	    DW_LNE_set_address main_label
+	    line [gdb_get_line_number "main set global_var"]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address main_label2
+	    line [gdb_get_line_number "main call foo"]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address foo_label
+	    line [gdb_get_line_number "foo inc global_var"]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address main_label3
+	    line [gdb_get_line_number "main jmp foo"]
+	    DW_LNS_negate_stmt
+		DW_LNS_copy
+	    DW_LNS_negate_stmt
+
+
+	    DW_LNE_set_address foo_label2
+	    line [gdb_get_line_number "bar nop"]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address foo_label3
+	    line [gdb_get_line_number "foo jmp bar"]
+	    DW_LNS_copy
+
+		DW_LNE_set_address main_label4
+	    line [gdb_get_line_number "main call bar"]
+	    DW_LNS_copy
+
+		DW_LNE_set_address bar_label
+	    line [gdb_get_line_number "bar nop"]
+	    DW_LNS_copy
+
+		DW_LNE_set_address bar_label2
+	    line [gdb_get_line_number "bar inc global_var"]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address main_label5
+	    line [gdb_get_line_number "main return global_var"]
+	    DW_LNS_copy
+	    DW_LNE_set_address $main_end
+	    DW_LNE_end_sequence
+	}
+    }
+
+    ranges {is_64 [is_64_target]} {
+	cu_ranges_label: sequence {
+	    range ${main_start} ${main_end}
+	}
+	foo_ranges_label: sequence {
+		range {foo_label} {main_label3}
+		range {foo_label2} {main_label4}
+	}
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_test "step" ".*main call foo.*" "step to foo callsite"
+gdb_test "step" ".*foo inc global_var.*" "step into foo"
+
+gdb_test "next" ".foo jmp bar.*" "next within foo"
+
+gdb_test "next" ".*return global_var.*" "next over bar"
-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 2/3] gdb: Don't stop at non-statement line after stepping into inline function
  2024-02-01 15:21 ` [PATCH 2/3] gdb: Don't stop at non-statement line after stepping into inline function Toby Lloyd Davies
@ 2024-02-01 15:43   ` Toby Lloyd Davies
  2024-02-01 20:45   ` [PATCH " Carl Love
  1 sibling, 0 replies; 7+ messages in thread
From: Toby Lloyd Davies @ 2024-02-01 15:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Toby Lloyd Davies

Generally when stepping we only want to stop at lines that mark the
beginning of a statement i.e. have is_stmt set. However, when we stepped
into an inline function we would always stop regardless of whether
is_stmt was set on first line of the inline function. Fix this in
infrun.c:process_event_stop_test by not immediately stopping when
entering an inline function. Then code later in the function will decide
whether to stop based on whether is_stmt is set on the line.
Additionally, check if is_stmt is set on the line after stepping from an
inline callsite in infcmd.c:prepare_one_step. This fixes the bug when
the step starts directly at the inline function callsite.
---
 gdb/infcmd.c                                  |   3 +-
 gdb/infrun.c                                  |  32 ++--
 .../gdb.dwarf2/dw2-inline-stepping-3.c        |  50 ++++++
 .../gdb.dwarf2/dw2-inline-stepping-3.exp      | 149 ++++++++++++++++++
 4 files changed, 222 insertions(+), 12 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.exp

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 5e5f75021f2..22ab4dbebbc 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -964,7 +964,8 @@ prepare_one_step (thread_info *tp, struct step_command_fsm *sm)
 		fn = sym->print_name ();
 
 	      if (sal.line == 0
-		  || !function_name_is_marked_for_skip (fn, sal))
+		  || ((inline_skipped_frames (tp) || sal.is_stmt)
+		      && !function_name_is_marked_for_skip (fn, sal)))
 		{
 		  sm->count--;
 		  return prepare_one_step (tp, sm);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index a782c115cad..927e464e479 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8104,10 +8104,10 @@ process_event_stop_test (struct execution_control_state *ecs)
 
       if (ecs->event_thread->control.step_over_calls == STEP_OVER_ALL
 	  || inline_frame_is_marked_for_skip (false, ecs->event_thread))
-	keep_going (ecs);
-      else
-	end_stepping_range (ecs);
-      return;
+	{
+	  keep_going (ecs);
+	  return;
+	}
     }
 
   /* Look for "calls" to inlined functions, part two.  If the inline
@@ -8122,25 +8122,35 @@ process_event_stop_test (struct execution_control_state *ecs)
 
       if (ecs->event_thread->control.step_over_calls != STEP_OVER_ALL)
 	{
-	  /* For "step", we're going to stop.  But if the call site
-	     for this inlined function is on the same source line as
-	     we were previously stepping, go down into the function
-	     first.  Otherwise stop at the call site.  */
+	  /* For "step", if the call site for this inlined function is on the
+	     same source line as we were previously stepping, go down into the
+	     function first before deciding whether to stop.  Otherwise stop at
+	     the call site.  */
 
 	  if (*curr_frame_id == original_frame_id
 	      && call_sal.line == ecs->event_thread->current_line
 	      && call_sal.symtab == ecs->event_thread->current_symtab)
 	    {
 	      step_into_inline_frame (ecs->event_thread);
+	      frame = get_current_frame ();
 	      if (inline_frame_is_marked_for_skip (false, ecs->event_thread))
 		{
 		  keep_going (ecs);
 		  return;
 		}
+	      else if (inline_skipped_frames (ecs->event_thread))
+		{
+		  end_stepping_range (ecs);
+		  return;
+		}
+	      /* We are no longer at an inline function callsite.
+		 We use the checks further down to determine whether to stop. */
+	    }
+	  else
+	    {
+	      end_stepping_range (ecs);
+	      return;
 	    }
-
-	  end_stepping_range (ecs);
-	  return;
 	}
       else
 	{
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c
new file mode 100644
index 00000000000..3b6bc84fb05
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c
@@ -0,0 +1,50 @@
+/* Copyright 2019-2024 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 relies on foo and bar being inlined into main. */
+
+volatile int global_var;
+
+static inline int  __attribute__ ((always_inline))
+foo ()
+{
+  asm ("foo_label: .globl foo_label");
+  global_var++;					/* foo inc global_var*/
+  asm ("foo_label2: .globl foo_label2");
+  return global_var;				/* foo return global_var */
+}						/* foo end */
+
+static inline int  __attribute__ ((always_inline))
+bar ()
+{
+  asm ("bar_label: .globl bar_label");
+  global_var++;					/* bar inc global_var*/
+  asm ("bar_label2: .globl bar_label2");
+  return global_var;				/* bar return global_var */
+}						/* bar end */
+
+int
+main ()
+{						/* main prologue */
+  int ans;
+  asm ("main_label: .globl main_label");
+  global_var = 0;				/* main set global_var */
+  asm ("main_label2: .globl main_label2");
+  ans = foo ();                                 /* main call foo */
+  asm ("main_label3: .globl main_label3");
+  asm ("nop"); ans = bar ();		        /* main call bar */
+  asm ("main_label4: .globl main_label4");
+  return ans;
+}						/* main end */
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.exp b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.exp
new file mode 100644
index 00000000000..638c2635d5f
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.exp
@@ -0,0 +1,149 @@
+# Copyright 2019-2024 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 checks that when stepping into an inline function we step past the
+# first line if it does not have is_stmt set. We check two cases. One where the
+# inline callsite line contains no instructions (i.e. it begins at the same
+# address that the inline function begins). This exercices the codepath in
+# infcmd.c:prepare_one_step.  The other is when the inline callsite line
+# contains one instruction. This exercises the codepath in
+# infrun.c:prepare_one_step.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+require dwarf2_support
+
+# The .c files use __attribute__.
+require is_c_compiler_gcc
+
+standard_testfile .c .S
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    global srcdir subdir srcfile srcfile2
+    declare_labels ranges_label lines_label foo_prog bar_prog
+
+    lassign [function_range main [list ${srcdir}/${subdir}/$srcfile]] \
+	main_start main_len
+    set main_end "$main_start + $main_len"
+
+    set foo_call_line [gdb_get_line_number "main call foo"]
+    set bar_call_line [gdb_get_line_number "main call bar"]
+
+    cu {} {
+	compile_unit {
+	    {language @DW_LANG_C}
+	    {name dw2-inline-stepping-2.c}
+	    {low_pc 0 addr}
+	    {stmt_list ${lines_label} DW_FORM_sec_offset}
+	    {ranges ${ranges_label} DW_FORM_sec_offset}
+	} {
+	    bar_prog: subprogram {
+		{name bar}
+		{inline 3 data1}
+	    }
+	    foo_prog: subprogram {
+		{name foo}
+		{inline 3 data1}
+	    }
+	    subprogram {
+		{external 1 flag}
+		{name main}
+		{low_pc $main_start addr}
+		{high_pc "$main_start + $main_len" addr}
+	    } {
+		inlined_subroutine {
+		    {abstract_origin %$foo_prog}
+		    {low_pc main_label2 addr}
+		    {high_pc main_label3 addr}
+		    {call_file 1 data1}
+		    {call_line $foo_call_line data1}
+		}
+		inlined_subroutine {
+		    {abstract_origin %$bar_prog}
+		    {low_pc bar_label addr}
+		    {high_pc main_label4 addr}
+		    {call_file 1 data1}
+		    {call_line $bar_call_line data1}
+		}
+	    }
+	}
+    }
+
+    lines {version 2} lines_label {
+	include_dir "${srcdir}/${subdir}"
+	file_name "$srcfile" 1
+
+	program {
+
+	    DW_LNE_set_address main_label
+	    line [gdb_get_line_number "main set global_var"]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address main_label2
+	    DW_LNS_negate_stmt
+	    line [gdb_get_line_number "foo inc global_var"]
+	    DW_LNS_copy
+	    DW_LNS_negate_stmt
+
+	    DW_LNE_set_address foo_label2
+	    line [gdb_get_line_number "foo return global_var"]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address main_label3
+	    line [gdb_get_line_number "main call bar"]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address bar_label
+	    DW_LNS_negate_stmt
+	    line [gdb_get_line_number "bar inc global_var"]
+	    DW_LNS_copy
+	    DW_LNS_negate_stmt
+
+	    DW_LNE_set_address bar_label2
+	    line [gdb_get_line_number "bar return global_var"]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address main_label4
+	    line [gdb_get_line_number "return ans"]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address $main_end
+	    DW_LNE_end_sequence
+	}
+    }
+
+    ranges {is_64 [is_64_target]} {
+	ranges_label: sequence {
+	    range ${main_start} ${main_end}
+	}
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_test "next" ".* main call foo.*" "step to foo callsite"
+gdb_test "step" ".* foo return global_var.*" "step to foo inc global_var"
+
+gdb_test "next" ".*main call bar.*" "step to bar callsite"
+gdb_test "step" ".* bar return global_var.*" "step to bar inc global_var"
-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] gdb: Don't stop at non-statement line after stepping into inline function
  2024-02-01 15:21 ` [PATCH 2/3] gdb: Don't stop at non-statement line after stepping into inline function Toby Lloyd Davies
  2024-02-01 15:43   ` [PATCH v2 " Toby Lloyd Davies
@ 2024-02-01 20:45   ` Carl Love
  2024-02-01 21:35     ` Carl Love
  1 sibling, 1 reply; 7+ messages in thread
From: Carl Love @ 2024-02-01 20:45 UTC (permalink / raw)
  To: gdb-patches, Carl Love

Toby:

A couple of things.  

First, I tried the series of patches on Power 10. I am getting 
failures for test gdb.dwarf2/dw2-inline-stepping-3.exp on Power. I attached the log
file at the bottom of this message.  I haven't had time yet to dig into the
failures.  I will take a look.

Second, see below, there is a line where you define frame but I don't see
any use of it?  Am I missing something?

On 2/1/24 07:21, Toby Lloyd Davies wrote:
> Generally when stepping we only want to stop at lines that mark the
> beginning of a statement i.e. have is_stmt set. However, when we stepped
> into an inline function we would always stop regardless of whether
> is_stmt was set on first line of the inline function. Fix this in
> infrun.c:process_event_stop_test by not immediately stopping when
> entering an inline function. Then code later in the function will decide
> whether to stop based on whether is_stmt is set on the line.
> Additionally, check if is_stmt is set on the line after stepping from an
> inline callsite in infcmd.c:prepare_one_step. This fixes the bug when
> the step starts directly at the inline function callsite.
> ---
>  gdb/infcmd.c                                  |   3 +-
>  gdb/infrun.c                                  |  32 ++--
>  .../gdb.dwarf2/dw2-inline-stepping-3.c        |  50 ++++++
>  .../gdb.dwarf2/dw2-inline-stepping-3.exp      | 153 ++++++++++++++++++
>  4 files changed, 226 insertions(+), 12 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c
>  create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.exp
> 
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 5e5f75021f2..22ab4dbebbc 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -964,7 +964,8 @@ prepare_one_step (thread_info *tp, struct step_command_fsm *sm)
>  		fn = sym->print_name ();
>  
>  	      if (sal.line == 0
> -		  || !function_name_is_marked_for_skip (fn, sal))
> +		  || ((inline_skipped_frames (tp) || sal.is_stmt)
> +		      && !function_name_is_marked_for_skip (fn, sal)))
>  		{
>  		  sm->count--;
>  		  return prepare_one_step (tp, sm);
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index a782c115cad..927e464e479 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -8104,10 +8104,10 @@ process_event_stop_test (struct execution_control_state *ecs)
>  
>        if (ecs->event_thread->control.step_over_calls == STEP_OVER_ALL
>  	  || inline_frame_is_marked_for_skip (false, ecs->event_thread))
> -	keep_going (ecs);
> -      else
> -	end_stepping_range (ecs);
> -      return;
> +	{
> +	  keep_going (ecs);
> +	  return;
> +	}
>      }
>  
>    /* Look for "calls" to inlined functions, part two.  If the inline
> @@ -8122,25 +8122,35 @@ process_event_stop_test (struct execution_control_state *ecs)
>  
>        if (ecs->event_thread->control.step_over_calls != STEP_OVER_ALL)
>  	{
> -	  /* For "step", we're going to stop.  But if the call site
> -	     for this inlined function is on the same source line as
> -	     we were previously stepping, go down into the function
> -	     first.  Otherwise stop at the call site.  */
> +	  /* For "step", if the call site for this inlined function is on the
> +	     same source line as we were previously stepping, go down into the
> +	     function first before deciding whether to stop.  Otherwise stop at
> +	     the call site.  */
>  
>  	  if (*curr_frame_id == original_frame_id
>  	      && call_sal.line == ecs->event_thread->current_line
>  	      && call_sal.symtab == ecs->event_thread->current_symtab)
>  	    {
>  	      step_into_inline_frame (ecs->event_thread);
> +	      frame = get_current_frame ();

Here, I don't see any use of frame.  I tried commenting it out and the patch
still seems to compile ok?

>  	      if (inline_frame_is_marked_for_skip (false, ecs->event_thread))
>  		{
>  		  keep_going (ecs);
>  		  return;
>  		}
> +	      else if (inline_skipped_frames (ecs->event_thread))
> +		{
> +		  end_stepping_range (ecs);
> +		  return;
> +		}
> +	      /* We are no longer at an inline function callsite.
> +		 We use the checks further down to determine whether to stop. */
> +	    }
> +	  else
> +	    {
> +	      end_stepping_range (ecs);
> +	      return;
>  	    }
> -
> -	  end_stepping_range (ecs);
> -	  return;
>  	}
>        else
>  	{
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c
> new file mode 100644
> index 00000000000..3b6bc84fb05
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c
> @@ -0,0 +1,50 @@
> +/* Copyright 2019-2024 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 relies on foo and bar being inlined into main. */
> +
> +volatile int global_var;
> +
> +static inline int  __attribute__ ((always_inline))
> +foo ()
> +{
> +  asm ("foo_label: .globl foo_label");
> +  global_var++;					/* foo inc global_var*/
> +  asm ("foo_label2: .globl foo_label2");
> +  return global_var;				/* foo return global_var */
> +}						/* foo end */
> +
> +static inline int  __attribute__ ((always_inline))
> +bar ()
> +{
> +  asm ("bar_label: .globl bar_label");
> +  global_var++;					/* bar inc global_var*/
> +  asm ("bar_label2: .globl bar_label2");
> +  return global_var;				/* bar return global_var */
> +}						/* bar end */
> +
> +int
> +main ()
> +{						/* main prologue */
> +  int ans;
> +  asm ("main_label: .globl main_label");
> +  global_var = 0;				/* main set global_var */
> +  asm ("main_label2: .globl main_label2");
> +  ans = foo ();                                 /* main call foo */
> +  asm ("main_label3: .globl main_label3");
> +  asm ("nop"); ans = bar ();		        /* main call bar */
> +  asm ("main_label4: .globl main_label4");
> +  return ans;
> +}						/* main end */
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.exp b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.exp
> new file mode 100644
> index 00000000000..ef864d943b3
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.exp
> @@ -0,0 +1,153 @@
> +# Copyright 2019-2024 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 checks that we stop at an inline callsite when returning
> +# from a non-inlined function. Specifically when the first line of the
> +# inline function does not have is_stmt set.
> +#
> +# Additionally, we check that when stepping into the inline function we
> +# step past this first line without is_stmt set. We check two cases. One
> +# where the inline callsite line contains no instructions (i.e. it
> +# begins at the same address that the inline function begins). This
> +# exercices the codepath in infcmd.c:prepare_one_step.  The other is
> +# when the inline callsite line contains one instruction. This exercises
> +# the codepath in infrun.c:prepare_one_step.
> +
> +load_lib dwarf.exp
> +
> +# This test can only be run on targets which support DWARF-2 and use gas.
> +require dwarf2_support
> +
> +# The .c files use __attribute__.
> +require is_c_compiler_gcc
> +
> +standard_testfile .c .S
> +
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> +    global srcdir subdir srcfile srcfile2
> +    declare_labels ranges_label lines_label foo_prog bar_prog
> +
> +    lassign [function_range main [list ${srcdir}/${subdir}/$srcfile]] \
> +	main_start main_len
> +    set main_end "$main_start + $main_len"
> +
> +    set foo_call_line [gdb_get_line_number "main call foo"]
> +    set bar_call_line [gdb_get_line_number "main call bar"]
> +
> +    cu {} {
> +	compile_unit {
> +	    {language @DW_LANG_C}
> +	    {name dw2-inline-stepping-2.c}
> +	    {low_pc 0 addr}
> +	    {stmt_list ${lines_label} DW_FORM_sec_offset}
> +	    {ranges ${ranges_label} DW_FORM_sec_offset}
> +	} {
> +	    bar_prog: subprogram {
> +		{name bar}
> +		{inline 3 data1}
> +	    }
> +	    foo_prog: subprogram {
> +		{name foo}
> +		{inline 3 data1}
> +	    }
> +	    subprogram {
> +		{external 1 flag}
> +		{name main}
> +		{low_pc $main_start addr}
> +		{high_pc "$main_start + $main_len" addr}
> +	    } {
> +		inlined_subroutine {
> +		    {abstract_origin %$foo_prog}
> +		    {low_pc main_label2 addr}
> +		    {high_pc main_label3 addr}
> +		    {call_file 1 data1}
> +		    {call_line $foo_call_line data1}
> +		}
> +		inlined_subroutine {
> +		    {abstract_origin %$bar_prog}
> +		    {low_pc bar_label addr}
> +		    {high_pc main_label4 addr}
> +		    {call_file 1 data1}
> +		    {call_line $bar_call_line data1}
> +		}
> +	    }
> +	}
> +    }
> +
> +    lines {version 2} lines_label {
> +	include_dir "${srcdir}/${subdir}"
> +	file_name "$srcfile" 1
> +
> +	program {
> +
> +	    DW_LNE_set_address main_label
> +	    line [gdb_get_line_number "main set global_var"]
> +	    DW_LNS_copy
> +
> +	    DW_LNE_set_address main_label2
> +	    DW_LNS_negate_stmt
> +	    line [gdb_get_line_number "foo inc global_var"]
> +	    DW_LNS_copy
> +	    DW_LNS_negate_stmt
> +
> +	    DW_LNE_set_address foo_label2
> +	    line [gdb_get_line_number "foo return global_var"]
> +	    DW_LNS_copy
> +
> +	    DW_LNE_set_address main_label3
> +	    line [gdb_get_line_number "main call bar"]
> +	    DW_LNS_copy
> +
> +	    DW_LNE_set_address bar_label
> +	    DW_LNS_negate_stmt
> +	    line [gdb_get_line_number "bar inc global_var"]
> +	    DW_LNS_copy
> +	    DW_LNS_negate_stmt
> +
> +	    DW_LNE_set_address bar_label2
> +	    line [gdb_get_line_number "bar return global_var"]
> +	    DW_LNS_copy
> +
> +	    DW_LNE_set_address main_label4
> +	    line [gdb_get_line_number "return ans"]
> +	    DW_LNS_copy
> +
> +	    DW_LNE_set_address $main_end
> +	    DW_LNE_end_sequence
> +	}
> +    }
> +
> +    ranges {is_64 [is_64_target]} {
> +	ranges_label: sequence {
> +	    range ${main_start} ${main_end}
> +	}
> +    }
> +}
> +
> +if { [prepare_for_testing "failed to prepare" ${testfile} \
> +	  [list $srcfile $asm_file] {nodebug}] } {
> +    return -1
> +}
> +
> +if ![runto_main] {
> +    return -1
> +}
> +
> +gdb_test "next" ".* main call foo.*" "step to foo callsite"
> +gdb_test "step" ".* foo return global_var.*" "step to foo inc global_var"
> +
> +gdb_test "next" ".*main call bar.*" "step to bar callsite"
> +gdb_test "step" ".* bar return global_var.*" "step to bar inc global_var"
------------------------------------------------------------------------

Here is the gdb log file from the test failures on Power 10

Test run by carll on Thu Feb  1 15:24:46 2024
Native configuration is powerpc64le-unknown-linux-gnu

		=== gdb tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/carll/GDB/build-next/gdb/testsuite/../../../binutils-next-gdb/gdb/testsuite/config/unix.exp as tool-and-target-specific interface file.
Running /home/carll/GDB/build-next/gdb/testsuite/../../../binutils-next-gdb/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.exp ...
Executing on build: rm -rf /home/carll/GDB/build-next/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-stepping-3    (timeout = 300)
builtin_spawn -ignore SIGHUP rm -rf /home/carll/GDB/build-next/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-stepping-3
get_compiler_info: gcc-13-2-1
Executing on host: gcc  -fno-stack-protector  -fdiagnostics-color=never -c  -o /home/carll/GDB/build-next/gdb/testsuite/temp/3439835/is_64_target.o /home/carll/GDB/build-next/gdb/testsuite/temp/3439835/is_64_target.c    (timeout = 300)
builtin_spawn -ignore SIGHUP gcc -fno-stack-protector -fdiagnostics-color=never -c -o /home/carll/GDB/build-next/gdb/testsuite/temp/3439835/is_64_target.o /home/carll/GDB/build-next/gdb/testsuite/temp/3439835/is_64_target.c
Executing on host: gcc  -fno-stack-protector /home/carll/GDB/build-next/gdb/testsuite/../../../binutils-next-gdb/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c  -fdiagnostics-color=never -g  -lm   -o /home/carll/GDB/build-next/gdb/testsuite/temp/3439835/func_addr.x    (timeout = 300)
builtin_spawn -ignore SIGHUP gcc -fno-stack-protector /home/carll/GDB/build-next/gdb/testsuite/../../../binutils-next-gdb/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c -fdiagnostics-color=never -g -lm -o /home/carll/GDB/build-next/gdb/testsuite/temp/3439835/func_addr.x
builtin_spawn /home/carll/bin/gdb -nw -nx -q -iex set height 0 -iex set width 0
(gdb) set height 0
(gdb) set width 0
(gdb) kill
The program is not being run.
(gdb) file /home/carll/GDB/build-next/gdb/testsuite/temp/3439835/func_addr.x
Reading symbols from /home/carll/GDB/build-next/gdb/testsuite/temp/3439835/func_addr.x...
(gdb) p main_label - main
$1 = 20
(gdb) disassemble main
Dump of assembler code for function main:
   0x000000001000065c <+0>:	lis     r2,4098
   0x0000000010000660 <+4>:	addi    r2,r2,32512
   0x0000000010000664 <+8>:	std     r31,-8(r1)
   0x0000000010000668 <+12>:	stdu    r1,-64(r1)
   0x000000001000066c <+16>:	mr      r31,r1
   0x0000000010000670 <+20>:	nop
   0x0000000010000674 <+24>:	addi    r9,r2,-32472
   0x0000000010000678 <+28>:	li      r10,0
   0x000000001000067c <+32>:	stw     r10,0(r9)
   0x0000000010000680 <+36>:	nop
   0x0000000010000684 <+40>:	addi    r9,r2,-32472
   0x0000000010000688 <+44>:	lwz     r9,0(r9)
   0x000000001000068c <+48>:	addi    r10,r9,1
   0x0000000010000690 <+52>:	nop
   0x0000000010000694 <+56>:	addi    r9,r2,-32472
   0x0000000010000698 <+60>:	stw     r10,0(r9)
   0x000000001000069c <+64>:	nop
   0x00000000100006a0 <+68>:	addi    r9,r2,-32472
   0x00000000100006a4 <+72>:	lwz     r9,0(r9)
   0x00000000100006a8 <+76>:	stw     r9,32(r31)
   0x00000000100006ac <+80>:	nop
   0x00000000100006b0 <+84>:	nop
   0x00000000100006b4 <+88>:	addi    r9,r2,-32472
   0x00000000100006b8 <+92>:	lwz     r9,0(r9)
   0x00000000100006bc <+96>:	addi    r10,r9,1
   0x00000000100006c0 <+100>:	nop
   0x00000000100006c4 <+104>:	addi    r9,r2,-32472
   0x00000000100006c8 <+108>:	stw     r10,0(r9)
   0x00000000100006cc <+112>:	nop
   0x00000000100006d0 <+116>:	addi    r9,r2,-32472
   0x00000000100006d4 <+120>:	lwz     r9,0(r9)
   0x00000000100006d8 <+124>:	stw     r9,32(r31)
   0x00000000100006dc <+128>:	lwz     r9,32(r31)
   0x00000000100006e0 <+132>:	extsw   r9,r9
   0x00000000100006e4 <+136>:	mr      r3,r9
   0x00000000100006e8 <+140>:	addi    r1,r31,64
   0x00000000100006ec <+144>:	ld      r31,-8(r1)
   0x00000000100006f0 <+148>:	blr
   0x00000000100006f4 <+152>:	.long 0x0
   0x00000000100006f8 <+156>:	.long 0x0
   0x00000000100006fc <+160>:	.long 0x1000180
End of assembler dump.
(gdb) with print asm-demangle on -- x/2i main+160
   0x100006fc <main+160>:	.long 0x1000180
   0x10000700 <main_label4+36>:	.long 0x1f8f0
(gdb) Executing on host: gcc  -fno-stack-protector  -fdiagnostics-color=never -c  -o /home/carll/GDB/build-next/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-stepping-3/dw2-inline-stepping-30.o /home/carll/GDB/build-next/gdb/testsuite/../../../binutils-next-gdb/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c    (timeout = 300)
builtin_spawn -ignore SIGHUP gcc -fno-stack-protector -fdiagnostics-color=never -c -o /home/carll/GDB/build-next/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-stepping-3/dw2-inline-stepping-30.o /home/carll/GDB/build-next/gdb/testsuite/../../../binutils-next-gdb/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c
Executing on host: gcc  -fno-stack-protector  -fdiagnostics-color=never -c  -o /home/carll/GDB/build-next/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-stepping-3/dw2-inline-stepping-31.o /home/carll/GDB/build-next/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-stepping-3/dw2-inline-stepping-3.S    (timeout = 300)
builtin_spawn -ignore SIGHUP gcc -fno-stack-protector -fdiagnostics-color=never -c -o /home/carll/GDB/build-next/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-stepping-3/dw2-inline-stepping-31.o /home/carll/GDB/build-next/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-stepping-3/dw2-inline-stepping-3.S
Executing on host: gcc  -fno-stack-protector /home/carll/GDB/build-next/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-stepping-3/dw2-inline-stepping-30.o /home/carll/GDB/build-next/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-stepping-3/dw2-inline-stepping-31.o  -fdiagnostics-color=never  -lm   -o /home/carll/GDB/build-next/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-stepping-3/dw2-inline-stepping-3    (timeout = 300)
builtin_spawn -ignore SIGHUP gcc -fno-stack-protector /home/carll/GDB/build-next/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-stepping-3/dw2-inline-stepping-30.o /home/carll/GDB/build-next/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-stepping-3/dw2-inline-stepping-31.o -fdiagnostics-color=never -lm -o /home/carll/GDB/build-next/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-stepping-3/dw2-inline-stepping-3
builtin_spawn /home/carll/bin/gdb -nw -nx -q -iex set height 0 -iex set width 0
(gdb) set height 0
(gdb) set width 0
(gdb) dir
Reinitialize source path to empty? (y or n) y
Source directories searched: $cdir:$cwd
(gdb) dir /home/carll/GDB/build-next/gdb/testsuite/../../../binutils-next-gdb/gdb/testsuite/gdb.dwarf2
Source directories searched: /home/carll/GDB/build-next/gdb/testsuite/../../../binutils-next-gdb/gdb/testsuite/gdb.dwarf2:$cdir:$cwd
(gdb) kill
The program is not being run.
(gdb) file /home/carll/GDB/build-next/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-stepping-3/dw2-inline-stepping-3
Reading symbols from /home/carll/GDB/build-next/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-stepping-3/dw2-inline-stepping-3...
(gdb) delete breakpoints
(gdb) info breakpoints
No breakpoints or watchpoints.
(gdb) break -qualified main
Breakpoint 1 at 0x100006ac: file /home/carll/GDB/build-next/gdb/testsuite/../../../binutils-next-gdb/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c, line 47.
(gdb) run 
Starting program: /home/carll/GDB/build-next/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-stepping-3/dw2-inline-stepping-3 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".

Breakpoint 1, main () at /home/carll/GDB/build-next/gdb/testsuite/../../../binutils-next-gdb/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c:47
47	  asm ("nop"); ans = bar ();		        /* main call bar */
(gdb) next
49	  return ans;
(gdb) FAIL: gdb.dwarf2/dw2-inline-stepping-3.exp: step to foo callsite
step
0x00007ffff7a30a2c in __libc_start_call_main () from /lib64/libc.so.6
(gdb) FAIL: gdb.dwarf2/dw2-inline-stepping-3.exp: step to foo inc global_var
next
Single stepping until exit from function __libc_start_call_main,
which has no line number information.
[Inferior 1 (process 3439945) exited with code 02]
(gdb) FAIL: gdb.dwarf2/dw2-inline-stepping-3.exp: step to bar callsite (the program exited)
step
The program is not being run.
(gdb) FAIL: gdb.dwarf2/dw2-inline-stepping-3.exp: step to bar inc global_var (the program is no longer running)
testcase /home/carll/GDB/build-next/gdb/testsuite/../../../binutils-next-gdb/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.exp completed in 1 seconds

		=== gdb Summary ===

# of unexpected failures	4
Executing on host: /home/carll/bin/gdb -nw -nx -q -iex "set height 0" -iex "set width 0" --version    (timeout = 300)
builtin_spawn -ignore SIGHUP /home/carll/bin/gdb -nw -nx -q -iex set height 0 -iex set width 0 --version
GNU gdb (GDB) 15.0.50.20240201-git
Copyright (C) 2024 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
/home/carll/bin/gdb version  15.0.50.20240201-git -nw -nx -q -iex "set height 0" -iex "set width 0" 

runtest completed at Thu Feb  1 15:24:47 2024

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] gdb: Don't stop at non-statement line after stepping into inline function
  2024-02-01 20:45   ` [PATCH " Carl Love
@ 2024-02-01 21:35     ` Carl Love
  0 siblings, 0 replies; 7+ messages in thread
From: Carl Love @ 2024-02-01 21:35 UTC (permalink / raw)
  To: gdb-patches, tlloyddavies, Carl Love



On 2/1/24 12:45, Carl Love wrote:
> Toby:
>
> A couple of things.  
>
> First, I tried the series of patches on Power 10. I am getting 
> failures for test gdb.dwarf2/dw2-inline-stepping-3.exp on Power. I attached the log
> file at the bottom of this message.  I haven't had time yet to dig into the
> failures.  I will take a look.

So, looking at the test:


if ![runto_main] {
    return -1
}

gdb_test "next" ".* main call foo.*" "step to foo callsite"
gdb_test "step" ".* foo return global_var.*" "step to foo inc global_var"

gdb_test "next" ".*main call bar.*" "step to bar callsite"
gdb_test "step" ".* bar return global_var.*" "step to bar inc global_var"

It appears you expect to be at the source code line "main call foo":

int
main ()
{                                               /* main prologue */
  int ans;
  asm ("main_label: .globl main_label");
  global_var = 0;                               /* main set global_var */
  asm ("main_label2: .globl main_label2");
  ans = foo ();                                 /* main call foo */       <---- Expect run to main to stop here
  asm ("main_label3: .globl main_label3");
  asm ("nop"); ans = bar ();                    /* main call bar */       <---- Appears this is where gdb actually
                                                                                stops.
  asm ("main_label4: .globl main_label4");
  return ans;
}                                               /* main end */

based on the test log file:

Breakpoint 1, main () at /home/carll/GDB/build-next/gdb/testsuite/../../../binu\
tils-next-gdb/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c:47^M
47        asm ("nop"); ans = bar ();                    /* main call bar */^M
(gdb) next^M
49        return ans;^M
(gdb) FAIL: gdb.dwarf2/dw2-inline-stepping-3.exp: step to foo callsite


Note, I tried executing the test under gdb by hand and see the same thing:

gdb) break main
Breakpoint 1 at 0x100006ac: file /home/carll/GDB/build-next/gdb/testsuite/../../../binutils-next-gdb/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c, line 47.
(gdb) display/x $pc
1: /x $pc = <error: No registers.>
(gdb) r
Starting program: /home/carll/GDB/build-next/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-stepping-3/dw2-inline-stepping-3 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".

Breakpoint 1, main ()
    at /home/carll/GDB/build-next/gdb/testsuite/../../../binutils-next-gdb/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c:47
47	  asm ("nop"); ans = bar ();		        /* main call bar */
1: /x $pc = 0x100006ac
(gdb) n
49	  return ans;
1: /x $pc = 0x100006dc
(gdb) n
Downloading source file /usr/src/debug/glibc-2.37-16.fc38.ppc64le/csu/../sysdeps/nptl/libc_start_call_main.h
__libc_start_call_main (main=main@entry=0x1000065c <main>, argc=argc@entry=1,   
    argv=argv@entry=0x7fffffffe978, auxvec=auxvec@entry=0x7fffffffeb60)
    at ../sysdeps/nptl/libc_start_call_main.h:74
74	  exit (result);
1: /x $pc = 0x7ffff7a30a30


My first guess is the inlining completely eliminated the call to foo?

So, I threw a printf for ans in after the call for foo.  That fixes the first error,
we now stop at the call to foo. But we don't seem to but we don't seem to stop in
the expected places in foo.

source file:

  int ans;
  asm ("main_label: .globl main_label");
  global_var = 0;                               /* main set global_var */
  asm ("main_label2: .globl main_label2");
  ans = foo ();                                 /* main call foo */
  printf("ans = %d\n", ans);       	      	   /* printf */
  asm ("main_label3: .globl main_label3");
  asm ("nop"); ans = bar ();                    /* main call bar */
  asm ("main_label4: .globl main_label4");
  return ans;
 
expect file:

gdb_test "next" ".* main call foo.*" "step to foo callsite"
gdb_test "step" ".* foo return global_var.*" "step to foo inc global_var"
gdb_test "next" ".*printf.*" "step to printf"
gdb_test "next" ".*main call bar.*" "step to bar callsite"
gdb_test "step" ".* bar return global_var.*" "step to bar inc global_var"



Here is the output of my manual run:
(gdb) run 
Starting program: /.../gdb.dwarf2/dw2-inline-stepping-3/dw2-inline-stepping-3 

[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".

Breakpoint 1, main () at /.../gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c:44
44        global_var = 0;                               /* main set global_var */
(gdb) next
46        ans = foo ();                                 /* main call foo */
(gdb) PASS: gdb.dwarf2/dw2-inline-stepping-3.exp: step to foo callsite
step
0x00000000100006e8 in foo () at /.../gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c:25
25        global_var++;                                 /* foo inc global_var*/
(gdb) FAIL: gdb.dwarf2/dw2-inline-stepping-3.exp: step to foo inc global_var
next
27        return global_var;                            /* foo return global_var */
(gdb) FAIL: gdb.dwarf2/dw2-inline-stepping-3.exp: step to printf
next
ans = 1
main () at /.../gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c:49
49        asm ("nop"); ans = bar ();                    /* main call bar */
(gdb) PASS: gdb.dwarf2/dw2-inline-stepping-3.exp: step to bar callsite
step
0x0000000010000730 in bar () at /..../gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-3.c:34
34        global_var++;                                 /* bar inc global_var*/
(gdb) FAIL: gdb.dwarf2/dw2-inline-stepping-3.exp: step to bar inc global_var
testcase /..../gdb.dwarf2/dw2-inline-stepping-3.exp completed in 0 seconds

It looks to me like you need just a little more "meat" in the function calls
to keep the compiler from optimizing everything away.  

Let me know if you have some new version of the test you would like me to run.  More
than happy to help with the Power testing.

                            Carl 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-02-01 21:36 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-01 15:21 [PATCH 0/3] Fix inline function stepping Toby Lloyd Davies
2024-02-01 15:21 ` [PATCH 1/3] gdb: Fix missed inline callsite when the frame has changed Toby Lloyd Davies
2024-02-01 15:21 ` [PATCH 2/3] gdb: Don't stop at non-statement line after stepping into inline function Toby Lloyd Davies
2024-02-01 15:43   ` [PATCH v2 " Toby Lloyd Davies
2024-02-01 20:45   ` [PATCH " Carl Love
2024-02-01 21:35     ` Carl Love
2024-02-01 15:21 ` [PATCH 3/3] gdb: Fix 'next' skipping over remainder of " Toby Lloyd Davies

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).