This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [rfc][2/2] Signal delivery + software single-step is broken
- From: Pedro Alves <pedro at codesourcery dot com>
- To: "Ulrich Weigand" <uweigand at de dot ibm dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 19 Jan 2011 13:30:36 -0700
- Subject: Re: [rfc][2/2] Signal delivery + software single-step is broken
- References: <201101191848.p0JImKuL015762@d06av02.portsmouth.uk.ibm.com>
On Wednesday 19 January 2011 11:48:20, Ulrich Weigand wrote:
> Pedro Alves wrote:
> > On Wednesday 19 January 2011 09:42:49, Ulrich Weigand wrote:
> > > The patch below changes linux_nat_wait_1 to actually look at the
> > > stepping status of the thread directly, instead of relying on the
> > > "step" flag. This means the "currently_stepping" routine has to be
> > > exported from infrun.c so it can be called here.
> >
> >
> >
> > I'm not objecting, but I'm curious on whether you've thought about how
> > the same problem would be solved in gdbserver/linux-low.c, which
> > can't call currently_stepping, since it's running in a different process?
>
> Good point, however this test already must use information only available
> in GDB itself, namely whether or not the signal is to be passed to the
> inferior or intercepted by GDB:
>
> if (!lp->step
> && inf->control.stop_soon == NO_STOP_QUIETLY
> && signal_stop_state (signo) == 0
> && signal_print_state (signo) == 0
> && signal_pass_state (signo) == 1)
>
> (The "stop_soon" state likewise is GDB private data.)
>
> Presumably because of this, gdbserver today does not appear to be
> implementing this particular optimization at all, but always reports
> all signals back to GDB to decide what to do with them.
It does implement it. It's done in gdbserver/linux-low.c:linux_wait_1.
Look for:
(pass_signals[target_signal_from_host (WSTOPSIG (w))]
In linux-nat.c's case, stop_soon used to be set to STOP_QUIETLY
while doing the startup_inferior dance, but it no longer does ever
since you've rewriten startup_inferior to not use wait_for_inferior,
but use target_resume/target_wait directly, I think. I guess it
may be a bug that stop_soon is no longer set in that case?
In any case, gdbserver does it's own startup_inferior equivalent
(gdbserver/server.c:start_inferior), so it does not need to peek
into gdb's inf->control.stop_soon for this case. It can use a flag
in its own inferior/process structure.
The flag is also set to STOP_QUIETLY in svr4_solib_create_inferior_hook,
but that's in code that only gets built for SCO. In this case,
if we cared for SCO, I'd say we'd wrap the loop in
svr4_solib_create_inferior_hook with the same
QPassSignals dance (the target method in question
is target_notice_signals, btw).
>
> > One way to do it would be to do:
> >
> > QPassSignals:
> > vCont;c
> > QPassSignals:foo;bar
> >
> >
> > but that is a lot of extra roundtrips, and not really (inferior)
> > threadsafe in non-stop mode.
>
> I agree that this would probably be the way to go about it. I'm not sure
> thread safety is really a concern here, given that we're talking about an
> optimization. If the implementation is conservative in the right
> direction, the worst thing that could happen is that a signal is reported
> that might have gotten short-circuited ..
Right.
>
> Similarly, the number of roundtrips could probably be reduced by only
> sending a QPassSignals when the list of interesting signal changes.
> For example, once we start single-stepping, we'd once send the
> QPassSignals:
> and then not send and further QPassSignals until we go back to letting
> the inferior continue freely.
Yeah. remote.c:remote_pass_signals already avoids sending the
QPassSignals packet to the target if the list of interesting
signals didn't change.
>
> > It sounds like we'd need to tweak the target resume interface to be
> > able to say "continue, but I'm interested in signals and everything
> > else", or, "I'm telling you to continue, but you're really
> > single-stepping", like a new vCont;cs or some such?
>
> I'm not so sure I like this, as it introduces somewhat less well-
> defined semantics: what does "*really* single-stepping" mean, other
> than in terms of doing whatever it is GDB does now ...
Agreed.
--
Pedro Alves