Fix adjust_pc_after_break, remove still current thread check

On decr_pc_after_break targets, GDB adjusts the PC incorrectly if a
background single-step stops somewhere where PC-$decr_pc has a
breakpoint, and the thread that finishes the step is not the current
thread, like:

   ADDR1 nop <-- breakpoint here
   ADDR2 jmp PC

IOW, say thread A is stepping ADDR2's line in the background (an
infinite loop), and the user switches focus to thread B.  GDB's
adjust_pc_after_break logic confuses the single-step stop of thread A
for a hit of the breakpoint at ADDR1, and thus adjusts thread A's PC
to point at ADDR1 when it should not, and reports a breakpoint hit,
when thread A did not execute the instruction at ADDR1 at all.

The test added by this patch exercises exactly that.

I can't find any reason we'd need the "thread to be examined is still
the current thread" condition in adjust_pc_after_break, at least
nowadays; it might have made sense in the past.  Best just remove it,
and rely on currently_stepping().

Here's the test's log of a run with an unpatched GDB:

 35        while (1);
 (gdb) PASS: gdb.threads/step-bg-decr-pc-switch-thread.exp: next over nop
 next&
 (gdb) PASS: gdb.threads/step-bg-decr-pc-switch-thread.exp: next& over inf loop
 thread 1
 [Switching to thread 1 (Thread 0x7ffff7fc2740 (LWP 29027))](running)
 (gdb)
 PASS: gdb.threads/step-bg-decr-pc-switch-thread.exp: switch to main thread
 Breakpoint 2, thread_function (arg=0x0) at ...src/gdb/testsuite/gdb.threads/step-bg-decr-pc-switch-thread.c:34
 34        NOP; /* set breakpoint here */
 FAIL: gdb.threads/step-bg-decr-pc-switch-thread.exp: no output while stepping

gdb/ChangeLog:
2015-02-11  Pedro Alves  <pedro@codesourcery.com>

	* infrun.c (adjust_pc_after_break): Don't adjust the PC just
	because the event thread is not the current thread.

gdb/testsuite/ChangeLog:
2015-02-11  Pedro Alves  <pedro@codesourcery.com>

	* gdb.threads/step-bg-decr-pc-switch-thread.c: New file.
	* gdb.threads/step-bg-decr-pc-switch-thread.exp: New file.
This commit is contained in:
Pedro Alves 2015-02-11 09:45:41 +00:00
parent 07f107f306
commit 0703599a49
5 changed files with 155 additions and 2 deletions

View File

@ -1,3 +1,8 @@
2015-02-11 Pedro Alves <pedro@codesourcery.com>
* infrun.c (adjust_pc_after_break): Don't adjust the PC just
because the event thread is not the current thread.
2015-02-11 Doug Evans <xdje42@gmail.com>
* gdbtypes.c (internal_type_self_type): If TYPE_SPECIFIC_FIELD hasn't

View File

@ -3487,7 +3487,6 @@ adjust_pc_after_break (struct execution_control_state *ecs)
The SIGTRAP can be due to a completed hardware single-step only if
- we didn't insert software single-step breakpoints
- the thread to be examined is still the current thread
- this thread is currently being stepped
If any of these events did not occur, we must have stopped due
@ -3499,7 +3498,6 @@ adjust_pc_after_break (struct execution_control_state *ecs)
we also need to back up to the breakpoint address. */
if (thread_has_single_step_breakpoints_set (ecs->event_thread)
|| !ptid_equal (ecs->ptid, inferior_ptid)
|| !currently_stepping (ecs->event_thread)
|| (ecs->event_thread->stepped_breakpoint
&& ecs->event_thread->prev_pc == breakpoint_pc))

View File

@ -1,3 +1,8 @@
2015-02-11 Pedro Alves <pedro@codesourcery.com>
* gdb.threads/step-bg-decr-pc-switch-thread.c: New file.
* gdb.threads/step-bg-decr-pc-switch-thread.exp: New file.
2015-02-10 Doug Evans <xdje42@gmail.com>
* lib/gdb.exp (gdb_load): Always return a result.

View File

@ -0,0 +1,54 @@
/* This testcase is part of GDB, the GNU debugger.
Copyright 2015 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 <unistd.h>
#include <pthread.h>
#include <assert.h>
/* NOP instruction: must have the same size as the breakpoint
instruction for the test to be effective. */
#if defined (__s390__) || defined (__s390x__)
# define NOP asm ("nopr 0")
#else
# define NOP asm ("nop")
#endif
void *
thread_function (void *arg)
{
NOP; /* set breakpoint here */
while (1);
}
int
main (void)
{
int res;
pthread_t thread;
alarm (300);
res = pthread_create (&thread,
NULL,
thread_function,
NULL);
assert (res == 0);
pthread_join (thread, NULL);
return 0;
}

View File

@ -0,0 +1,91 @@
# Copyright (C) 2014-2015 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/>.
# On decr_pc_after_break targets, GDB used to adjust the PC
# incorrectly if a background single-step stopped somewhere where
# PC-$decr_pc had a breakpoint, and the thread was not the current
# thread, like:
#
# ADDR1 nop <-- breakpoint here
# ADDR2 jmp PC
#
# IOW, say thread A is stepping ADDR2's line in the background (an
# infinite loop), and the user switches focus to thread B. GDB's
# adjust_pc_after_break logic would confuse the single-step stop of
# thread A for a hit of the breakpoint at ADDR1, and thus adjust
# thread A's PC to point at ADDR1 when it should not: the thread had
# been single-stepped, not continued.
standard_testfile
if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug pthreads}] == -1} {
return -1
}
clean_restart $binfile
if ![runto_main] {
continue
}
# Make sure it's GDB's decr_pc logic that's being tested, not the
# target's.
gdb_test_no_output "set range-stepping off"
delete_breakpoints
gdb_breakpoint [gdb_get_line_number "set breakpoint here"]
gdb_continue_to_breakpoint "run to nop breakpoint"
gdb_test "info threads" "\\\* 2 .* 1.*" "info threads shows all threads"
gdb_test "next" "while.*" "next over nop"
gdb_test_no_output "next&" "next& over inf loop"
set test "switch to main thread"
gdb_test_multiple "thread 1" $test {
-re "Cannot execute this command while the target is running.*$gdb_prompt $" {
unsupported $test
# With remote targets, we can't send any other remote packet
# until the target stops. Switching thread wants to ask the
# remote side whether the thread is alive.
return
}
-re "Switching to thread 1.*\\(running\\)\r\n$gdb_prompt " {
# Prefer to match the prompt without an anchor. If there's a
# bug and output comes after the prompt immediately, it's
# faster to handle that in the following test, instead of
# waiting for a timeout here.
pass $test
}
}
# Wait a bit. Use gdb_expect instead of sleep so that any (bad) GDB
# output is visible in the log.
gdb_expect 4 {}
set test "no output while stepping"
gdb_test_multiple "" $test {
-timeout 1
timeout {
pass $test
}
-re "." {
# If we see any output, it's a failure. On the original bug,
# this would be a breakpoint hit.
fail $test
}
}