From d43b7a2d5718f303a9e284f0d14219f05fbcfa17 Mon Sep 17 00:00:00 2001 From: Tankut Baris Aktemur Date: Tue, 21 Apr 2020 17:24:03 +0200 Subject: [PATCH] gdb/infrun: switch the context before 'displaced_step_restore' 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 [] ()] (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-21 Tankut Baris Aktemur * infrun.c (displaced_step_fixup): Switch to the event_thread before calling displaced_step_restore, not after. gdb/testsuite/ChangeLog: 2020-04-21 Tankut Baris Aktemur * gdb.multi/run-only-second-inf.c: New file. * gdb.multi/run-only-second-inf.exp: New file. --- gdb/ChangeLog | 5 ++ gdb/infrun.c | 11 ++-- gdb/testsuite/ChangeLog | 5 ++ gdb/testsuite/gdb.multi/run-only-second-inf.c | 22 ++++++++ .../gdb.multi/run-only-second-inf.exp | 50 +++++++++++++++++++ 5 files changed, 88 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/ChangeLog b/gdb/ChangeLog index bdec0f3907..89940c658e 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2020-04-21 Tankut Baris Aktemur + + * infrun.c (displaced_step_fixup): Switch to the event_thread + before calling displaced_step_restore, not after. + 2020-04-21 Markus Metzger * record-btrace.c (record_btrace_enable_warn): Ignore thread if diff --git a/gdb/infrun.c b/gdb/infrun.c index 5d60e64230..0e1ba6986b 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/ChangeLog b/gdb/testsuite/ChangeLog index 038df8634b..affdb63cd7 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-04-21 Tankut Baris Aktemur + + * gdb.multi/run-only-second-inf.c: New file. + * gdb.multi/run-only-second-inf.exp: New file. + 2020-04-21 Markus Metzger * gdb.btrace/multi-inferior.c: New test. 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 0000000000..f4825c8a7c --- /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 . */ + +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 0000000000..4cce712bd6 --- /dev/null +++ b/gdb/testsuite/gdb.multi/run-only-second-inf.exp @@ -0,0 +1,50 @@ +# 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 . + +# Create two inferiors that are not running. Then run only the second +# one. Expect it to start normally. + +standard_testfile + +if {[use_gdb_stub]} { + return 0 +} + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug}]} { + return -1 +} + +# Setting the target to non-stop mode revealed a bug where the context +# was not being switched before making ptid-dependent memory access. +# So, start GDB with this setting. +save_vars { GDBFLAGS } { + append GDBFLAGS " -ex \"maint set target-non-stop on\"" + clean_restart ${binfile} +} + +# 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.34.1