This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[PATCHv3] Make "skip" work on inline frames
On 10/31/19 7:00 PM, Pedro Alves wrote:
> On 10/31/19 4:53 PM, Simon Marchi wrote:
>> On 2019-10-31 12:42 p.m., Pedro Alves wrote:
>>> On 10/30/19 9:56 PM, Bernd Edlinger wrote:
>>>> +if { [prepare_for_testing "failed to prepare" "skip2" \
>>>> + {skip2.c skip1.c } \
>>>> + {debug nowarnings optimize=-O2}] } {
>>>
>>> Instead of -O2, could you make this use -O0 (the default),
>>> and then use attribute((always_inline)) to force inlining?
>>> We do that in some tests. E.g., gdb.opt/inline-locals.c.
>>
>> I think that's a good suggestion, but just be aware that there used to be
>> some problems with always_inline, e.g.:
>
> I don't think always_inline changes anything in the debug info special,
> it just tells the compiler to inline the function even at -O0, which is
> what we're after.
>
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=13263
>
> This one seems to be about attribute((artificial)), and the desire
> to not step into such functions automatically:
>
> "Given that I marked the function always_inline and artificial, I
> was expecting not to step into its body."
>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=12429
>>
>
> This one seems like a generic inlining issue.
>
>> I'm not sure if those are still valid. If they are, it might be more difficult
>> that expected to use always_inline.
> I don't think always_inline is anything special compared to inlining
> because of -O2.
>
> Thanks,
> Pedro Alves
>
Ah, thanks for that hint!
always_inline works quite well.
The debug session started (using gcc 4.8.4) with -O2 -g on the line with "{" in main,
and with -O0 -g at the first real statement, so I had to remove one step.
I did not worry about it initially, but now I agree that would have caused trouble.
I attached both parts of the patch, the fist part unchanged from previous.
The test case now makes sure to not repeat the names.
Thanks
Bernd.
From fa00b1890e525b4e8e9a8397bddfa9963c253080 Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Fri, 18 Oct 2019 14:28:45 +0200
Subject: [PATCH 1/2] Check all inline frames if they are marked for skip.
This makes the skip command work in optimized builds,
where skipped functions may be inlined.
Previously that was only working when stepping into
a non-inlined function.
---
gdb/infcmd.c | 20 ++++++++++++++++++--
gdb/infrun.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 69 insertions(+), 5 deletions(-)
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 7105774..f4c183c 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -58,6 +58,7 @@
#include "thread-fsm.h"
#include "top.h"
#include "interps.h"
+#include "skip.h"
#include "gdbsupport/gdb_optional.h"
#include "source.h"
#include "cli/cli-style.h"
@@ -1112,14 +1113,29 @@ prepare_one_step (struct step_command_fsm *sm)
&& inline_skipped_frames (tp))
{
ptid_t resume_ptid;
+ const char *fn = NULL;
+ symtab_and_line sal;
+ struct symbol *sym;
/* Pretend that we've ran. */
resume_ptid = user_visible_resume_ptid (1);
set_running (resume_ptid, 1);
step_into_inline_frame (tp);
- sm->count--;
- return prepare_one_step (sm);
+
+ frame = get_current_frame ();
+ sal = find_frame_sal (frame);
+ sym = get_frame_function (frame);
+
+ if (sym != NULL)
+ fn = SYMBOL_PRINT_NAME (sym);
+
+ if (sal.line == 0
+ || !function_name_is_marked_for_skip (fn, sal))
+ {
+ sm->count--;
+ return prepare_one_step (sm);
+ }
}
pc = get_frame_pc (frame);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 07aebfa..04c1eee 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4041,6 +4041,45 @@ stepped_in_from (struct frame_info *frame, struct frame_id step_frame_id)
return 0;
}
+/* 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
+ first non-inline frame, or at the frame where the
+ step started. */
+
+static bool
+inline_frame_is_marked_for_skip (bool prev_frame, struct thread_info *tp)
+{
+ struct frame_info *frame = get_current_frame ();
+
+ if (prev_frame)
+ frame = get_prev_frame (frame);
+
+ for (; frame != NULL; frame = get_prev_frame (frame))
+ {
+ const char *fn = NULL;
+ symtab_and_line sal;
+ struct symbol *sym;
+
+ if (frame_id_eq (get_frame_id (frame), tp->control.step_frame_id))
+ break;
+ if (get_frame_type (frame) != INLINE_FRAME)
+ break;
+
+ sal = find_frame_sal (frame);
+ sym = get_frame_function (frame);
+
+ if (sym != NULL)
+ fn = SYMBOL_PRINT_NAME (sym);
+
+ if (sal.line != 0
+ && function_name_is_marked_for_skip (fn, sal))
+ return true;
+ }
+
+ return false;
+}
+
/* If the event thread has the stop requested flag set, pretend it
stopped for a GDB_SIGNAL_0 (i.e., as if it stopped due to
target_stop). */
@@ -6531,7 +6570,8 @@ process_event_stop_test (struct execution_control_state *ecs)
tmp_sal = find_pc_line (ecs->stop_func_start, 0);
if (tmp_sal.line != 0
&& !function_name_is_marked_for_skip (ecs->stop_func_name,
- tmp_sal))
+ tmp_sal)
+ && !inline_frame_is_marked_for_skip (true, ecs->event_thread))
{
if (execution_direction == EXEC_REVERSE)
handle_step_into_function_backward (gdbarch, ecs);
@@ -6697,7 +6737,14 @@ process_event_stop_test (struct execution_control_state *ecs)
if (call_sal.line == ecs->event_thread->current_line
&& call_sal.symtab == ecs->event_thread->current_symtab)
- step_into_inline_frame (ecs->event_thread);
+ {
+ step_into_inline_frame (ecs->event_thread);
+ if (inline_frame_is_marked_for_skip (false, ecs->event_thread))
+ {
+ keep_going (ecs);
+ return;
+ }
+ }
end_stepping_range (ecs);
return;
@@ -6731,7 +6778,8 @@ process_event_stop_test (struct execution_control_state *ecs)
fprintf_unfiltered (gdb_stdlog,
"infrun: stepping through inlined function\n");
- if (ecs->event_thread->control.step_over_calls == STEP_OVER_ALL)
+ 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);
--
1.9.1
From d8fc12de59c47c94ef42f91e0793554df1e4d714 Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Wed, 30 Oct 2019 21:35:22 +0100
Subject: [PATCH 2/2] Add a test case for skip with inlined functions
---
gdb/testsuite/gdb.base/skip2.c | 64 ++++++++++++++++++++++++++++++++
gdb/testsuite/gdb.base/skip2.exp | 80 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 144 insertions(+)
create mode 100644 gdb/testsuite/gdb.base/skip2.c
create mode 100644 gdb/testsuite/gdb.base/skip2.exp
diff --git a/gdb/testsuite/gdb.base/skip2.c b/gdb/testsuite/gdb.base/skip2.c
new file mode 100644
index 0000000..d1abd45
--- /dev/null
+++ b/gdb/testsuite/gdb.base/skip2.c
@@ -0,0 +1,64 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2019 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/>. */
+
+#include <stdlib.h>
+
+int bar (void);
+int baz (int);
+void skip1_test_skip_file_and_function (void);
+void test_skip_file_and_function (void);
+
+__attribute__((__always_inline__)) static inline int
+foo (void)
+{
+ return bar ();
+}
+
+int
+main ()
+{
+ volatile int x;
+
+ /* step immediately into the inlined code */
+ x = baz (foo ());
+
+ /* step first over non-inline code, this involves a different code path */
+ x = 0; x = baz (foo ());
+
+ test_skip_file_and_function ();
+
+ return 0;
+}
+
+static void
+test_skip (void)
+{
+}
+
+static void
+end_test_skip_file_and_function (void)
+{
+ abort ();
+}
+
+void
+test_skip_file_and_function (void)
+{
+ test_skip ();
+ skip1_test_skip_file_and_function ();
+ end_test_skip_file_and_function ();
+}
diff --git a/gdb/testsuite/gdb.base/skip2.exp b/gdb/testsuite/gdb.base/skip2.exp
new file mode 100644
index 0000000..a59dd0f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/skip2.exp
@@ -0,0 +1,80 @@
+# Copyright 2019 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/>.
+
+load_lib completion-support.exp
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" "skip2" \
+ {skip2.c skip1.c } \
+ {debug nowarnings}] } {
+ return -1
+}
+
+set srcfile skip2.c
+set srcfile1 skip1.c
+
+if ![runto_main] {
+ fail "can't run to main"
+ return
+}
+
+# Create a skiplist entry for a specified file and function.
+
+gdb_test "skip function foo" "Function foo will be skipped when stepping\."
+
+gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
+gdb_test "step" ".*" "step into baz, since foo will be skipped"
+gdb_test "bt" "\\s*\\#0\\s+baz.*" "in the baz, since foo was skipped"
+gdb_test "step" ".*" "step in the baz"
+gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz"
+gdb_test "step" ".*" "step back to main"
+gdb_test "bt" "\\s*\\#0\\s+main.*" "again in the main"
+gdb_test "step" ".*" "step again into baz, since foo will be skipped"
+gdb_test "bt" "\\s*\\#0\\s+baz.*" "again in the baz"
+gdb_test "step" ".*" "step in the baz, again"
+gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz, again"
+gdb_test "step" ".*" "step back to main, again"
+gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
+
+if ![runto_main] {
+ fail "can't run to main"
+ return
+}
+
+with_test_prefix "double step" {
+ gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
+ gdb_test "step 2" ".*" "step into baz, since foo will be skipped"
+ gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz"
+ gdb_test "step" ".*" "step back to main"
+ gdb_test "bt" "\\s*\\#0\\s+main.*" "again in the main"
+ gdb_test "step 2" ".*" "step again into baz, since foo will be skipped"
+ gdb_test "bt" "\\s*\\#0\\s+baz.*" "again in the baz"
+ gdb_test "step" ".*" "step back to main, again"
+ gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
+}
+
+if ![runto_main] {
+ fail "can't run to main"
+ return
+}
+
+with_test_prefix "triple step" {
+ gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
+ gdb_test "step 3" ".*" "step over baz"
+ gdb_test "bt" "\\s*\\#0\\s+main.*" "again in the main"
+ gdb_test "step 3" ".*" "step over baz, again"
+ gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
+}
--
1.9.1