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

[Doug Evans <dje at google dot com>]

On Mon, Dec 1, 2008 at 7:00 PM, Doug Evans <dje@google.com> wrote:
> Hi.  In http://sourceware.org/ml/gdb-patches/2008-11/msg00531.html
> I wrote:
>
>> I wrote a testcase that calls functions in multiple threads (with
>> scheduler-locking on) by setting a breakpoint on the function being
>> called.  I think there's a bug in scheduler-locking support as when I
>> make the second call in the second thread, gdb makes the first thread
>> step over the breakpoint and then resume, and control returns to
>> call_function_by_hand with inferior_ptid changed to the first thread.
>
> Here's a patch.
> This is separate from my dummy-frames cleanup patch.
>
> Ok to check in?

Except of course nothing is ever this easy. :-(

My testcase isn't complete:

set scheduler-locking off
continue

Adding the above to the end of the testcase reveals the fact that this
patch causes gdb to lose track of the fact that it needs to single
step over the breakpoint at all_threads_running in the main thread,
and resuming execution causes the breakpoint to be hit again.  Global
state, gotta love it.

I'm assuming non-stop mode doesn't have this problem.
Can we record in struct thread_info (or some such) the last stop
reason and before resuming with !scheduler-locking iterate over all
threads, single stepping them as necessary?  Is there a better
solution?

> 2008-12-01  Doug Evans  <dje@google.com>
>
>        * infrun.c (prepare_to_proceed): Document.  Assert !non_stop.
>        If scheduler-locking is enabled, we're not going to be singlestepping
>        any other previously stopped thread.
>
>        * gdb.threads/hand-call-in-threads.exp: New file.
>        * gdb.threads/hand-call-in-threads.c: New file.
>
> Index: infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.342
> diff -u -p -r1.342 infrun.c
> --- infrun.c    22 Nov 2008 04:41:45 -0000      1.342
> +++ infrun.c    2 Dec 2008 02:10:59 -0000
> @@ -1240,13 +1240,21 @@ clear_proceed_status (void)
>     }
>  }
>
> -/* This should be suitable for any targets that support threads. */
> +/* Check the current thread against the thread that reported the most recent
> +   event.  If a step-over is required return TRUE and set the current thread
> +   to the old thread.  Otherwise return FALSE.
> +
> +   This should be suitable for any targets that support threads. */
>
>  static int
>  prepare_to_proceed (int step)
>  {
>   ptid_t wait_ptid;
>   struct target_waitstatus wait_status;
> +  int schedlock_enabled;
> +
> +  /* With non-stop mode on, threads are always handled individually.  */
> +  gdb_assert (! non_stop);
>
>   /* Get the last target status returned by target_wait().  */
>   get_last_target_status (&wait_ptid, &wait_status);
> @@ -1258,9 +1266,15 @@ prepare_to_proceed (int step)
>       return 0;
>     }
>
> +  schedlock_enabled = (scheduler_mode == schedlock_on
> +                      || (scheduler_mode == schedlock_step
> +                          && step));
> +
>   /* Switched over from WAIT_PID.  */
>   if (!ptid_equal (wait_ptid, minus_one_ptid)
> -      && !ptid_equal (inferior_ptid, wait_ptid))
> +      && !ptid_equal (inferior_ptid, wait_ptid)
> +      /* Don't single step WAIT_PID if scheduler locking is on.  */
> +      && !schedlock_enabled)
>     {
>       struct regcache *regcache = get_thread_regcache (wait_ptid);
>
> Index: testsuite/gdb.threads/hand-call-in-threads.c
> ===================================================================
> RCS file: testsuite/gdb.threads/hand-call-in-threads.c
> diff -N testsuite/gdb.threads/hand-call-in-threads.c
> --- /dev/null   1 Jan 1970 00:00:00 -0000
> +++ testsuite/gdb.threads/hand-call-in-threads.c        2 Dec 2008 02:54:49 -0000
> @@ -0,0 +1,134 @@
> +/* Test case for hand function calls in multi-threaded program.
> +
> +   Copyright 2008 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   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 <pthread.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <time.h>
> +#include <unistd.h>
> +
> +/* NOTE: Also defined in hand-call-in-threads.exp.  */
> +#define NR_THREADS 4
> +
> +int thread_count;
> +
> +pthread_mutex_t thread_count_mutex;
> +
> +pthread_cond_t thread_count_condvar;
> +
> +void
> +incr_thread_count (void)
> +{
> +  pthread_mutex_lock (&thread_count_mutex);
> +  ++thread_count;
> +  if (thread_count == NR_THREADS)
> +    pthread_cond_signal (&thread_count_condvar);
> +  pthread_mutex_unlock (&thread_count_mutex);
> +}
> +
> +void
> +cond_wait (pthread_cond_t *cond, pthread_mutex_t *mut)
> +{
> +  pthread_mutex_lock (mut);
> +  pthread_cond_wait (cond, mut);
> +  pthread_mutex_unlock (mut);
> +}
> +
> +void
> +noreturn (void)
> +{
> +  pthread_mutex_t mut;
> +  pthread_cond_t cond;
> +
> +  pthread_mutex_init (&mut, NULL);
> +  pthread_cond_init (&cond, NULL);
> +
> +  /* Wait for a condition that will never be signaled, so we effectively
> +     block the thread here.  */
> +  cond_wait (&cond, &mut);
> +}
> +
> +void *
> +forever_pthread (void *unused)
> +{
> +  incr_thread_count ();
> +  noreturn ();
> +}
> +
> +void
> +hand_call (void)
> +{
> +}
> +
> +/* Wait until all threads are running.  */
> +
> +void
> +wait_all_threads_running (void)
> +{
> +  int timeout = 60; /* seconds */
> +  int i;
> +
> +  for (i = 0; i < timeout; ++i)
> +    {
> +      pthread_mutex_lock (&thread_count_mutex);
> +      if (thread_count == NR_THREADS)
> +       {
> +         pthread_mutex_unlock (&thread_count_mutex);
> +         return;
> +       }
> +      pthread_cond_wait (&thread_count_condvar, &thread_count_mutex);
> +      if (thread_count == NR_THREADS)
> +       {
> +         pthread_mutex_unlock (&thread_count_mutex);
> +         return;
> +       }
> +      pthread_mutex_unlock (&thread_count_mutex);
> +      sleep (1);
> +    }
> +
> +  printf ("timeout waiting for all threads to start\n");
> +  abort ();
> +}
> +
> +/* Called when all threads are running.
> +   Easy place for a breakpoint.  */
> +
> +void
> +all_threads_running (void)
> +{
> +}
> +
> +int
> +main (void)
> +{
> +  pthread_t forever[NR_THREADS];
> +  int i;
> +
> +  pthread_mutex_init (&thread_count_mutex, NULL);
> +  pthread_cond_init (&thread_count_condvar, NULL);
> +
> +  for (i = 0; i < NR_THREADS; ++i)
> +    pthread_create (&forever[i], NULL, forever_pthread, NULL);
> +
> +  wait_all_threads_running ();
> +  all_threads_running ();
> +
> +  return 0;
> +}
> +
> Index: testsuite/gdb.threads/hand-call-in-threads.exp
> ===================================================================
> RCS file: testsuite/gdb.threads/hand-call-in-threads.exp
> diff -N testsuite/gdb.threads/hand-call-in-threads.exp
> --- /dev/null   1 Jan 1970 00:00:00 -0000
> +++ testsuite/gdb.threads/hand-call-in-threads.exp      2 Dec 2008 02:54:49 -0000
> @@ -0,0 +1,140 @@
> +# Copyright (C) 2004, 2007, 2008 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/>.
> +
> +# Test making hand function calls in multiple threads.
> +
> +# NOTE: Also defined in hand-call-in-threads.c.
> +set NR_THREADS 4
> +
> +if $tracelevel then {
> +       strace $tracelevel
> +}
> +
> +set testfile "hand-call-in-threads"
> +set srcfile ${testfile}.c
> +set binfile ${objdir}/${subdir}/${testfile}
> +
> +if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "incdir=${objdir}"]] != "" } {
> +    return -1
> +}
> +
> +# Some targets can't do function calls, so don't even bother with this
> +# test.
> +if [target_info exists gdb,cannot_call_functions] {
> +    setup_xfail "*-*-*" 2416
> +    fail "This target can not call functions"
> +    continue
> +}
> +
> +proc get_dummy_frame_number { } {
> +  global gdb_prompt
> +
> +  send_gdb "bt\n"
> +  gdb_expect {
> +    -re "#(\[0-9\]*) *<function called from gdb>.*$gdb_prompt $"
> +      {
> +       return $expect_out(1,string)
> +      }
> +    -re "$gdb_prompt $"
> +      {
> +       return ""
> +      }
> +    timeout
> +      {
> +       return ""
> +      }
> +  }
> +  return ""
> +}
> +
> +gdb_exit
> +gdb_start
> +gdb_reinitialize_dir $srcdir/$subdir
> +gdb_load ${binfile}
> +
> +if { ![runto_main] } {
> +    fail "Can't run to main"
> +    return 0
> +}
> +
> +gdb_test "break all_threads_running" \
> +         "Breakpoint 2 at .*: file .*${srcfile}, line .*" \
> +         "breakpoint on all_threads_running"
> +
> +gdb_test "break hand_call" \
> +         "Breakpoint 3 at .*: file .*${srcfile}, line .*" \
> +         "breakpoint on hand_call"
> +
> +# Run the program and make sure GDB reports that we stopped after
> +# hitting breakpoint 2 in all_threads_running().
> +
> +gdb_test "continue" \
> +         ".*Breakpoint 2, all_threads_running ().*" \
> +         "run to all_threads_running"
> +
> +# Before we start making hand function calls, turn on scheduler locking.
> +
> +gdb_test "set scheduler-locking on" "" "enable scheduler locking"
> +gdb_test "show scheduler-locking" ".* locking scheduler .* is \"on\"." "show scheduler locking"
> +
> +# Now hand-call a function in each thread, having the function
> +# stop without returning.
> +
> +# Add one for the main thread.
> +set total_nr_threads [expr $NR_THREADS + 1]
> +
> +# Thread numbering in gdb is origin-1, so begin numbering at 1.
> +for { set i 1 } { $i <= $total_nr_threads } { incr i } {
> +    set thread_nr $i
> +    gdb_test "thread $thread_nr" "" "prepare to make hand call, thread $thread_nr"
> +    gdb_test "call hand_call()" "Breakpoint 3, .*" "hand call, thread $thread_nr"
> +}
> +
> +# Now have each hand-called function return.
> +
> +# Turn confirmation off for the "return" command.
> +gdb_test "set confirm off" ""
> +
> +clear_xfail "*-*-*"
> +
> +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
> +    } else {
> +       pass "dummy stack frame number, thread $thread_nr"
> +    }
> +    # Pop the dummy frame.
> +    gdb_test "frame $frame_number" "" "setting frame, thread $thread_nr"
> +    gdb_test "return" "" "discard hand call, thread $thread_nr"
> +    # In case getting the dummy frame number failed, re-enable for next iter.
> +    clear_xfail "*-*-*"
> +}
> +
> +# Make sure all dummy frames got popped.
> +
> +gdb_test_multiple "maint print dummy-frames" "all dummies popped" {
> +    -re ".*stack=.*$gdb_prompt $" {
> +       fail "all dummies popped"
> +    }
> +    -re ".*$gdb_prompt $" {
> +       pass "all dummies popped"
> +    }
> +}
>