* [rfc][1/2] Signal delivery + software single-step is broken
@ 2011-01-19 16:43 Ulrich Weigand
2011-04-27 17:17 ` [commit] " Ulrich Weigand
0 siblings, 1 reply; 10+ messages in thread
From: Ulrich Weigand @ 2011-01-19 16:43 UTC (permalink / raw)
To: gdb-patches
Hello,
we're seeing failing testcases related to signal delivery on ARM,
which turn out to be related to software single-stepping.
If GDB wants to pass a signal to the inferior, it calls ptrace
with non-zero "data" argument. If at this point, the inferior
was stopped on a breakpoint location, GDB will in addition need
to single-step off that breakpoint.
Now, if the platform supports hardware single-stepping, this will
end up as a PTRACE_SINGLESTEP ptrace call with non-zero "data".
If delivery of that signal results in invocation of a handler,
GDB will expect the OS to trap back to the debugger when the
inferior is at the first instruction of the handler. (At this
point, GDB usually skip the handler by placing a step-resume
breakpoint at its return address and the single-stepping
through the signal return trampoline.)
However, if the platform does *not* support hardware single-
stepping, things seem broken. GDB will attempt to fall back
to inserting software single-step breakpoints. Now this works
only if GDB is able to determine the set of potential next
instruction addresses. If a signal is about to be delivered,
however, the start of the signal handler is one of these
potential addresses -- but the software single-step code does
not know about those. (And in fact, there does not seem to be
an easy way to safely and efficiently determine them.)
So what happens is that we'll set single-step breakpoints only
on the original successors to the current instruction, and then
deliver the signal. This means execution runs through the
signal handler, while GDB has removed all other breakpoints
(since we're stepping off a breakpoint!). If there was a
breakpoint somewhere in the signal handler, we will therefore
miss it.
I'm not completely sure how to best fix this problem. One option
could be to change GDB's method of handling signal delivery while
stopped on a breakpoint location: we could just deliver the signal
first, while a step-resume breakpoint on the current location is
in effect; and once that breakpoint is hit, we *then* single-step
off the location as usual. This is actually already implemented
in handle_inferior_event when delivering a signal that originated
in the inferior itself. The patch below moves that mechanism to
resume, so that it gets also used for delivering signals that
originate in GDB (e.g. via the "signal" command).
This fixes the following set of test cases on ARM:
FAIL: gdb.base/annota1.exp: send SIGUSR1 (timeout)
FAIL: gdb.base/annota1.exp: backtrace @ signal handler (timeout)
FAIL: gdb.base/annota3.exp: send SIGUSR1 (pattern 4)
FAIL: gdb.base/annota3.exp: backtrace @ signal handler (pattern 1)
and in conjunction with the follow-on patch, it also fixes:
FAIL: gdb.base/sigstep.exp: step on breakpoint, to handler; performing step
FAIL: gdb.base/sigstep.exp: next on breakpoint, to handler; performing next
FAIL: gdb.base/sigstep.exp: continue on breakpoint, to handler; performing continue
FAIL: gdb.base/sigstep.exp: step on breakpoint, to handler entry; performing step
FAIL: gdb.base/sigstep.exp: next on breakpoint, to handler entry; performing next
FAIL: gdb.base/sigstep.exp: continue on breakpoint, to handler entry; performing continue
I've tested the patch on i386-linux, s390-linux, and powerpc-linux
(including Cell/B.E.) with no regressions.
One potential problem that this patch still does not resolve is if we
at the same time are also doing software-emulated watchpoints (which
are implemented by single-stepping all the time). With the patch as
below, software single-stepping would be disabled while a signal
handler is executing, so we'd notice changes to a watches location
only after the handler has completed ...
I'd appreciate any comments -- is there a better way of doing this?
Bye,
Ulrich
ChangeLog:
* infrun.c (resume): Do not single-step into signal delivery
when stepping off a breakpoint location.
(handle_inferior_event): Update to new resume behaviour.
(insert_step_resume_breakpoint_at_frame): Move prototype earlier.
(insert_step_resume_breakpoint_at_caller): Likewise.
(insert_step_resume_breakpoint_at_sal): Likewise.
testsuite/ChangeLog:
* gdb.base/annota1.exp: Accept breakpoints-invalid annotation
while delivering signal.
Index: gdb/infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.467
diff -u -p -r1.467 infrun.c
--- gdb/infrun.c 9 Jan 2011 03:08:56 -0000 1.467
+++ gdb/infrun.c 17 Jan 2011 19:15:05 -0000
@@ -99,6 +97,14 @@ void _initialize_infrun (void);
void nullify_last_target_wait_ptid (void);
+static void insert_step_resume_breakpoint_at_frame (struct frame_info *);
+
+static void insert_step_resume_breakpoint_at_caller (struct frame_info *);
+
+static void insert_step_resume_breakpoint_at_sal (struct gdbarch *,
+ struct symtab_and_line ,
+ struct frame_id);
+
/* When set, stop the 'step' command if we enter a function which has
no line number information. The normal behavior is that we step
over such function. */
@@ -1666,6 +1672,27 @@ a command like `return' or `jump' to con
displaced->step_closure);
}
+ /* Stepping over a breakpoint while at the same time delivering a signal
+ has a problem: we cannot use displaced stepping (see above), but we
+ also cannot use software single-stepping, because we do not know
+ where execution will continue if a signal handler is installed.
+
+ On the other hand, if there is a signal handler we'd have to step
+ over it anyway. So what we do instead is to install a step-resume
+ handler at the current address right away, deliver the signal without
+ stepping (which means we may have to re-insert breakpoints), and once
+ we arrive back at the step-resume breakpoint, step once more over the
+ original breakpoint we wanted to step over. */
+ else if (step && tp->control.trap_expected && sig != TARGET_SIGNAL_0
+ && execution_direction == EXEC_FORWARD)
+ {
+ insert_step_resume_breakpoint_at_frame (get_current_frame ());
+ insert_breakpoints ();
+ tp->step_after_step_resume_breakpoint = 1;
+ tp->control.trap_expected = 0;
+ step = 0;
+ }
+
/* Do we need to do it the hard way, w/temp breakpoints? */
else if (step)
step = maybe_software_singlestep (gdbarch, pc);
@@ -2232,11 +2259,6 @@ static void handle_step_into_function (s
struct execution_control_state *ecs);
static void handle_step_into_function_backward (struct gdbarch *gdbarch,
struct execution_control_state *ecs);
-static void insert_step_resume_breakpoint_at_frame (struct frame_info *);
-static void insert_step_resume_breakpoint_at_caller (struct frame_info *);
-static void insert_step_resume_breakpoint_at_sal (struct gdbarch *,
- struct symtab_and_line ,
- struct frame_id);
static void insert_longjmp_resume_breakpoint (struct gdbarch *, CORE_ADDR);
static void check_exception_resume (struct execution_control_state *,
struct frame_info *, struct symbol *);
@@ -4060,7 +4082,8 @@ process_event_stop_test:
Instead this signal arrives. This signal will take us out
of the stepping range so GDB needs to remember to, when
the signal handler returns, resume stepping off that
- breakpoint. */
+ breakpoint. This happens in resume, so we simply need
+ to keep stepping here. */
/* To simplify things, "continue" is forced to use the same
code paths as single-step - set a breakpoint at the
signal return address and then, once hit, step off that
@@ -4070,8 +4093,7 @@ process_event_stop_test:
"infrun: signal arrived while stepping over "
"breakpoint\n");
- insert_step_resume_breakpoint_at_frame (frame);
- ecs->event_thread->step_after_step_resume_breakpoint = 1;
+ ecs->event_thread->stepping_over_breakpoint = 1;
keep_going (ecs);
return;
}
Index: gdb/testsuite/gdb.base/annota1.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/annota1.exp,v
retrieving revision 1.40
diff -u -p -r1.40 annota1.exp
--- gdb/testsuite/gdb.base/annota1.exp 1 Jan 2011 15:33:40 -0000 1.40
+++ gdb/testsuite/gdb.base/annota1.exp 17 Jan 2011 19:15:13 -0000
@@ -266,10 +266,10 @@ if [target_info exists gdb,nosignals] {
unsupported "backtrace @ signal handler"
} else {
gdb_test_multiple "signal SIGUSR1" "send SIGUSR1" {
- -re "\r\n\032\032post-prompt\r\nContinuing with signal SIGUSR1.\r\n\r\n\032\032starting\(\r\n\r\n\032\032frames-invalid\)+\r\n\r\n\032\032breakpoint 2\r\n\r\nBreakpoint 2, \r\n\032\032frame-begin 0 $hex\r\n\r\n\032\032frame-function-name\r\nhandle_USR1\r\n\032\032frame-args\r\n \\(\r\n\032\032arg-begin\r\nsig\r\n\032\032arg-name-end\r\n=\r\n\032\032arg-value -\r\n$decimal\r\n\032\032arg-end\r\n\\)\r\n\032\032frame-source-begin\r\n at \r\n\032\032frame-source-file\r\n${escapedsrcfile}\r\n\032\032frame-source-file-end\r\n:\r\n\032\032frame-source-line\r\n.*\r\n\032\032frame-source-end\r\n\r\n\r\n\032\032source.*annota1.c:.*:.*:beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n$gdb_prompt$" {
+ -re "\r\n\032\032post-prompt\r\nContinuing with signal SIGUSR1.\r\n\r\n\032\032starting\(\(\r\n\r\n\032\032frames-invalid\)|\(\r\n\r\n\032\032breakpoints-invalid\)\)+\r\n\r\n\032\032breakpoint 2\r\n\r\nBreakpoint 2, \r\n\032\032frame-begin 0 $hex\r\n\r\n\032\032frame-function-name\r\nhandle_USR1\r\n\032\032frame-args\r\n \\(\r\n\032\032arg-begin\r\nsig\r\n\032\032arg-name-end\r\n=\r\n\032\032arg-value -\r\n$decimal\r\n\032\032arg-end\r\n\\)\r\n\032\032frame-source-begin\r\n at \r\n\032\032frame-source-file\r\n${escapedsrcfile}\r\n\032\032frame-source-file-end\r\n:\r\n\032\032frame-source-line\r\n.*\r\n\032\032frame-source-end\r\n\r\n\r\n\032\032source.*annota1.c:.*:.*:beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n$gdb_prompt$" {
pass "send SIGUSR1"
}
- -re "\r\n\032\032post-prompt\r\nContinuing with signal SIGUSR1.\r\n\r\n\032\032starting\(\r\n\r\n\032\032frames-invalid\)+\r\n\r\n\032\032breakpoint 2\r\n\r\nBreakpoint 2, \r\n\032\032frame-begin 0 $hex\r\n\r\n\032\032frame-function-name\r\nhandle_USR1\r\n\032\032frame-args\r\n \\(\r\n\032\032arg-begin\r\nsig\r\n\032\032arg-name-end\r\n=\r\n\032\032arg-value -\r\n$decimal\r\n\032\032arg-end\r\n\\)\r\n\032\032frame-source-begin\r\n at \r\n\032\032frame-source-file\r\n.*${srcfile}\r\n\032\032frame-source-file-end\r\n:\r\n\032\032frame-source-line\r\n.*\r\n\032\032frame-source-end\r\n\r\n\r\n\032\032source.*annota1.c:.*:.*:beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n$gdb_prompt$" {
+ -re "\r\n\032\032post-prompt\r\nContinuing with signal SIGUSR1.\r\n\r\n\032\032starting\(\(\r\n\r\n\032\032frames-invalid\)|\(\r\n\r\n\032\032breakpoints-invalid\)\)+\r\n\r\n\032\032breakpoint 2\r\n\r\nBreakpoint 2, \r\n\032\032frame-begin 0 $hex\r\n\r\n\032\032frame-function-name\r\nhandle_USR1\r\n\032\032frame-args\r\n \\(\r\n\032\032arg-begin\r\nsig\r\n\032\032arg-name-end\r\n=\r\n\032\032arg-value -\r\n$decimal\r\n\032\032arg-end\r\n\\)\r\n\032\032frame-source-begin\r\n at \r\n\032\032frame-source-file\r\n.*${srcfile}\r\n\032\032frame-source-file-end\r\n:\r\n\032\032frame-source-line\r\n.*\r\n\032\032frame-source-end\r\n\r\n\r\n\032\032source.*annota1.c:.*:.*:beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n$gdb_prompt$" {
setup_xfail "*-*-*" 1270
fail "send SIGUSR1"
}
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* [commit] Re: [rfc][1/2] Signal delivery + software single-step is broken
2011-01-19 16:43 [rfc][1/2] Signal delivery + software single-step is broken Ulrich Weigand
@ 2011-04-27 17:17 ` Ulrich Weigand
2011-04-27 18:15 ` Pedro Alves
0 siblings, 1 reply; 10+ messages in thread
From: Ulrich Weigand @ 2011-04-27 17:17 UTC (permalink / raw)
To: gdb-patches
Hello,
now that the difficult part is fixed, I've updated this patch as well.
> we're seeing failing testcases related to signal delivery on ARM,
> which turn out to be related to software single-stepping.
> This fixes the following set of test cases on ARM:
> FAIL: gdb.base/annota1.exp: send SIGUSR1 (timeout)
> FAIL: gdb.base/annota1.exp: backtrace @ signal handler (timeout)
> FAIL: gdb.base/annota3.exp: send SIGUSR1 (pattern 4)
> FAIL: gdb.base/annota3.exp: backtrace @ signal handler (pattern 1)
Since the sigstep.exp cases are already completely fixed by
http://sourceware.org/ml/gdb-patches/2011-03/msg01223.html
this part of the patch, which is still needed to fix signal delivery
done directly by GDB due to "pass" commands, can actually be simplified
a bit. This version is now implemented in proceed instead of resume,
which means it doesn't have to undo things in resume that were already
done in proceed. Otherwise, the logic is exactly the same.
Retested on armv7l-unknown-linux-gnueabi and i386-linux.
Committed to mainline.
Bye,
Ulrich
ChangeLog:
* infrun.c (proceed): Do not single-step into signal delivery
when stepping off a breakpoint location.
(insert_step_resume_breakpoint_at_frame): Move prototype earlier.
(insert_step_resume_breakpoint_at_caller): Likewise.
(insert_step_resume_breakpoint_at_sal): Likewise.
(insert_longjmp_resume_breakpoint): Likewise.
testsuite/ChangeLog:
* gdb.base/annota1.exp: Accept breakpoints-invalid annotation
while delivering signal.
Index: gdb/infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.475
diff -u -p -r1.475 infrun.c
--- gdb/infrun.c 27 Apr 2011 13:29:13 -0000 1.475
+++ gdb/infrun.c 27 Apr 2011 16:08:44 -0000
@@ -99,6 +99,16 @@ void _initialize_infrun (void);
void nullify_last_target_wait_ptid (void);
+static void insert_step_resume_breakpoint_at_frame (struct frame_info *);
+
+static void insert_step_resume_breakpoint_at_caller (struct frame_info *);
+
+static void insert_step_resume_breakpoint_at_sal (struct gdbarch *,
+ struct symtab_and_line ,
+ struct frame_id);
+
+static void insert_longjmp_resume_breakpoint (struct gdbarch *, CORE_ADDR);
+
/* When set, stop the 'step' command if we enter a function which has
no line number information. The normal behavior is that we step
over such function. */
@@ -2054,24 +2064,6 @@ proceed (CORE_ADDR addr, enum target_sig
/* prepare_to_proceed may change the current thread. */
tp = inferior_thread ();
- if (oneproc)
- {
- tp->control.trap_expected = 1;
- /* If displaced stepping is enabled, we can step over the
- breakpoint without hitting it, so leave all breakpoints
- inserted. Otherwise we need to disable all breakpoints, step
- one instruction, and then re-add them when that step is
- finished. */
- if (!use_displaced_stepping (gdbarch))
- remove_breakpoints ();
- }
-
- /* We can insert breakpoints if we're not trying to step over one,
- or if we are stepping over one but we're using displaced stepping
- to do so. */
- if (! tp->control.trap_expected || use_displaced_stepping (gdbarch))
- insert_breakpoints ();
-
if (!non_stop)
{
/* Pass the last stop signal to the thread we're resuming,
@@ -2141,6 +2133,42 @@ proceed (CORE_ADDR addr, enum target_sig
/* Reset to normal state. */
init_infwait_state ();
+ /* Stepping over a breakpoint while at the same time delivering a signal
+ has a problem: we cannot use displaced stepping, but we also cannot
+ use software single-stepping, because we do not know where execution
+ will continue if a signal handler is installed.
+
+ On the other hand, if there is a signal handler we'd have to step
+ over it anyway. So what we do instead is to install a step-resume
+ handler at the current address right away, deliver the signal without
+ stepping, and once we arrive back at the step-resume breakpoint, step
+ once more over the original breakpoint we wanted to step over. */
+ if (oneproc && tp->suspend.stop_signal != TARGET_SIGNAL_0
+ && execution_direction != EXEC_REVERSE)
+ {
+ insert_step_resume_breakpoint_at_frame (get_current_frame ());
+ tp->step_after_step_resume_breakpoint = 1;
+ oneproc = 0;
+ }
+
+ if (oneproc)
+ {
+ tp->control.trap_expected = 1;
+ /* If displaced stepping is enabled, we can step over the
+ breakpoint without hitting it, so leave all breakpoints
+ inserted. Otherwise we need to disable all breakpoints, step
+ one instruction, and then re-add them when that step is
+ finished. */
+ if (!use_displaced_stepping (gdbarch))
+ remove_breakpoints ();
+ }
+
+ /* We can insert breakpoints if we're not trying to step over one,
+ or if we are stepping over one but we're using displaced stepping
+ to do so. */
+ if (! tp->control.trap_expected || use_displaced_stepping (gdbarch))
+ insert_breakpoints ();
+
/* Resume inferior. */
resume (oneproc || step || bpstat_should_step (), tp->suspend.stop_signal);
@@ -2257,12 +2285,6 @@ static void handle_step_into_function (s
struct execution_control_state *ecs);
static void handle_step_into_function_backward (struct gdbarch *gdbarch,
struct execution_control_state *ecs);
-static void insert_step_resume_breakpoint_at_frame (struct frame_info *);
-static void insert_step_resume_breakpoint_at_caller (struct frame_info *);
-static void insert_step_resume_breakpoint_at_sal (struct gdbarch *,
- struct symtab_and_line ,
- struct frame_id);
-static void insert_longjmp_resume_breakpoint (struct gdbarch *, CORE_ADDR);
static void check_exception_resume (struct execution_control_state *,
struct frame_info *, struct symbol *);
Index: gdb/testsuite/gdb.base/annota1.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/annota1.exp,v
retrieving revision 1.40
diff -u -p -r1.40 annota1.exp
--- gdb/testsuite/gdb.base/annota1.exp 1 Jan 2011 15:33:40 -0000 1.40
+++ gdb/testsuite/gdb.base/annota1.exp 27 Apr 2011 16:08:45 -0000
@@ -266,10 +266,10 @@ if [target_info exists gdb,nosignals] {
unsupported "backtrace @ signal handler"
} else {
gdb_test_multiple "signal SIGUSR1" "send SIGUSR1" {
- -re "\r\n\032\032post-prompt\r\nContinuing with signal SIGUSR1.\r\n\r\n\032\032starting\(\r\n\r\n\032\032frames-invalid\)+\r\n\r\n\032\032breakpoint 2\r\n\r\nBreakpoint 2, \r\n\032\032frame-begin 0 $hex\r\n\r\n\032\032frame-function-name\r\nhandle_USR1\r\n\032\032frame-args\r\n \\(\r\n\032\032arg-begin\r\nsig\r\n\032\032arg-name-end\r\n=\r\n\032\032arg-value -\r\n$decimal\r\n\032\032arg-end\r\n\\)\r\n\032\032frame-source-begin\r\n at \r\n\032\032frame-source-file\r\n${escapedsrcfile}\r\n\032\032frame-source-file-end\r\n:\r\n\032\032frame-source-line\r\n.*\r\n\032\032frame-source-end\r\n\r\n\r\n\032\032source.*annota1.c:.*:.*:beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n$gdb_prompt$" {
+ -re "\r\n\032\032post-prompt\r\nContinuing with signal SIGUSR1.\r\n\r\n\032\032starting\(\(\r\n\r\n\032\032frames-invalid\)|\(\r\n\r\n\032\032breakpoints-invalid\)\)+\r\n\r\n\032\032breakpoint 2\r\n\r\nBreakpoint 2, \r\n\032\032frame-begin 0 $hex\r\n\r\n\032\032frame-function-name\r\nhandle_USR1\r\n\032\032frame-args\r\n \\(\r\n\032\032arg-begin\r\nsig\r\n\032\032arg-name-end\r\n=\r\n\032\032arg-value -\r\n$decimal\r\n\032\032arg-end\r\n\\)\r\n\032\032frame-source-begin\r\n at \r\n\032\032frame-source-file\r\n${escapedsrcfile}\r\n\032\032frame-source-file-end\r\n:\r\n\032\032frame-source-line\r\n.*\r\n\032\032frame-source-end\r\n\r\n\r\n\032\032source.*annota1.c:.*:.*:beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n$gdb_prompt$" {
pass "send SIGUSR1"
}
- -re "\r\n\032\032post-prompt\r\nContinuing with signal SIGUSR1.\r\n\r\n\032\032starting\(\r\n\r\n\032\032frames-invalid\)+\r\n\r\n\032\032breakpoint 2\r\n\r\nBreakpoint 2, \r\n\032\032frame-begin 0 $hex\r\n\r\n\032\032frame-function-name\r\nhandle_USR1\r\n\032\032frame-args\r\n \\(\r\n\032\032arg-begin\r\nsig\r\n\032\032arg-name-end\r\n=\r\n\032\032arg-value -\r\n$decimal\r\n\032\032arg-end\r\n\\)\r\n\032\032frame-source-begin\r\n at \r\n\032\032frame-source-file\r\n.*${srcfile}\r\n\032\032frame-source-file-end\r\n:\r\n\032\032frame-source-line\r\n.*\r\n\032\032frame-source-end\r\n\r\n\r\n\032\032source.*annota1.c:.*:.*:beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n$gdb_prompt$" {
+ -re "\r\n\032\032post-prompt\r\nContinuing with signal SIGUSR1.\r\n\r\n\032\032starting\(\(\r\n\r\n\032\032frames-invalid\)|\(\r\n\r\n\032\032breakpoints-invalid\)\)+\r\n\r\n\032\032breakpoint 2\r\n\r\nBreakpoint 2, \r\n\032\032frame-begin 0 $hex\r\n\r\n\032\032frame-function-name\r\nhandle_USR1\r\n\032\032frame-args\r\n \\(\r\n\032\032arg-begin\r\nsig\r\n\032\032arg-name-end\r\n=\r\n\032\032arg-value -\r\n$decimal\r\n\032\032arg-end\r\n\\)\r\n\032\032frame-source-begin\r\n at \r\n\032\032frame-source-file\r\n.*${srcfile}\r\n\032\032frame-source-file-end\r\n:\r\n\032\032frame-source-line\r\n.*\r\n\032\032frame-source-end\r\n\r\n\r\n\032\032source.*annota1.c:.*:.*:beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n$gdb_prompt$" {
setup_xfail "*-*-*" 1270
fail "send SIGUSR1"
}
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [commit] Re: [rfc][1/2] Signal delivery + software single-step is broken
2011-04-27 17:17 ` [commit] " Ulrich Weigand
@ 2011-04-27 18:15 ` Pedro Alves
2011-04-27 19:12 ` Ulrich Weigand
0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2011-04-27 18:15 UTC (permalink / raw)
To: gdb-patches; +Cc: Ulrich Weigand
On Wednesday 27 April 2011 18:17:25, Ulrich Weigand wrote:
> + /* Stepping over a breakpoint while at the same time delivering a signal
> + has a problem: we cannot use displaced stepping, but we also cannot
> + use software single-stepping, because we do not know where execution
> + will continue if a signal handler is installed.
> +
> + On the other hand, if there is a signal handler we'd have to step
> + over it anyway. So what we do instead is to install a step-resume
> + handler at the current address right away, deliver the signal without
> + stepping, and once we arrive back at the step-resume breakpoint, step
> + once more over the original breakpoint we wanted to step over. */
> + if (oneproc && tp->suspend.stop_signal != TARGET_SIGNAL_0
> + && execution_direction != EXEC_REVERSE)
> + {
> + insert_step_resume_breakpoint_at_frame (get_current_frame ());
> + tp->step_after_step_resume_breakpoint = 1;
> + oneproc = 0;
> + }
This unfortunately somewhat breaks hardware-step archs:
(the below is a simple program that writes to 0, and has a
SIGSEGV handler installed)
Program received signal SIGSEGV, Segmentation fault.
0x00000000004008ca in main2 () at siginfo.c:104
104 *(int *)p = 0;
(gdb) b
Breakpoint 1 at 0x4008ca: file siginfo.c, line 104.
(gdb) si
Program received signal SIGTRAP, Trace/breakpoint trap.
handler (sig=0, info=0xffffefbd5a360000, context=0xffffffffb4400000) at siginfo.c:69
69 {
(gdb)
Same with debug output (next, step or stepi, same thing):
(gdb) set debug infrun 1
(gdb) s
infrun: clear_proceed_status_thread (Thread 0x7ffff7fd5700 (LWP 18800))
infrun: proceed (addr=0xffffffffffffffff, signal=144, step=1)
infrun: inserting step-resume breakpoint at 0x4008ca
infrun: resume (step=1, signal=11), trap_expected=0
infrun: wait_for_inferior (treat_exec_as_sigtrap=0)
infrun: target_wait (-1, status) =
infrun: 18800 [Thread 0x7ffff7fd5700 (LWP 18800)],
infrun: status->kind = stopped, signal = SIGTRAP
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_STOPPED
infrun: stop_pc = 0x4007a7
infrun: random signal 5
Program received signal SIGTRAP, Trace/breakpoint trap.
infrun: stop_stepping
handler (sig=0, info=0xffffefbd5a360000, context=0xffffffffb4400000) at siginfo.c:69
69 {
(gdb)
We'd previously step into the installed handler without
that spurious SIGTRAP:
Program received signal SIGSEGV, Segmentation fault.
infrun: stop_stepping
0x00000000004008ca in main2 () at siginfo.c:104
104 *(int *)p = 0;
(gdb) b
Breakpoint 2 at 0x4008ca: file siginfo.c, line 104.
(gdb) s
infrun: clear_proceed_status_thread (Thread 0x7ffff7fd5700 (LWP 18970))
infrun: proceed (addr=0xffffffffffffffff, signal=144, step=1)
infrun: resume (step=1, signal=11), trap_expected=1
infrun: wait_for_inferior (treat_exec_as_sigtrap=0)
infrun: target_wait (-1, status) =
infrun: 18970 [Thread 0x7ffff7fd5700 (LWP 18970)],
infrun: status->kind = stopped, signal = SIGTRAP
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_STOPPED
infrun: stop_pc = 0x4007a7
infrun: stepped to a different line
infrun: stop_stepping
handler (sig=32767, info=0xffffffffbe600000, context=0x7ffff7bb8b40) at siginfo.c:69
69 {
(gdb)
--
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [commit] Re: [rfc][1/2] Signal delivery + software single-step is broken
2011-04-27 18:15 ` Pedro Alves
@ 2011-04-27 19:12 ` Ulrich Weigand
2011-04-27 19:44 ` Pedro Alves
0 siblings, 1 reply; 10+ messages in thread
From: Ulrich Weigand @ 2011-04-27 19:12 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves wrote:
> On Wednesday 27 April 2011 18:17:25, Ulrich Weigand wrote:
> > + /* Stepping over a breakpoint while at the same time delivering a signal
> > + has a problem: we cannot use displaced stepping, but we also cannot
> > + use software single-stepping, because we do not know where execution
> > + will continue if a signal handler is installed.
> > +
> > + On the other hand, if there is a signal handler we'd have to step
> > + over it anyway. So what we do instead is to install a step-resume
> > + handler at the current address right away, deliver the signal without
> > + stepping, and once we arrive back at the step-resume breakpoint, step
> > + once more over the original breakpoint we wanted to step over. */
> > + if (oneproc && tp->suspend.stop_signal != TARGET_SIGNAL_0
> > + && execution_direction != EXEC_REVERSE)
> > + {
> > + insert_step_resume_breakpoint_at_frame (get_current_frame ());
> > + tp->step_after_step_resume_breakpoint = 1;
> > + oneproc = 0;
> > + }
>
> This unfortunately somewhat breaks hardware-step archs:
>
> (the below is a simple program that writes to 0, and has a
> SIGSEGV handler installed)
>
> Program received signal SIGSEGV, Segmentation fault.
> 0x00000000004008ca in main2 () at siginfo.c:104
> 104 *(int *)p = 0;
> (gdb) b
> Breakpoint 1 at 0x4008ca: file siginfo.c, line 104.
> (gdb) si
That's an interesting test case :-)
The problem is that if a step-resume breakpoint is installed,
handle_inferior_event will never expect hardware single-step
traps (unless a software watchpoint is also in effect), because
it assumes nobody ever uses a step-resume breakpoint and then
hardware single-steps as well (because currently_stepping
returns 0 in that case).
The simple way to fix the inconsistency would be to just add a
step = 0;
line to the above if. Of course this changes the behaviour for
hardware single-step platforms: they now would not step into
the signal handler in this case (just like software single-step
platforms don't).
On the one hand, this adds consistency: both types of platforms
behave the same. (On software single-step platforms, it seems
we simply cannot step into the handler, because we can't find
it.)
On the other hand, in some sense this reduces functionality.
So one could try to treat the two platforms separately, and
have hardware single-step platforms step into the handler,
even while software single-step platforms don't.
[ This would probably mean to move support back into resume,
because we'd want to do that only if we actually use software
single-step for this particular step. ]
Thoughts? Which behaviour would you prefer?
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [commit] Re: [rfc][1/2] Signal delivery + software single-step is broken
2011-04-27 19:12 ` Ulrich Weigand
@ 2011-04-27 19:44 ` Pedro Alves
2011-04-28 8:55 ` [patch] " Ulrich Weigand
0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2011-04-27 19:44 UTC (permalink / raw)
To: gdb-patches; +Cc: Ulrich Weigand
On Wednesday 27 April 2011 20:11:53, Ulrich Weigand wrote:
> :-)
> The problem is that if a step-resume breakpoint is installed,
> handle_inferior_event will never expect hardware single-step
> traps (unless a software watchpoint is also in effect), because
> it assumes nobody ever uses a step-resume breakpoint and then
> hardware single-steps as well (because currently_stepping
> returns 0 in that case).
>
> The simple way to fix the inconsistency would be to just add a
> step = 0;
> line to the above if. Of course this changes the behaviour for
> hardware single-step platforms: they now would not step into
> the signal handler in this case (just like software single-step
> platforms don't).
>
> On the one hand, this adds consistency: both types of platforms
> behave the same.
IIUC, with that change alone, the behavior on hw-step archs
would change depending on there being a breakpoint at PC or
not. Without a breakpoint, you'd step into the handler, with
a breakpoint, you'd not.
> (On software single-step platforms, it seems
> we simply cannot step into the handler, because we can't find
> it.)
IMO, this could/should be fixed with kernel help.
>
> On the other hand, in some sense this reduces functionality.
> So one could try to treat the two platforms separately, and
> have hardware single-step platforms step into the handler,
> even while software single-step platforms don't.
>
> [ This would probably mean to move support back into resume,
> because we'd want to do that only if we actually use software
> single-step for this particular step. ]
>
> Thoughts? Which behaviour would you prefer?
I'd prefer to keep the possibility to step into a handler.
I find this very useful when you don't actually know what
handler is installed. Much easier than grepping for "signal" and
trying to guess what is installed (which may even have been done
by some external dependency / library).
--
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* [patch] Re: [commit] Re: [rfc][1/2] Signal delivery + software single-step is broken
2011-04-27 19:44 ` Pedro Alves
@ 2011-04-28 8:55 ` Ulrich Weigand
2011-04-28 12:01 ` Pedro Alves
0 siblings, 1 reply; 10+ messages in thread
From: Ulrich Weigand @ 2011-04-28 8:55 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves wrote:
> On Wednesday 27 April 2011 20:11:53, Ulrich Weigand wrote:
> > The simple way to fix the inconsistency would be to just add a
> > step = 0;
> > line to the above if. Of course this changes the behaviour for
> > hardware single-step platforms: they now would not step into
> > the signal handler in this case (just like software single-step
> > platforms don't).
> >
> > On the one hand, this adds consistency: both types of platforms
> > behave the same.
>
> IIUC, with that change alone, the behavior on hw-step archs
> would change depending on there being a breakpoint at PC or
> not. Without a breakpoint, you'd step into the handler, with
> a breakpoint, you'd not.
Good point. That does not seem desirable.
> I'd prefer to keep the possibility to step into a handler.
> I find this very useful when you don't actually know what
> handler is installed. Much easier than grepping for "signal" and
> trying to guess what is installed (which may even have been done
> by some external dependency / library).
OK. The patch below should restore the prior behavior on
hardware single-step targets, and strictly improve the situation
on software single-step targets (even though of course it still
is not quite the same as with hardware single-step). I've also
attempted to better document the situation in a comment ...
Does this seem OK to you?
Bye,
Ulrich
ChangeLog:
* infrun.c (proceed): Revert previous change.
(resume): Instead, handle the case of signal delivery while stepping
off a breakpoint location here, and only if software single-stepping
is used.
Index: gdb/infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.476
diff -u -p -r1.476 infrun.c
--- gdb/infrun.c 27 Apr 2011 17:08:41 -0000 1.476
+++ gdb/infrun.c 28 Apr 2011 08:29:36 -0000
@@ -1703,6 +1703,42 @@ a command like `return' or `jump' to con
else if (step)
step = maybe_software_singlestep (gdbarch, pc);
+ /* Currently, our software single-step implementation leads to different
+ results than hardware single-stepping in one situation: when stepping
+ into delivering a signal which has an associated signal handler,
+ hardware single-step will stop at the first instruction of the handler,
+ while software single-step will simply skip execution of the handler.
+
+ For now, this difference in behavior is accepted since there is no
+ easy way to actually implement single-stepping into a signal handler
+ without kernel support.
+
+ However, there is one scenario where this difference leads to follow-on
+ problems: if we're stepping off a breakpoint by removing all breakpoints
+ and then single-stepping. In this case, the software single-step
+ behavior means that even if there is a *breakpoint* in the signal
+ handler, GDB still would not stop.
+
+ Fortunately, we can at least fix this particular issue. We detect
+ here the case where we are about to deliver a signal while software
+ single-stepping with breakpoints removed. In this situation, we
+ revert the decisions to remove all breakpoints and insert single-
+ step breakpoints, and instead we install a step-resume breakpoint
+ at the current address, deliver the signal without stepping, and
+ once we arrive back at the step-resume breakpoint, actually step
+ over the breakpoint we originally wanted to step over. */
+ if (singlestep_breakpoints_inserted_p
+ && tp->control.trap_expected && sig != TARGET_SIGNAL_0)
+ {
+ remove_single_step_breakpoints ();
+ singlestep_breakpoints_inserted_p = 0;
+ tp->control.trap_expected = 0;
+
+ insert_step_resume_breakpoint_at_frame (get_current_frame ());
+ insert_breakpoints ();
+ tp->step_after_step_resume_breakpoint = 1;
+ }
+
if (should_resume)
{
ptid_t resume_ptid;
@@ -2064,6 +2100,24 @@ proceed (CORE_ADDR addr, enum target_sig
/* prepare_to_proceed may change the current thread. */
tp = inferior_thread ();
+ if (oneproc)
+ {
+ tp->control.trap_expected = 1;
+ /* If displaced stepping is enabled, we can step over the
+ breakpoint without hitting it, so leave all breakpoints
+ inserted. Otherwise we need to disable all breakpoints, step
+ one instruction, and then re-add them when that step is
+ finished. */
+ if (!use_displaced_stepping (gdbarch))
+ remove_breakpoints ();
+ }
+
+ /* We can insert breakpoints if we're not trying to step over one,
+ or if we are stepping over one but we're using displaced stepping
+ to do so. */
+ if (! tp->control.trap_expected || use_displaced_stepping (gdbarch))
+ insert_breakpoints ();
+
if (!non_stop)
{
/* Pass the last stop signal to the thread we're resuming,
@@ -2133,42 +2187,6 @@ proceed (CORE_ADDR addr, enum target_sig
/* Reset to normal state. */
init_infwait_state ();
- /* Stepping over a breakpoint while at the same time delivering a signal
- has a problem: we cannot use displaced stepping, but we also cannot
- use software single-stepping, because we do not know where execution
- will continue if a signal handler is installed.
-
- On the other hand, if there is a signal handler we'd have to step
- over it anyway. So what we do instead is to install a step-resume
- handler at the current address right away, deliver the signal without
- stepping, and once we arrive back at the step-resume breakpoint, step
- once more over the original breakpoint we wanted to step over. */
- if (oneproc && tp->suspend.stop_signal != TARGET_SIGNAL_0
- && execution_direction != EXEC_REVERSE)
- {
- insert_step_resume_breakpoint_at_frame (get_current_frame ());
- tp->step_after_step_resume_breakpoint = 1;
- oneproc = 0;
- }
-
- if (oneproc)
- {
- tp->control.trap_expected = 1;
- /* If displaced stepping is enabled, we can step over the
- breakpoint without hitting it, so leave all breakpoints
- inserted. Otherwise we need to disable all breakpoints, step
- one instruction, and then re-add them when that step is
- finished. */
- if (!use_displaced_stepping (gdbarch))
- remove_breakpoints ();
- }
-
- /* We can insert breakpoints if we're not trying to step over one,
- or if we are stepping over one but we're using displaced stepping
- to do so. */
- if (! tp->control.trap_expected || use_displaced_stepping (gdbarch))
- insert_breakpoints ();
-
/* Resume inferior. */
resume (oneproc || step || bpstat_should_step (), tp->suspend.stop_signal);
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] Re: [commit] Re: [rfc][1/2] Signal delivery + software single-step is broken
2011-04-28 8:55 ` [patch] " Ulrich Weigand
@ 2011-04-28 12:01 ` Pedro Alves
2011-04-28 15:18 ` [patch v2] " Ulrich Weigand
0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2011-04-28 12:01 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
On Thursday 28 April 2011 09:54:39, Ulrich Weigand wrote:
> Index: gdb/infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.476
> diff -u -p -r1.476 infrun.c
> --- gdb/infrun.c 27 Apr 2011 17:08:41 -0000 1.476
> +++ gdb/infrun.c 28 Apr 2011 08:29:36 -0000
> @@ -1703,6 +1703,42 @@ a command like `return' or `jump' to con
> else if (step)
> step = maybe_software_singlestep (gdbarch, pc);
>
> + /* Currently, our software single-step implementation leads to different
> + results than hardware single-stepping in one situation: when stepping
> + into delivering a signal which has an associated signal handler,
> + hardware single-step will stop at the first instruction of the handler,
> + while software single-step will simply skip execution of the handler.
> +
> + For now, this difference in behavior is accepted since there is no
> + easy way to actually implement single-stepping into a signal handler
> + without kernel support.
> +
> + However, there is one scenario where this difference leads to follow-on
> + problems: if we're stepping off a breakpoint by removing all breakpoints
> + and then single-stepping. In this case, the software single-step
> + behavior means that even if there is a breakpoint in the signal
> + handler, GDB still would not stop.
> +
> + Fortunately, we can at least fix this particular issue. We detect
> + here the case where we are about to deliver a signal while software
> + single-stepping with breakpoints removed. In this situation, we
> + revert the decisions to remove all breakpoints and insert single-
> + step breakpoints, and instead we install a step-resume breakpoint
> + at the current address, deliver the signal without stepping, and
> + once we arrive back at the step-resume breakpoint, actually step
> + over the breakpoint we originally wanted to step over. */
> + if (singlestep_breakpoints_inserted_p
> + && tp->control.trap_expected && sig != TARGET_SIGNAL_0)
> + {
> + remove_single_step_breakpoints ();
> + singlestep_breakpoints_inserted_p = 0;
> + tp->control.trap_expected = 0;
> +
> + insert_step_resume_breakpoint_at_frame (get_current_frame ());
> + insert_breakpoints ();
> + tp->step_after_step_resume_breakpoint = 1;
> + }
> +
(I wish there was no need to undo stuff, and do the decision
before actually inserting sss breakpoints, but I see why you
did it. We need to call gdbarch_software_single_step to
know whether sss breakpoints will be inserted, which, actually
inserts them.)
I'm trying to think whether this works okay with nested
signals. Consider that you had a signal to deliver while
stepping over a breakpoint, and you hit that new code.
Now, the signal handler runs with breakpoint inserted, and
happens to hit a breakpoint that shouldn't cause a stop and
so needs stepping over immediately (e.g., "b foo if 0"). This sets
stepping_over_breakpoint=1, and keeps going. Say an asynchronous pass/nostop
signal happens while trying to step over that breakpoint, and we
see it before the "step" finishes. While handling it, we end up in keep_going,
here, because we already had a step-resume breakpoint set, I think:
/* Note: step_resume_breakpoint may be non-NULL. This occures
when either there's a nested signal, or when there's a
pending signal enabled just as the signal handler returns
(leaving the inferior at the step-resume-breakpoint without
actually executing it). Either way continue until the
breakpoint is really hit. */
keep_going (ecs);
return;
}
eventually re-reaching resume with trap_expected set, sss breakpoints
inserted, and with a signal to deliver.
Since you'll already have one step-resume breakpoint set (back in the
main code), and you can't set another -- you'd hit the
assert in insert_step_resume_breakpoint_at_sal trying to insert
a second one.
But I'm not certain I'm guessing the flow correctly on software-step
archs.
Here's a small test that nests SIGSEGV handlers. Setting a breakpoint
at the *(int *)p lines like I mentioned above should do the trick.
#include <string.h>
#include <signal.h>
#include <unistd.h>
static void *p;
static void
handler (int sig, siginfo_t *info, void *context)
{
/* Trigger a nested SIGSEGV. */
*(int *)p = 0;
_exit (0);
}
int
main (void)
{
/* Set up the signal handler. */
struct sigaction action;
memset (&action, 0, sizeof (action));
action.sa_sigaction = handler;
action.sa_flags |= SA_SIGINFO | SA_NODEFER;
if (sigaction (SIGSEGV, &action, NULL))
{
perror ("sigaction");
return 1;
}
/* Trigger SIGSEGV. */
*(int *)p = 0;
return 0;
}
--
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* [patch v2] Re: [commit] Re: [rfc][1/2] Signal delivery + software single-step is broken
2011-04-28 12:01 ` Pedro Alves
@ 2011-04-28 15:18 ` Ulrich Weigand
2011-04-28 15:46 ` Pedro Alves
0 siblings, 1 reply; 10+ messages in thread
From: Ulrich Weigand @ 2011-04-28 15:18 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves wrote:
> (I wish there was no need to undo stuff, and do the decision
> before actually inserting sss breakpoints, but I see why you
> did it. We need to call gdbarch_software_single_step to
> know whether sss breakpoints will be inserted, which, actually
> inserts them.)
Fully agreed ...
> I'm trying to think whether this works okay with nested
> signals.
[...]
> Since you'll already have one step-resume breakpoint set (back in the
> main code), and you can't set another -- you'd hit the
> assert in insert_step_resume_breakpoint_at_sal trying to insert
> a second one.
Hmm, good point. Indeed I was able to trigger that assert.
However, this is easily fixable just the same as in handle_inferior_event:
if we already have a step-resume breakpoint, we don't need to insert
another one, but just continue running until we hit the original one.
Fixed in the patch below, together with a test case that triggers
the assert with the original patch.
Retested on armv7l-unknown-linux-gnueabi and i386-linux.
Does this look OK to you?
Thanks,
Ulrich
ChangeLog:
gdb/
* infrun.c (proceed): Revert previous change.
(resume): Instead, handle the case of signal delivery while stepping
off a breakpoint location here, and only if software single-stepping
is used. Handle nested signals.
gdb/testsuite/
* gdb.base/signest.exp: New file.
* gdb.base/signest.c: Likewise.
Index: gdb/infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.476
diff -u -p -r1.476 infrun.c
--- gdb/infrun.c 27 Apr 2011 17:08:41 -0000 1.476
+++ gdb/infrun.c 28 Apr 2011 14:24:44 -0000
@@ -1703,6 +1703,51 @@ a command like `return' or `jump' to con
else if (step)
step = maybe_software_singlestep (gdbarch, pc);
+ /* Currently, our software single-step implementation leads to different
+ results than hardware single-stepping in one situation: when stepping
+ into delivering a signal which has an associated signal handler,
+ hardware single-step will stop at the first instruction of the handler,
+ while software single-step will simply skip execution of the handler.
+
+ For now, this difference in behavior is accepted since there is no
+ easy way to actually implement single-stepping into a signal handler
+ without kernel support.
+
+ However, there is one scenario where this difference leads to follow-on
+ problems: if we're stepping off a breakpoint by removing all breakpoints
+ and then single-stepping. In this case, the software single-step
+ behavior means that even if there is a *breakpoint* in the signal
+ handler, GDB still would not stop.
+
+ Fortunately, we can at least fix this particular issue. We detect
+ here the case where we are about to deliver a signal while software
+ single-stepping with breakpoints removed. In this situation, we
+ revert the decisions to remove all breakpoints and insert single-
+ step breakpoints, and instead we install a step-resume breakpoint
+ at the current address, deliver the signal without stepping, and
+ once we arrive back at the step-resume breakpoint, actually step
+ over the breakpoint we originally wanted to step over. */
+ if (singlestep_breakpoints_inserted_p
+ && tp->control.trap_expected && sig != TARGET_SIGNAL_0)
+ {
+ /* If we have nested signals or a pending signal is delivered
+ immediately after a handler returns, might might already have
+ a step-resume breakpoint set on the earlier handler. We cannot
+ set another step-resume breakpoint; just continue on until the
+ original breakpoint is hit. */
+ if (tp->control.step_resume_breakpoint == NULL)
+ {
+ insert_step_resume_breakpoint_at_frame (get_current_frame ());
+ tp->step_after_step_resume_breakpoint = 1;
+ }
+
+ remove_single_step_breakpoints ();
+ singlestep_breakpoints_inserted_p = 0;
+
+ insert_breakpoints ();
+ tp->control.trap_expected = 0;
+ }
+
if (should_resume)
{
ptid_t resume_ptid;
@@ -2064,6 +2109,24 @@ proceed (CORE_ADDR addr, enum target_sig
/* prepare_to_proceed may change the current thread. */
tp = inferior_thread ();
+ if (oneproc)
+ {
+ tp->control.trap_expected = 1;
+ /* If displaced stepping is enabled, we can step over the
+ breakpoint without hitting it, so leave all breakpoints
+ inserted. Otherwise we need to disable all breakpoints, step
+ one instruction, and then re-add them when that step is
+ finished. */
+ if (!use_displaced_stepping (gdbarch))
+ remove_breakpoints ();
+ }
+
+ /* We can insert breakpoints if we're not trying to step over one,
+ or if we are stepping over one but we're using displaced stepping
+ to do so. */
+ if (! tp->control.trap_expected || use_displaced_stepping (gdbarch))
+ insert_breakpoints ();
+
if (!non_stop)
{
/* Pass the last stop signal to the thread we're resuming,
@@ -2133,42 +2196,6 @@ proceed (CORE_ADDR addr, enum target_sig
/* Reset to normal state. */
init_infwait_state ();
- /* Stepping over a breakpoint while at the same time delivering a signal
- has a problem: we cannot use displaced stepping, but we also cannot
- use software single-stepping, because we do not know where execution
- will continue if a signal handler is installed.
-
- On the other hand, if there is a signal handler we'd have to step
- over it anyway. So what we do instead is to install a step-resume
- handler at the current address right away, deliver the signal without
- stepping, and once we arrive back at the step-resume breakpoint, step
- once more over the original breakpoint we wanted to step over. */
- if (oneproc && tp->suspend.stop_signal != TARGET_SIGNAL_0
- && execution_direction != EXEC_REVERSE)
- {
- insert_step_resume_breakpoint_at_frame (get_current_frame ());
- tp->step_after_step_resume_breakpoint = 1;
- oneproc = 0;
- }
-
- if (oneproc)
- {
- tp->control.trap_expected = 1;
- /* If displaced stepping is enabled, we can step over the
- breakpoint without hitting it, so leave all breakpoints
- inserted. Otherwise we need to disable all breakpoints, step
- one instruction, and then re-add them when that step is
- finished. */
- if (!use_displaced_stepping (gdbarch))
- remove_breakpoints ();
- }
-
- /* We can insert breakpoints if we're not trying to step over one,
- or if we are stepping over one but we're using displaced stepping
- to do so. */
- if (! tp->control.trap_expected || use_displaced_stepping (gdbarch))
- insert_breakpoints ();
-
/* Resume inferior. */
resume (oneproc || step || bpstat_should_step (), tp->suspend.stop_signal);
--- /dev/null 2011-04-28 13:38:53.488264001 +0200
+++ gdb/testsuite/gdb.base/signest.exp 2011-04-28 16:15:16.000000000 +0200
@@ -0,0 +1,67 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2011 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/>.
+
+set testfile "signest"
+set srcfile ${testfile}.c
+
+if [target_info exists gdb,nosignals] {
+ verbose "Skipping ${testfile}.exp because of nosignals."
+ return -1
+}
+
+if [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug}] {
+ untested ${testfile}.exp
+ return -1
+}
+
+if ![runto_main] then {
+ untested ${testfile}.exp
+ return -1
+}
+
+# If we can examine what's at memory address 0, it is possible that we
+# could also execute it. This could probably make us run away,
+# executing random code, which could have all sorts of ill effects,
+# especially on targets without an MMU. Don't run the tests in that
+# case.
+
+gdb_test_multiple "x 0" "memory at address 0" {
+ -re "0x0:.*Cannot access memory at address 0x0.*$gdb_prompt $" { }
+ -re "0x0:.*Error accessing memory address 0x0.*$gdb_prompt $" { }
+ -re ".*$gdb_prompt $" {
+ untested "Memory at address 0 is possibly executable"
+ return -1
+ }
+}
+
+# Run until we hit the SIGSEGV (or SIGBUS on some platforms).
+gdb_test "continue" \
+ ".*Program received signal (SIGBUS|SIGSEGV).*bowler.*" \
+ "continue to fault"
+
+# Insert conditional breakpoint at faulting instruction
+gdb_test "break if 0" ".*" "set conditional breakpoint"
+
+# Set SIGSEGV/SIGBUS to pass+nostop
+gdb_test "handle SIGSEGV nostop print pass" ".*" "pass SIGSEGV"
+gdb_test "handle SIGBUS nostop print pass" ".*" "pass SIGBUS"
+
+# Step off the faulting instruction into the handler, triggering nested faults
+gdb_test "continue" \
+ ".*Program received signal (SIGBUS|SIGSEGV).*Program received signal (SIGBUS|SIGSEGV).*exited normally.*" \
+ "run through nested faults"
+
--- /dev/null 2011-04-28 13:38:53.488264001 +0200
+++ gdb/testsuite/gdb.base/signest.c 2011-04-28 16:04:52.000000000 +0200
@@ -0,0 +1,53 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2011 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 <signal.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+volatile char *p = NULL;
+
+extern long
+bowler (void)
+{
+ return *p;
+}
+
+extern void
+keeper (int sig)
+{
+ static int recurse = 0;
+ if (++recurse < 3)
+ bowler ();
+
+ _exit (0);
+}
+
+int
+main (void)
+{
+ struct sigaction act;
+ memset (&act, 0, sizeof act);
+ act.sa_handler = keeper;
+ act.sa_flags = SA_NODEFER;
+ sigaction (SIGSEGV, &act, NULL);
+ sigaction (SIGBUS, &act, NULL);
+
+ bowler ();
+ return 0;
+}
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch v2] Re: [commit] Re: [rfc][1/2] Signal delivery + software single-step is broken
2011-04-28 15:18 ` [patch v2] " Ulrich Weigand
@ 2011-04-28 15:46 ` Pedro Alves
2011-04-28 16:04 ` Ulrich Weigand
0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2011-04-28 15:46 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
On Thursday 28 April 2011 16:18:16, Ulrich Weigand wrote:
> Hmm, good point. Indeed I was able to trigger that assert.
>
> However, this is easily fixable just the same as in handle_inferior_event:
> if we already have a step-resume breakpoint, we don't need to insert
> another one, but just continue running until we hit the original one.
>
> Fixed in the patch below, together with a test case that triggers
> the assert with the original patch.
>
> Retested on armv7l-unknown-linux-gnueabi and i386-linux.
>
> Does this look OK to you?
Yes, thanks!
--
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch v2] Re: [commit] Re: [rfc][1/2] Signal delivery + software single-step is broken
2011-04-28 15:46 ` Pedro Alves
@ 2011-04-28 16:04 ` Ulrich Weigand
0 siblings, 0 replies; 10+ messages in thread
From: Ulrich Weigand @ 2011-04-28 16:04 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves wrote:
> On Thursday 28 April 2011 16:18:16, Ulrich Weigand wrote:
>
> > Hmm, good point. Indeed I was able to trigger that assert.
> >
> > However, this is easily fixable just the same as in handle_inferior_event:
> > if we already have a step-resume breakpoint, we don't need to insert
> > another one, but just continue running until we hit the original one.
> >
> > Fixed in the patch below, together with a test case that triggers
> > the assert with the original patch.
> >
> > Retested on armv7l-unknown-linux-gnueabi and i386-linux.
> >
> > Does this look OK to you?
>
> Yes, thanks!
Great; thanks again for your thorough review!
I've checked in the patch now.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-04-28 16:04 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-19 16:43 [rfc][1/2] Signal delivery + software single-step is broken Ulrich Weigand
2011-04-27 17:17 ` [commit] " Ulrich Weigand
2011-04-27 18:15 ` Pedro Alves
2011-04-27 19:12 ` Ulrich Weigand
2011-04-27 19:44 ` Pedro Alves
2011-04-28 8:55 ` [patch] " Ulrich Weigand
2011-04-28 12:01 ` Pedro Alves
2011-04-28 15:18 ` [patch v2] " Ulrich Weigand
2011-04-28 15:46 ` Pedro Alves
2011-04-28 16:04 ` Ulrich Weigand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).