Re: [RFA] Fix hand called function when another thread has hit a bp.

[Doug Evans <dje@google.com>]

On Mon, Mar 30, 2009 at 11:40 AM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Doug Evans wrote:
>
>> Hi.  Here's an updated version of the patch.
>
> Sorry for the late reply on this!

No worries.

>> Handling the restart after several threads are all stopped at a
>> breakpoint (via scheduler-locking = on), is left for a later patch
>> (it's happens more rarely).
>
> This patch, as far as I can see, just replaces one incorrect
> behaviour with a different incorrect behaviour, right?

One could look at it that way.  I look at it as taking one step of a
two step fix. :-)

> That is to say, in the scenario where we have
>
>  - set scheduler-locking on
>  - stop on BP in thread A
>  - manually switch to thread B
>  - continue execution
>
> the behaviour today is:
>
>  - GDB will switch back to A and single-step
>  - (correctly) bypass the already-hit breakpoint in A
>  - (incorrectly) continue execution thread A
>
> i.e. the incorrect behaviour is that thread A is continued,
> and not thread B.
>
> With your patch, the behaviour is:
>
>  - GDB (correctly) continues execution of thread B
>  - but the next time thread A is run, GDB will (incorrectly)
>   report a second time the same breakpoint the user already saw
>
> Did I miss anything here?
>
> In any case, I guess I agree that the "new" type of incorrect
> behaviour is probably less bad that what we have today, so
> your patch does seem to be a step forward.
>
> Will you be working on a follow-on patch to fix the new
> incorrect behaviour?

That's the plan.  It involves recording the stop reason for every
thread, and appropriately applying it when resuming.

>
>>+for { set i 1 } { $i <= $total_nr_threads } { incr i } {
>>+    set thread_nr $i
>>+    gdb_test "thread $thread_nr" "" "prepare to discard hand call, thread $thread_nr"
>>+    set frame_number [get_dummy_frame_number]
>>+    if { "$frame_number" == "" } {
>>+      fail "dummy stack frame number, thread $thread_nr"
>>+      setup_xfail "*-*-*"
>>+      # Need something.
>>+      set frame_number 0
>
> Why do we need this xfail here?

It's not needed.  I can remove it.

>>+# Continue one last time, the program should exit normally.
>>+#
>>+# ??? This currently doesn't work because gdb doesn't know how to singlestep
>>+# over reported breakpoints that weren't in the last thread to run.
>>+# Fixing this first requires defining what the correct behaviour is.
>>+# Commented out until then.
>>+#
>>+# Manually set the thread back to the first thread: the program is still at
>>+# the all_threads_running breakpoint, which wasn't the last thread to run,
>>+# and gdb doesn't know how to singlestep over reported breakpoints that
>>+# weren't in the last thread to run.
>>+#gdb_test "thread 1" "" "set thread to 1, prepare to resume"
>>+#
>>+#gdb_continue_to_end "hand-call-in-threads"
>
> Should the "thread 1" really be here?  It seems to me this was just an
> unsuccessful attempt to work-around the bug ...

It's there to document the bug (until it's fixed).

> Otherwise, the patch is OK.