[commit] Re: [rfc][1/2] Signal delivery + software single-step is broken

Ulrich Weigand
2011-04-27T17:17:46Z

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"
}