This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch/testsuite/gdbserver] unsuspend lwps after step over


Hi Yao,

On 02/17/2012 02:29 AM, Yao Qi wrote:

> continue^M
> Continuing.^M
> Remote connection closed^M
> (gdb) FAIL: gdb.trace/trace-break-mt.exp: 1 trace on: continue to
> set_point1 2
> ../../../gdb/gdb/gdbserver/linux-low.c:2801: A problem internal to
> GDBserver has been detected.^M
> stuck_in_jump_pad_callback: Assertion `lwp->suspended == 0' failed.^M
> 
> trace-break-mt.exp is a "multi-threaded" version of trace-break.exp.

I don't object to the test, but I'm wondering if you found all that
baggage really necessary, such as testing fast tracepoints, or setting a
breakpoint on top of the tracepoint?  I think the test only really needs:

- more than one thread
- all-stop
- setting a tracepoint at $pc
- tstart
- stepi

All other combinations that don't end up as this would pass, AFAICS.

> This patch just decrements `suspended' flag before calling
> stabilize_threads below.  

And that's correct.

> This part of code in gdbserver is quite new to
> me, and comments are welcome.

Yeah, this stepping of threads behind gdb's back code was quite tricky to
get right... It went through several different designs (and a lot of
hair pulling) until it ended up as it is.
>  
> +      /* If we just finished a step-over, all other threads had been
> +	 paused in function start_step_over.  We have to unsuspend
> +	 them, say, decrement `suspended' flag, otherwise, we'll get
> +	 an assertion failure else where.  */

Suggest tweaking this a bit:

    /* If we were going a step-over, all other threads but the stepping one
       had been paused in start_step_over, with their suspend counts
       incremented.  We don't want to do a full unstop/unpause, because we're
       in all-stop mode (so we want threads stopped), but we still need to
       unsuspend the other threads, to decrement their `suspended' count back.  */

> +      if (step_over_finished)
> +	unsuspend_all_lwps (event_child);
> +
>        /* Stabilize threads (move out of jump pads).  */
>        stabilize_threads ();
>      }

> +# Set breakpoint and tracepoint at the same address.
> +
> +proc break_trace_same_addr_1 { trace_type option } {
> +    global executable
> +    global pf_prefix
> +    global hex
> +
> +    set old_pf_prefix $pf_prefix
> +    set pf_prefix "$pf_prefix 1 $trace_type $option:"

Can you please adjust the test to use the new with_test_prefix
mechanism instead?  Sorry about that.  What that "1" in the prefix
mean, btw?

-- 
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]