* [PATCH 1/2] testsuite: introduce 'default_gdb_start_post_cmd' as an overridable function
2020-04-20 16:24 [PATCH 0/2] Context-switch in finish-step-over flow Tankut Baris Aktemur
@ 2020-04-20 16:24 ` Tankut Baris Aktemur
2020-04-20 16:24 ` [PATCH 2/2] gdb/infrun: switch the context before 'displaced_step_restore' Tankut Baris Aktemur
1 sibling, 0 replies; 5+ messages in thread
From: Tankut Baris Aktemur @ 2020-04-20 16:24 UTC (permalink / raw)
To: gdb-patches; +Cc: palves
Introduce a function named 'default_gdb_start_post_cmd' that is
invoked at the end of 'default_gdb_start'. The function is by default
empty and can be overridden in a board file or a test to perform
setup right after GDB starts.
The need for this function arose from the fact that doing
'maint set target-non-stop on' was not possible when using the
native-extended-gdbserver board file. This board file overrides
gdb_start as below:
proc gdb_start { } {
# Spawn GDB.
default_gdb_start
# And then GDBserver, ready for extended-remote mode.
gdbserver_start_multi
return 0
}
That is, GDB is started and an extended-remote target is defined
immediately following it. However, to set the remote target to
non-stop mode, we would have to issue the 'maint set target-non-stop on'
code ...
proc gdb_start { } {
# Spawn GDB.
default_gdb_start
--> ### ... right here.
# And then GDBserver, ready for extended-remote mode.
gdbserver_start_multi
return 0
}
If the target-non-stop mode is set after making the connection,
we see the following problem:
$ gdb
(gdb) file a.out
...
(gdb) set remote exec-file a.out
(gdb) target extended-remote | gdbserver --once --multi -
...
(gdb) maint set target-non-stop on
(gdb) run
...
Unexpected vCont reply in non-stop mode: T05swbreak:;06:f0dfffffff7f0000;07:10deffffff7f0000;10:d194ddf7ff7f0000;thread:p4706.4706;core:d;
This patch provides a generalized solution and gives the ability to do
custom setup after starting GDB but before connecting to the target.
gdb/testsuite/ChangeLog:
2020-04-20 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* lib/gdb.exp (default_gdb_start_post_cmd): New overridable
function.
(default_gdb_start): Invoke 'default_gdb_start_post_cmd'
at the end.
---
gdb/testsuite/lib/gdb.exp | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 8418c3d8753..626edee6265 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1849,6 +1849,17 @@ proc default_gdb_spawn { } {
return 0
}
+# Overridable procedure that is called at the end of
+# default_gdb_start.
+
+proc default_gdb_start_post_cmd { } {
+ # Do nothing by default.
+ # You can override this in your board file or your test.
+ #
+ # E.g. Do "maint set target-non-stop on" to start in
+ # all-stop-on-top-of-non-stop mode.
+}
+
# Default gdb_start procedure.
proc default_gdb_start { } {
@@ -1919,6 +1930,7 @@ proc default_gdb_start { } {
}
gdb_debug_init
+ default_gdb_start_post_cmd
return 0
}
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 2/2] gdb/infrun: switch the context before 'displaced_step_restore'
2020-04-20 16:24 [PATCH 0/2] Context-switch in finish-step-over flow Tankut Baris Aktemur
2020-04-20 16:24 ` [PATCH 1/2] testsuite: introduce 'default_gdb_start_post_cmd' as an overridable function Tankut Baris Aktemur
@ 2020-04-20 16:24 ` Tankut Baris Aktemur
2020-04-21 14:38 ` Pedro Alves
1 sibling, 1 reply; 5+ messages in thread
From: Tankut Baris Aktemur @ 2020-04-20 16:24 UTC (permalink / raw)
To: gdb-patches; +Cc: palves
In infrun.c's 'displaced_step_fixup', as part of the 'finish_step_over'
flow, switch to the eventing thread *before* calling
'displaced_step_restore', because down in the flow ptid-dependent
memory accesses are used via current_inferior() and current_top_target().
Without this patch, the problem is exposed with the scenario below:
$ gdb -q
(gdb) maint set target-non-stop on
(gdb) file a.out
Reading symbols from a.out...
(gdb) set remote exec-file a.out
(gdb) target extended-remote | gdbserver --once --multi -
...
(gdb) add-inferior
[New inferior 2]
Added inferior 2 on connection 1 (extended-remote ...)
(gdb) inferior 2
[Switching to inferior 2 [<null>] (<noexec>)]
(gdb) file a.out
Reading symbols from a.out...
(gdb) set remote exec-file a.out
(gdb) run
...
Cannot access memory at address 0x555555555042
(gdb)
The problem is, down inside 'displaced_step_restore', GDB wants to
access the memory for inferior 2 because of an internal breakpoint.
However, the current inferior and inferior_ptid are out of sync.
While inferior_ptid correctly points to the process of inf 2 that was
just started, current_inferior points to inf 1. Then, the attempt to
access the memory fails, because target_has_execution results in false
since inf 1 was not started. I was not able to simplify the failing
scenario, but it shows the problem.
After this patch, we get
... same steps above...
(gdb) run
...
[Inferior 2 (process 28652) exited normally]
(gdb)
Regression-tested on X86_64 Linux with `make check`s default board file
and also `--target_board=native-extended-gdbserver`. In fact, the bug
fixed by this patch was exposed when using the native-extended-gdbserver
board file.
gdb/ChangeLog:
2020-04-20 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* infrun.c (displaced_step_fixup): Switch to the event_thread
before calling displaced_step_restore, not after.
gdb/testsuite/ChangeLog:
2020-04-20 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* gdb.multi/run-only-second-inf.c: New file.
* gdb.multi/run-only-second-inf.exp: New file.
---
gdb/infrun.c | 11 +++--
gdb/testsuite/gdb.multi/run-only-second-inf.c | 22 +++++++++
.../gdb.multi/run-only-second-inf.exp | 49 +++++++++++++++++++
3 files changed, 77 insertions(+), 5 deletions(-)
create mode 100644 gdb/testsuite/gdb.multi/run-only-second-inf.c
create mode 100644 gdb/testsuite/gdb.multi/run-only-second-inf.exp
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 5d60e64230b..0e1ba6986b1 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1884,15 +1884,16 @@ displaced_step_fixup (thread_info *event_thread, enum gdb_signal signal)
if (displaced->step_thread != event_thread)
return 0;
- displaced_step_reset_cleanup cleanup (displaced);
-
- displaced_step_restore (displaced, displaced->step_thread->ptid);
-
/* Fixup may need to read memory/registers. Switch to the thread
that we're fixing up. Also, target_stopped_by_watchpoint checks
- the current thread. */
+ the current thread, and displaced_step_restore performs ptid-dependent
+ memory accesses using current_inferior() and current_top_target(). */
switch_to_thread (event_thread);
+ displaced_step_reset_cleanup cleanup (displaced);
+
+ displaced_step_restore (displaced, displaced->step_thread->ptid);
+
/* Did the instruction complete successfully? */
if (signal == GDB_SIGNAL_TRAP
&& !(target_stopped_by_watchpoint ()
diff --git a/gdb/testsuite/gdb.multi/run-only-second-inf.c b/gdb/testsuite/gdb.multi/run-only-second-inf.c
new file mode 100644
index 00000000000..f4825c8a7c1
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/run-only-second-inf.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2020 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+int
+main ()
+{
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.multi/run-only-second-inf.exp b/gdb/testsuite/gdb.multi/run-only-second-inf.exp
new file mode 100644
index 00000000000..874cdcd13e9
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/run-only-second-inf.exp
@@ -0,0 +1,49 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2020 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Create two inferiors where the first one is not running. Then run the
+# second one. Expect it to start normally.
+
+standard_testfile
+
+if {[use_gdb_stub]} {
+ return 0
+}
+
+proc default_gdb_start_post_cmd { } {
+ # This setting revealed a bug where the context
+ # was not being switched before making ptid-dependent
+ # memory access.
+ gdb_test_no_output "maint set target-non-stop on"
+}
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug}]} {
+ return -1
+}
+
+# Add and start the second inferior.
+gdb_test "add-inferior" "Added inferior 2.*" \
+ "add empty inferior 2"
+gdb_test "inferior 2" "Switching to inferior 2.*" \
+ "switch to inferior 2"
+gdb_load $binfile
+
+if {[gdb_start_cmd] < 0} {
+ fail "start the second inf"
+} else {
+ gdb_test "" ".*reakpoint ., main .*${srcfile}.*" "start the second inf"
+}
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread