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 [<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-21  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-21  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.
This commit is contained in:
Tankut Baris Aktemur 2020-04-21 17:24:03 +02:00
parent bb2a145347
commit d43b7a2d57
5 changed files with 88 additions and 5 deletions

View File

@ -1,3 +1,8 @@
2020-04-21 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.
2020-04-21 Markus Metzger <markus.t.metzger@intel.com>
* record-btrace.c (record_btrace_enable_warn): Ignore thread if

View File

@ -1884,15 +1884,16 @@ displaced_step_fixup (thread_info *event_thread, enum gdb_signal signal)
if (displaced->step_thread != event_thread)
return 0;
/* 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, 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);
/* 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. */
switch_to_thread (event_thread);
/* Did the instruction complete successfully? */
if (signal == GDB_SIGNAL_TRAP
&& !(target_stopped_by_watchpoint ()

View File

@ -1,3 +1,8 @@
2020-04-21 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.
2020-04-21 Markus Metzger <markus.t.metzger@intel.com>
* gdb.btrace/multi-inferior.c: New test.

View File

@ -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;
}

View File

@ -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 <http://www.gnu.org/licenses/>.
# 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"
}