[RFA] Fix build failure [was Re: [RFC] fork in multi-threaded apps, per-thread pending_fork, other follow-fork cleanups.]

[RFA] Fix build failure [was Re: [RFC] fork in multi-threaded apps, per-thread pending_fork, other follow-fork cleanups.]

[Doug Evans]

On Sun, May 24, 2009 at 11:01 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> [...]
> We're not worse than before, so I checked in this revision of that
> patch with a few extra comments and small cleanups.
>
> --
> Pedro Alves
>
> gdb/
> 2009-05-24  Pedro Alves  <pedro@codesourcery.com>
>
>        * gdbthread.h (struct thread_info): New `pending_follow' field.
>        * thread.c (new_thread): New function.
>        (add_thread_silent): Use it.
>        * breakpoint.c (internal_breakpoint_number): New global, moved
>        from inside...
>        (create_internal_breakpoint): ... this.
>        (clone_momentary_breakpoint): New.
>        * breakpoint.h (clone_momentary_breakpoint): Declare.
>        * infrun.c (nullify_last_target_wait_ptid): Move declaration
>        higher.
>        (pending_follow): Delete.
>        (follow_fork): Handle pending follow fork event here.  Moved the
>        preserving of thread stepping state here.
>        (resume): Don't handle pending follow fork events here.  Only
>        install the inferior's terminal modes if we're about to resume it.
>        (proceed): Handle possible pending follow fork events here.
>        (init_wait_for_inferior): No need to clear pending_follow anymore,
>        it's gone.
>        (handle_inferior_event): Adjust to per-thread `pending_follow'.
>        Call `follow_fork' to handle following the fork.  If the
>        follow-fork is cancelled, stop stepping.
>        * linux-nat.c (linux_child_follow_fork): Adjust to per-thread
>        `pending_follow' events.  Remove code that handled preserving the
>        thread stepping state.
>        * inf-ptrace.c (inf_ptrace_follow_fork): Ditto.
>        * inf-ttrace.c (inf_ttrace_follow_fork): Ditto.
>
> gdb/testsuite/
> 2009-05-24  Pedro Alves  <pedro@codesourcery.com>
>
>        * gdb.threads/fork-thread-pending.c: New.
>        * gdb.threads/fork-thread-pending.exp: New.
>
> ---
>  gdb/breakpoint.c                                  |   40 +++
>  gdb/breakpoint.h                                  |    2
>  gdb/gdbthread.h                                   |    5
>  gdb/inf-ptrace.c                                  |   36 ---
>  gdb/inf-ttrace.c                                  |   44 ----
>  gdb/infrun.c                                      |  241 ++++++++++++++++------
>  gdb/linux-nat.c                                   |   37 ---
>  gdb/testsuite/gdb.threads/fork-thread-pending.c   |  109 +++++++++
>  gdb/testsuite/gdb.threads/fork-thread-pending.exp |  128 +++++++++++
>  gdb/thread.c                                      |   37 ++-
>  10 files changed, 508 insertions(+), 171 deletions(-)
>
> Index: src/gdb/gdbthread.h
> ===================================================================
> --- src.orig/gdb/gdbthread.h    2009-05-21 23:45:24.000000000 +0100
> +++ src/gdb/gdbthread.h 2009-05-24 17:09:03.000000000 +0100
> @@ -165,6 +165,11 @@ struct thread_info
>      next time inferior stops if it stops due to stepping.  */
>   int step_multi;
>
> +  /* This is used to remember when a fork or vfork event was caught by
> +     a catchpoint, and thus the event is to be followed at the next
> +     resume of the thread, and not immediately.  */
> +  struct target_waitstatus pending_follow;
> +
>   /* Last signal that the inferior received (why it stopped).  */
>   enum target_signal stop_signal;
>
> Index: src/gdb/thread.c
> ===================================================================
> --- src.orig/gdb/thread.c       2009-05-21 23:45:24.000000000 +0100
> +++ src/gdb/thread.c    2009-05-24 17:14:18.000000000 +0100
> @@ -141,6 +141,28 @@ init_thread_list (void)
>   thread_list = NULL;
>  }
>
> +/* Allocate a new thread with target id PTID and add it to the thread
> +   list.  */
> +
> +static struct thread_info *
> +new_thread (ptid_t ptid)
> +{
> +  struct thread_info *tp;
> +
> +  tp = xcalloc (1, sizeof (*tp));
> +
> +  tp->ptid = ptid;
> +  tp->num = ++highest_thread_num;
> +  tp->next = thread_list;
> +  thread_list = tp;
> +
> +  /* Nothing to follow yet.  */
> +  tp->pending_follow.kind = TARGET_WAITKIND_SPURIOUS;
> +  tp->state_ = THREAD_STOPPED;
> +
> +  return tp;
> +}
> +
>  struct thread_info *
>  add_thread_silent (ptid_t ptid)
>  {
> @@ -162,12 +184,7 @@ add_thread_silent (ptid_t ptid)
>
>       if (ptid_equal (inferior_ptid, ptid))
>        {
> -         tp = xmalloc (sizeof (*tp));
> -         memset (tp, 0, sizeof (*tp));
> -         tp->ptid = minus_one_ptid;
> -         tp->num = ++highest_thread_num;
> -         tp->next = thread_list;
> -         thread_list = tp;
> +         tp = new_thread (ptid);
>
>          /* Make switch_to_thread not read from the thread.  */
>          tp->state_ = THREAD_EXITED;
> @@ -191,13 +208,7 @@ add_thread_silent (ptid_t ptid)
>        delete_thread (ptid);
>     }
>
> -  tp = (struct thread_info *) xmalloc (sizeof (*tp));
> -  memset (tp, 0, sizeof (*tp));
> -  tp->ptid = ptid;
> -  tp->num = ++highest_thread_num;
> -  tp->next = thread_list;
> -  thread_list = tp;
> -
> +  tp = new_thread (ptid);
>   observer_notify_new_thread (tp);
>
>   return tp;
> Index: src/gdb/breakpoint.c
> ===================================================================
> --- src.orig/gdb/breakpoint.c   2009-05-23 18:24:43.000000000 +0100
> +++ src/gdb/breakpoint.c        2009-05-24 17:18:54.000000000 +0100
> @@ -1456,10 +1456,11 @@ reattach_breakpoints (int pid)
>   return 0;
>  }
>
> +static int internal_breakpoint_number = -1;
> +
>  static struct breakpoint *
>  create_internal_breakpoint (CORE_ADDR address, enum bptype type)
>  {
> -  static int internal_breakpoint_number = -1;
>   struct symtab_and_line sal;
>   struct breakpoint *b;
>
> @@ -5007,6 +5008,43 @@ set_momentary_breakpoint (struct symtab_
>   return b;
>  }
>
> +/* Make a deep copy of momentary breakpoint ORIG.  Returns NULL if
> +   ORIG is NULL.  */
> +
> +struct breakpoint *
> +clone_momentary_breakpoint (struct breakpoint *orig)
> +{
> +  struct breakpoint *copy;
> +
> +  /* If there's nothing to clone, then return nothing.  */
> +  if (orig == NULL)
> +    return NULL;
> +
> +  copy = set_raw_breakpoint_without_location (orig->type);
> +  copy->loc = allocate_bp_location (copy);
> +  set_breakpoint_location_function (copy->loc);
> +
> +  copy->loc->requested_address = orig->loc->requested_address;
> +  copy->loc->address = orig->loc->address;
> +  copy->loc->section = orig->loc->section;
> +
> +  if (orig->source_file == NULL)
> +    copy->source_file = NULL;
> +  else
> +    copy->source_file = xstrdup (orig->source_file);
> +
> +  copy->line_number = orig->line_number;
> +  copy->frame_id = orig->frame_id;
> +  copy->thread = orig->thread;
> +
> +  copy->enable_state = bp_enabled;
> +  copy->disposition = disp_donttouch;
> +  copy->number = internal_breakpoint_number--;
> +
> +  update_global_location_list_nothrow (0);
> +  return copy;
> +}
> +
>  struct breakpoint *
>  set_momentary_breakpoint_at_pc (CORE_ADDR pc, enum bptype type)
>  {
> Index: src/gdb/breakpoint.h
> ===================================================================
> --- src.orig/gdb/breakpoint.h   2009-05-21 23:45:24.000000000 +0100
> +++ src/gdb/breakpoint.h        2009-05-24 17:09:03.000000000 +0100
> @@ -696,6 +696,8 @@ extern struct breakpoint *set_momentary_
>  extern struct breakpoint *set_momentary_breakpoint_at_pc
>   (CORE_ADDR pc, enum bptype type);
>
> +extern struct breakpoint *clone_momentary_breakpoint (struct breakpoint *bpkt);
> +
>  extern void set_ignore_count (int, int, int);
>
>  extern void set_default_breakpoint (int, CORE_ADDR, struct symtab *, int);
> Index: src/gdb/infrun.c
> ===================================================================
> --- src.orig/gdb/infrun.c       2009-05-24 17:02:20.000000000 +0100
> +++ src/gdb/infrun.c    2009-05-24 17:13:31.000000000 +0100
> @@ -83,6 +83,8 @@ static int prepare_to_proceed (int);
>
>  void _initialize_infrun (void);
>
> +void nullify_last_target_wait_ptid (void);
> +
>  /* 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.  */
> @@ -255,21 +257,6 @@ void init_thread_stepping_state (struct
>
>  void init_infwait_state (void);
>
> -/* This is used to remember when a fork or vfork event was caught by a
> -   catchpoint, and thus the event is to be followed at the next resume
> -   of the inferior, and not immediately.  */
> -static struct
> -{
> -  enum target_waitkind kind;
> -  struct
> -  {
> -    ptid_t parent_pid;
> -    ptid_t child_pid;
> -  }
> -  fork_event;
> -}
> -pending_follow;
> -
>  static const char follow_fork_mode_child[] = "child";
>  static const char follow_fork_mode_parent[] = "parent";
>
> @@ -290,12 +277,157 @@ Debugger response to a program call of f
>  }
>
>
> +/* Tell the target to follow the fork we're stopped at.  Returns true
> +   if the inferior should be resumed; false, if the target for some
> +   reason decided it's best not to resume.  */
> +
>  static int
>  follow_fork (void)
>  {
>   int follow_child = (follow_fork_mode_string == follow_fork_mode_child);
> +  int should_resume = 1;
> +  struct thread_info *tp;
> +
> +  /* Copy user stepping state to the new inferior thread.  FIXME: the
> +     followed fork child thread should have a copy of most of the
> +     parent thread structure's run control related fields, not just
> +     these.  */
> +  struct breakpoint *step_resume_breakpoint;
> +  CORE_ADDR step_range_start;
> +  CORE_ADDR step_range_end;
> +  struct frame_id step_frame_id;
> +
> +  if (!non_stop)
> +    {
> +      ptid_t wait_ptid;
> +      struct target_waitstatus wait_status;
> +
> +      /* Get the last target status returned by target_wait().  */
> +      get_last_target_status (&wait_ptid, &wait_status);
> +
> +      /* If not stopped at a fork event, then there's nothing else to
> +        do.  */
> +      if (wait_status.kind != TARGET_WAITKIND_FORKED
> +         && wait_status.kind != TARGET_WAITKIND_VFORKED)
> +       return 1;
> +
> +      /* Check if we switched over from WAIT_PTID, since the event was
> +        reported.  */
> +      if (!ptid_equal (wait_ptid, minus_one_ptid)
> +         && !ptid_equal (inferior_ptid, wait_ptid))
> +       {
> +         /* We did.  Switch back to WAIT_PTID thread, to tell the
> +            target to follow it (in either direction).  We'll
> +            afterwards refuse to resume, and inform the user what
> +            happened.  */
> +         switch_to_thread (wait_ptid);
> +         should_resume = 0;
> +       }
> +    }
> +
> +  tp = inferior_thread ();
> +
> +  /* If there were any forks/vforks that were caught and are now to be
> +     followed, then do so now.  */
> +  switch (tp->pending_follow.kind)
> +    {
> +    case TARGET_WAITKIND_FORKED:
> +    case TARGET_WAITKIND_VFORKED:
> +      {
> +       ptid_t parent, child;
> +
> +       /* If the user did a next/step, etc, over a fork call,
> +          preserve the stepping state in the fork child.  */
> +       if (follow_child && should_resume)
> +         {
> +           step_resume_breakpoint
> +             = clone_momentary_breakpoint (tp->step_resume_breakpoint);
> +           step_range_start = tp->step_range_start;
> +           step_range_end = tp->step_range_end;
> +           step_frame_id = tp->step_frame_id;
> +
> +           /* For now, delete the parent's sr breakpoint, otherwise,
> +              parent/child sr breakpoints are considered duplicates,
> +              and the child version will not be installed.  Remove
> +              this when the breakpoints module becomes aware of
> +              inferiors and address spaces.  */
> +           delete_step_resume_breakpoint (tp);
> +           tp->step_range_start = 0;
> +           tp->step_range_end = 0;
> +           tp->step_frame_id = null_frame_id;
> +         }
> +
> +       parent = inferior_ptid;
> +       child = tp->pending_follow.value.related_pid;
> +
> +       /* Tell the target to do whatever is necessary to follow
> +          either parent or child.  */
> +       if (target_follow_fork (follow_child))
> +         {
> +           /* Target refused to follow, or there's some other reason
> +              we shouldn't resume.  */
> +           should_resume = 0;
> +         }
> +       else
> +         {
> +           /* This pending follow fork event is now handled, one way
> +              or another.  The previous selected thread may be gone
> +              from the lists by now, but if it is still around, need
> +              to clear the pending follow request.  */
> +           tp = find_thread_pid (parent);
> +           if (tp)
> +             tp->pending_follow.kind = TARGET_WAITKIND_SPURIOUS;
> +
> +           /* This makes sure we don't try to apply the "Switched
> +              over from WAIT_PID" logic above.  */
> +           nullify_last_target_wait_ptid ();
> +
> +           /* If we followed the child, switch to it... */
> +           if (follow_child)
> +             {
> +               switch_to_thread (child);
> +
> +               /* ... and preserve the stepping state, in case the
> +                  user was stepping over the fork call.  */
> +               if (should_resume)
> +                 {
> +                   tp = inferior_thread ();
> +                   tp->step_resume_breakpoint = step_resume_breakpoint;
> +                   tp->step_range_start = step_range_start;
> +                   tp->step_range_end = step_range_end;
> +                   tp->step_frame_id = step_frame_id;
> +                 }
> +               else
> +                 {
> +                   /* If we get here, it was because we're trying to
> +                      resume from a fork catchpoint, but, the user
> +                      has switched threads away from the thread that
> +                      forked.  In that case, the resume command
> +                      issued is most likely not applicable to the
> +                      child, so just warn, and refuse to resume.  */
> +                   warning (_("\
> +Not resuming: switched threads before following fork child.\n"));
> +                 }
> +
> +               /* Reset breakpoints in the child as appropriate.  */
> +               follow_inferior_reset_breakpoints ();
> +             }
> +           else
> +             switch_to_thread (parent);
> +         }
> +      }
> +      break;
> +    case TARGET_WAITKIND_SPURIOUS:
> +      /* Nothing to follow.  */
> +      break;
> +    default:
> +      internal_error (__FILE__, __LINE__,
> +                     "Unexpected pending_follow.kind %d\n",
> +                     tp->pending_follow.kind);
> +      break;
> +    }
>
> -  return target_follow_fork (follow_child);
> +  return should_resume;
>  }
>
>  void
> @@ -987,8 +1119,6 @@ resume (int step, enum target_signal sig
>  {
>   int should_resume = 1;
>   struct cleanup *old_cleanups = make_cleanup (resume_cleanups, 0);
> -
> -  /* Note that these must be reset if we follow a fork below.  */
>   struct regcache *regcache = get_current_regcache ();
>   struct gdbarch *gdbarch = get_regcache_arch (regcache);
>   struct thread_info *tp = inferior_thread ();
> @@ -1058,31 +1188,6 @@ a command like `return' or `jump' to con
>   if (step)
>     step = maybe_software_singlestep (gdbarch, pc);
>
> -  /* If there were any forks/vforks/execs that were caught and are
> -     now to be followed, then do so.  */
> -  switch (pending_follow.kind)
> -    {
> -    case TARGET_WAITKIND_FORKED:
> -    case TARGET_WAITKIND_VFORKED:
> -      pending_follow.kind = TARGET_WAITKIND_SPURIOUS;
> -      if (follow_fork ())
> -       should_resume = 0;
> -
> -      /* Following a child fork will change our notion of current
> -        thread.  */
> -      tp = inferior_thread ();
> -      regcache = get_current_regcache ();
> -      gdbarch = get_regcache_arch (regcache);
> -      pc = regcache_read_pc (regcache);
> -      break;
> -
> -    default:
> -      break;
> -    }
> -
> -  /* Install inferior's terminal modes.  */
> -  target_terminal_inferior ();
> -
>   if (should_resume)
>     {
>       ptid_t resume_ptid;
> @@ -1164,6 +1269,9 @@ a command like `return' or `jump' to con
>           displaced_step_dump_bytes (gdb_stdlog, buf, sizeof (buf));
>         }
>
> +      /* Install inferior's terminal modes.  */
> +      target_terminal_inferior ();
> +
>       /* Avoid confusing the next resume, if the next stop/resume
>         happens to apply to another thread.  */
>       tp->stop_signal = TARGET_SIGNAL_0;
> @@ -1305,12 +1413,26 @@ prepare_to_proceed (int step)
>  void
>  proceed (CORE_ADDR addr, enum target_signal siggnal, int step)
>  {
> -  struct regcache *regcache = get_current_regcache ();
> -  struct gdbarch *gdbarch = get_regcache_arch (regcache);
> +  struct regcache *regcache;
> +  struct gdbarch *gdbarch;
>   struct thread_info *tp;
> -  CORE_ADDR pc = regcache_read_pc (regcache);
> +  CORE_ADDR pc;
>   int oneproc = 0;
>
> +  /* If we're stopped at a fork/vfork, follow the branch set by the
> +     "set follow-fork-mode" command; otherwise, we'll just proceed
> +     resuming the current thread.  */
> +  if (!follow_fork ())
> +    {
> +      /* The target for some reason decided not to resume.  */
> +      normal_stop ();
> +      return;
> +    }
> +
> +  regcache = get_current_regcache ();
> +  gdbarch = get_regcache_arch (regcache);
> +  pc = regcache_read_pc (regcache);
> +
>   if (step > 0)
>     step_start_function = find_pc_function (pc);
>   if (step < 0)
> @@ -1517,9 +1639,6 @@ init_wait_for_inferior (void)
>
>   breakpoint_init_inferior (inf_starting);
>
> -  /* The first resume is not following a fork/vfork/exec. */
> -  pending_follow.kind = TARGET_WAITKIND_SPURIOUS;      /* I.e., none. */
> -
>   clear_proceed_status ();
>
>   stepping_past_singlestep_breakpoint = 0;
> @@ -1698,8 +1817,6 @@ infrun_thread_stop_requested (ptid_t pti
>   iterate_over_threads (infrun_thread_stop_requested_callback, &ptid);
>  }
>
> -void nullify_last_target_wait_ptid (void);
> -
>  static void
>  infrun_thread_thread_exit (struct thread_info *tp, int silent)
>  {
> @@ -2407,10 +2524,6 @@ handle_inferior_event (struct execution_
>     case TARGET_WAITKIND_VFORKED:
>       if (debug_infrun)
>         fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_FORKED\n");
> -      pending_follow.kind = ecs->ws.kind;
> -
> -      pending_follow.fork_event.parent_pid = ecs->ptid;
> -      pending_follow.fork_event.child_pid = ecs->ws.value.related_pid;
>
>       if (!ptid_equal (ecs->ptid, inferior_ptid))
>        {
> @@ -2439,6 +2552,11 @@ handle_inferior_event (struct execution_
>          detach_breakpoints (child_pid);
>        }
>
> +      /* In case the event is caught by a catchpoint, remember that
> +        the event is to be followed at the next resume of the thread,
> +        and not immediately.  */
> +      ecs->event_thread->pending_follow = ecs->ws;
> +
>       stop_pc = regcache_read_pc (get_thread_regcache (ecs->ptid));
>
>       ecs->event_thread->stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid);
> @@ -2448,8 +2566,19 @@ handle_inferior_event (struct execution_
>       /* If no catchpoint triggered for this, then keep going.  */
>       if (ecs->random_signal)
>        {
> +         int should_resume;
> +
>          ecs->event_thread->stop_signal = TARGET_SIGNAL_0;
> -         keep_going (ecs);
> +
> +         should_resume = follow_fork ();
> +
> +         ecs->event_thread = inferior_thread ();
> +         ecs->ptid = inferior_ptid;
> +
> +         if (should_resume)
> +           keep_going (ecs);
> +         else
> +           stop_stepping (ecs);
>          return;
>        }
>       ecs->event_thread->stop_signal = TARGET_SIGNAL_TRAP;
> Index: src/gdb/linux-nat.c
> ===================================================================
> --- src.orig/gdb/linux-nat.c    2009-05-21 23:45:24.000000000 +0100
> +++ src/gdb/linux-nat.c 2009-05-24 17:09:03.000000000 +0100
> @@ -575,19 +575,17 @@ static int
>  linux_child_follow_fork (struct target_ops *ops, int follow_child)
>  {
>   sigset_t prev_mask;
> -  ptid_t last_ptid;
> -  struct target_waitstatus last_status;
>   int has_vforked;
>   int parent_pid, child_pid;
>
>   block_child_signals (&prev_mask);
>
> -  get_last_target_status (&last_ptid, &last_status);
> -  has_vforked = (last_status.kind == TARGET_WAITKIND_VFORKED);
> -  parent_pid = ptid_get_lwp (last_ptid);
> +  has_vforked = (inferior_thread ()->pending_follow.kind
> +                == TARGET_WAITKIND_VFORKED);
> +  parent_pid = ptid_get_lwp (inferior_ptid);
>   if (parent_pid == 0)
> -    parent_pid = ptid_get_pid (last_ptid);
> -  child_pid = PIDGET (last_status.value.related_pid);
> +    parent_pid = ptid_get_pid (inferior_ptid);
> +  child_pid = PIDGET (inferior_thread ()->pending_follow.value.related_pid);
>
>   if (! follow_child)
>     {
> @@ -625,7 +623,7 @@ linux_child_follow_fork (struct target_o
>          /* Add process to GDB's tables.  */
>          child_inf = add_inferior (child_pid);
>
> -         parent_inf = find_inferior_pid (GET_PID (last_ptid));
> +         parent_inf = current_inferior ();
>          child_inf->attach_flag = parent_inf->attach_flag;
>          copy_terminal_info (child_inf, parent_inf);
>
> @@ -692,21 +690,9 @@ linux_child_follow_fork (struct target_o
>     }
>   else
>     {
> -      struct thread_info *last_tp = find_thread_pid (last_ptid);
>       struct thread_info *tp;
> -      char child_pid_spelling[40];
>       struct inferior *parent_inf, *child_inf;
>
> -      /* Copy user stepping state to the new inferior thread.  */
> -      struct breakpoint *step_resume_breakpoint = last_tp->step_resume_breakpoint;
> -      CORE_ADDR step_range_start = last_tp->step_range_start;
> -      CORE_ADDR step_range_end = last_tp->step_range_end;
> -      struct frame_id step_frame_id = last_tp->step_frame_id;
> -
> -      /* Otherwise, deleting the parent would get rid of this
> -        breakpoint.  */
> -      last_tp->step_resume_breakpoint = NULL;
> -
>       /* Before detaching from the parent, remove all breakpoints from it. */
>       remove_breakpoints ();
>
> @@ -723,7 +709,7 @@ linux_child_follow_fork (struct target_o
>
>       child_inf = add_inferior (child_pid);
>
> -      parent_inf = find_inferior_pid (GET_PID (last_ptid));
> +      parent_inf = current_inferior ();
>       child_inf->attach_flag = parent_inf->attach_flag;
>       copy_terminal_info (child_inf, parent_inf);
>
> @@ -772,15 +758,6 @@ linux_child_follow_fork (struct target_o
>
>       linux_nat_switch_fork (inferior_ptid);
>       check_for_thread_db ();
> -
> -      tp = inferior_thread ();
> -      tp->step_resume_breakpoint = step_resume_breakpoint;
> -      tp->step_range_start = step_range_start;
> -      tp->step_range_end = step_range_end;
> -      tp->step_frame_id = step_frame_id;
> -
> -      /* Reset breakpoints in the child as appropriate.  */
> -      follow_inferior_reset_breakpoints ();
>     }
>
>   restore_child_signals_mask (&prev_mask);
> Index: src/gdb/inf-ptrace.c
> ===================================================================
> --- src.orig/gdb/inf-ptrace.c   2009-05-21 23:45:24.000000000 +0100
> +++ src/gdb/inf-ptrace.c        2009-05-24 18:20:14.000000000 +0100
> @@ -46,20 +46,8 @@ inf_ptrace_follow_fork (struct target_op
>  {
>   pid_t pid, fpid;
>   ptrace_state_t pe;
> -  struct thread_info *last_tp = NULL;
>
> -  /* FIXME: kettenis/20050720: This stuff should really be passed as
> -     an argument by our caller.  */
> -  {
> -    ptid_t ptid;
> -    struct target_waitstatus status;
> -
> -    get_last_target_status (&ptid, &status);
> -    gdb_assert (status.kind == TARGET_WAITKIND_FORKED);
> -
> -    pid = ptid_get_pid (ptid);
> -    last_tp = find_thread_pid (ptid);
> -  }
> +  pid = ptid_get_pid (inferior_ptid);
>
>   if (ptrace (PT_GET_PROCESS_STATE, pid,
>               (PTRACE_TYPE_ARG3)&pe, sizeof pe) == -1)
> @@ -70,18 +58,9 @@ inf_ptrace_follow_fork (struct target_op
>
>   if (follow_child)
>     {
> -      /* Copy user stepping state to the new inferior thread.  */
> -      struct breakpoint *step_resume_breakpoint = last_tp->step_resume_breakpoint;
> -      CORE_ADDR step_range_start = last_tp->step_range_start;
> -      CORE_ADDR step_range_end = last_tp->step_range_end;
> -      struct frame_id step_frame_id = last_tp->step_frame_id;
>       struct inferior *parent_inf, *child_inf;
>       struct thread_info *tp;
>
> -      /* Otherwise, deleting the parent would get rid of this
> -        breakpoint.  */
> -      last_tp->step_resume_breakpoint = NULL;
> -
>       parent_inf = find_inferior_pid (pid);
>
>       /* Add the child.  */
> @@ -102,26 +81,15 @@ inf_ptrace_follow_fork (struct target_op
>       /* Delete the parent.  */
>       detach_inferior (pid);
>
> -      tp = add_thread_silent (inferior_ptid);
> -
> -      tp->step_resume_breakpoint = step_resume_breakpoint;
> -      tp->step_range_start = step_range_start;
> -      tp->step_range_end = step_range_end;
> -      tp->step_frame_id = step_frame_id;
> -
> -      /* Reset breakpoints in the child as appropriate.  */
> -      follow_inferior_reset_breakpoints ();
> +      add_thread_silent (inferior_ptid);
>     }
>   else
>     {
> -      inferior_ptid = pid_to_ptid (pid);
> -
>       /* Breakpoints have already been detached from the child by
>         infrun.c.  */
>
>       if (ptrace (PT_DETACH, fpid, (PTRACE_TYPE_ARG3)1, 0) == -1)
>        perror_with_name (("ptrace"));
> -      detach_inferior (pid);
>     }
>
>   return 0;
> Index: src/gdb/inf-ttrace.c
> ===================================================================
> --- src.orig/gdb/inf-ttrace.c   2009-05-21 23:45:24.000000000 +0100
> +++ src/gdb/inf-ttrace.c        2009-05-24 17:09:03.000000000 +0100
> @@ -412,25 +412,13 @@ inf_ttrace_follow_fork (struct target_op
>   pid_t pid, fpid;
>   lwpid_t lwpid, flwpid;
>   ttstate_t tts;
> -  struct thread_info *last_tp = NULL;
> -  struct breakpoint *step_resume_breakpoint = NULL;
> -  CORE_ADDR step_range_start = 0, step_range_end = 0;
> -  struct frame_id step_frame_id = null_frame_id;
> -
> -  /* FIXME: kettenis/20050720: This stuff should really be passed as
> -     an argument by our caller.  */
> -  {
> -    ptid_t ptid;
> -    struct target_waitstatus status;
> -
> -    get_last_target_status (&ptid, &status);
> -    gdb_assert (status.kind == TARGET_WAITKIND_FORKED
> -               || status.kind == TARGET_WAITKIND_VFORKED);
> -
> -    pid = ptid_get_pid (ptid);
> -    lwpid = ptid_get_lwp (ptid);
> -    last_tp = find_thread_pid (ptid);
> -  }
> +  struct thread_info *tp = inferior_thread ();
> +
> +  gdb_assert (tp->pending_follow.kind == TARGET_WAITKIND_FORKED
> +             || tp->pending_follow.kind == TARGET_WAITKIND_VFORKED);
> +
> +  pid = ptid_get_pid (inferior_ptid);
> +  lwpid = ptid_get_lwp (inferior_ptid);
>
>   /* Get all important details that core GDB doesn't (and shouldn't)
>      know about.  */
> @@ -462,16 +450,6 @@ inf_ttrace_follow_fork (struct target_op
>
>       parent_inf = find_inferior_pid (pid);
>
> -      /* Copy user stepping state to the new inferior thread.  */
> -      step_resume_breakpoint = last_tp->step_resume_breakpoint;
> -      step_range_start = last_tp->step_range_start;
> -      step_range_end = last_tp->step_range_end;
> -      step_frame_id = last_tp->step_frame_id;
> -
> -      /* Otherwise, deleting the parent would get rid of this
> -        breakpoint.  */
> -      last_tp->step_resume_breakpoint = NULL;
> -
>       inferior_ptid = ptid_build (fpid, flwpid, 0);
>       inf = add_inferior (fpid);
>       inf->attach_flag = parent_inf->attach_flag;
> @@ -553,14 +531,6 @@ Detaching after fork from child process
>        xmalloc (sizeof (struct inf_ttrace_private_thread_info));
>       memset (ti->private, 0,
>              sizeof (struct inf_ttrace_private_thread_info));
> -
> -      ti->step_resume_breakpoint = step_resume_breakpoint;
> -      ti->step_range_start = step_range_start;
> -      ti->step_range_end = step_range_end;
> -      ti->step_frame_id = step_frame_id;
> -
> -      /* Reset breakpoints in the child as appropriate.  */
> -      follow_inferior_reset_breakpoints ();
>     }
>
>   return 0;
> Index: src/gdb/testsuite/gdb.threads/fork-thread-pending.c
> ===================================================================
> --- /dev/null   1970-01-01 00:00:00.000000000 +0000
> +++ src/gdb/testsuite/gdb.threads/fork-thread-pending.c 2009-05-24 17:09:03.000000000 +0100
> @@ -0,0 +1,109 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2008, 2009 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/>.  */
> +
> +#include <pthread.h>
> +#include <assert.h>
> +#include <unistd.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <errno.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +
> +#define NUMTHREADS 10
> +
> +volatile int done = 0;
> +
> +static void *
> +start (void *arg)
> +{
> +  while (!done)
> +    usleep (100);
> +  assert (0);
> +  return arg;
> +}
> +
> +void *
> +thread_function (void *arg)
> +{
> +  int x = * (int *) arg;
> +
> +  printf ("Thread <%d> executing\n", x);
> +
> +  while (!done)
> +    usleep (100);
> +
> +  return NULL;
> +}
> +
> +void *
> +thread_forker (void *arg)
> +{
> +  int x = * (int *) arg;
> +  pid_t pid;
> +  int rv;
> +  int i;
> +  pthread_t thread;
> +
> +  printf ("Thread forker <%d> executing\n", x);
> +
> +  switch ((pid = fork ()))
> +    {
> +    case -1:
> +      assert (0);
> +    default:
> +      wait (&rv);
> +      done = 1;
> +      break;
> +    case 0:
> +      i = pthread_create (&thread, NULL, start, NULL);
> +      assert (i == 0);
> +      i = pthread_join (thread, NULL);
> +      assert (i == 0);
> +
> +      assert (0);
> +    }
> +
> +  return NULL;
> +}
> +
> +int
> +main (void)
> +{
> +  pthread_t threads[NUMTHREADS];
> +  int args[NUMTHREADS];
> +  int i, j;
> +
> +  /* Create a few threads that do mostly nothing, and then one that
> +     forks.  */
> +  for (j = 0; j < NUMTHREADS - 1; ++j)
> +    {
> +      args[j] = j;
> +      pthread_create (&threads[j], NULL, thread_function, &args[j]);
> +    }
> +
> +  args[j] = j;
> +  pthread_create (&threads[j], NULL, thread_forker, &args[j]);
> +
> +  for (j = 0; j < NUMTHREADS; ++j)
> +    {
> +      pthread_join (threads[j], NULL);
> +    }
> +
> +  return 0;
> +}
> Index: src/gdb/testsuite/gdb.threads/fork-thread-pending.exp
> ===================================================================
> --- /dev/null   1970-01-01 00:00:00.000000000 +0000
> +++ src/gdb/testsuite/gdb.threads/fork-thread-pending.exp       2009-05-24 17:29:19.000000000 +0100
> @@ -0,0 +1,128 @@
> +# Copyright (C) 2009 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/>.
> +
> +# There's no support for `set follow-fork-mode' in the remote
> +# protocol.
> +if { [is_remote target] } {
> +    return 0
> +}
> +
> +# Only GNU/Linux is known to support `set follow-fork-mode child'.
> +#
> +if { ! [istarget "*-*-linux*"] } {
> +    return 0
> +}
> +
> +set testfile fork-thread-pending
> +set srcfile ${testfile}.c
> +set binfile ${objdir}/${subdir}/${testfile}
> +
> +if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
> +    return -1
> +}
> +
> +gdb_exit
> +gdb_start
> +gdb_reinitialize_dir $srcdir/$subdir
> +
> +gdb_load ${binfile}
> +if ![runto_main] then {
> +   fail "Can't run to main"
> +   return 0
> +}
> +
> +gdb_test "set follow-fork-mode child" "" "1, set follow-fork-mode child"
> +gdb_test "catch fork" "Catchpoint \[0-9\]* \\(fork\\)" "1, insert fork catchpoint"
> +gdb_breakpoint "start" "" "1, set breakpoint at start"
> +
> +gdb_test "continue" "Catchpoint.*" "1, get to the fork event"
> +
> +gdb_test "info threads" " Thread .* Thread .* Thread .* Thread .*" "1, multiple threads found"
> +
> +gdb_test "thread 2" "" "1, switched away from event thread"
> +
> +gdb_test "continue" "Not resuming.*" "1, refused to resume"
> +
> +set test "1, followed to the child, found one thread"
> +gdb_test_multiple "info threads" "metest" {
> +    -re " Thread .* Thread .*$gdb_prompt $" {
> +       fail "$test"
> +    }
> +    -re " Thread .*$gdb_prompt $" {
> +       pass "$test"
> +    }
> +    -re "$gdb_prompt $" {
> +       fail "$test (unknown output)"
> +    }
> +    timeout {
> +       fail "$test (timeout)"
> +    }
> +}
> +
> +gdb_test "continue" "Breakpoint 3, start.*" "1, get to the spawned thread in fork child"
> +
> +set test "1, followed to the child, found two threads"
> +gdb_test_multiple "info threads" "$test" {
> +    -re " Thread .* Thread .* Thread .*$gdb_prompt $" {
> +       fail "$test"
> +    }
> +    -re " Thread .* Thread .*$gdb_prompt $" {
> +       pass "$test"
> +    }
> +    -re "$gdb_prompt $" {
> +       fail "$test (unknown output)"
> +    }
> +    timeout {
> +       fail "$test (timeout)"
> +    }
> +}
> +
> +# Start over, but this time, don't switch away from the fork event thread.
> +
> +gdb_exit
> +gdb_start
> +gdb_reinitialize_dir $srcdir/$subdir
> +
> +gdb_load ${binfile}
> +if ![runto_main] then {
> +   fail "Can't run to main"
> +   return 0
> +}
> +
> +gdb_test "set follow-fork-mode child" "" "2, set follow-fork-mode child"
> +gdb_test "catch fork" "Catchpoint \[0-9\]* \\(fork\\)" "2, insert fork catchpoint"
> +gdb_breakpoint "start"
> +
> +gdb_test "continue" "Catchpoint.*" "2, get to the fork event"
> +
> +gdb_test "info threads" " Thread .* Thread .* Thread .* Thread .*" "2, multiple threads found"
> +
> +gdb_test "continue" "Breakpoint 3, start.*" "2, get to the spawned thread in fork child"
> +
> +set test "2, followed to the child, found two threads"
> +gdb_test_multiple "info threads" "$test" {
> +    -re " Thread .* Thread .* Thread .*$gdb_prompt $" {
> +       fail "$test"
> +    }
> +    -re " Thread .* Thread .*$gdb_prompt $" {
> +       pass "$test"
> +    }
> +    -re "$gdb_prompt $" {
> +       fail "$test (unknown output)"
> +    }
> +    timeout {
> +       fail "$test (timeout)"
> +    }
> +}
>

fyi, I just did an update and am getting a build failure (i686-linux).

gcc -m32 -g -O2   -I. -I../../../src/gdb -I../../../src/gdb/common
-I../../../src/gdb/config -DLOCALEDIR="\"/usr/share/locale\""
-DHAVE_CONFIG_H -I../../../src/\
gdb/../include/opcode -I../../../src/gdb/../readline/.. -I../bfd
-I../../../src/gdb/../bfd -I../../../src/gdb/../include
-I../libdecnumber -I../../../src/gdb/..\
/libdecnumber  -I../../../src/gdb/gnulib -Ignulib  -DMI_OUT=1 -DTUI=1
-Wall -Wdeclaration-after-statement -Wpointer-arith
-Wformat-nonliteral -Wno-pointer-sign\
 -Wno-unused -Wno-switch -Wno-char-subscripts -Werror -c -o infrun.o
-MT infrun.o -MMD -MP -MF .deps/infrun.Tpo ../../../src/gdb/infrun.c
cc1: warnings being treated as errors
../../../src/gdb/infrun.c: In function ‘follow_fork’:
../../../src/gdb/infrun.c:298: warning: ‘step_frame_id.special_addr_p’
may be used uninitialized in this function
../../../src/gdb/infrun.c:298: warning: ‘step_frame_id.code_addr_p’
may be used uninitialized in this function
../../../src/gdb/infrun.c:298: warning: ‘step_frame_id.stack_addr_p’
may be used uninitialized in this function
../../../src/gdb/infrun.c:298: warning: ‘step_frame_id.special_addr’
may be used uninitialized in this function
../../../src/gdb/infrun.c:298: warning: ‘step_frame_id.code_addr’ may
be used uninitialized in this function
../../../src/gdb/infrun.c:298: warning: ‘step_frame_id.stack_addr’ may
be used uninitialized in this function
../../../src/gdb/infrun.c:297: warning: ‘step_range_end’ may be used
uninitialized in this function
../../../src/gdb/infrun.c:296: warning: ‘step_range_start’ may be used
uninitialized in this function
../../../src/gdb/infrun.c:295: warning: ‘step_resume_breakpoint’ may
be used uninitialized in this function
make: *** [infrun.o] Error 1

gcc's not smart enough to detect it's actually ok.

2009-05-24  Doug Evans  <dje@google.com>

        * infrun.c (follow_fork): Initialize new step_* locals
        to avoid "may be used uninitialized" warnings.

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.377
diff -u -p -r1.377 infrun.c
--- infrun.c    24 May 2009 18:00:08 -0000      1.377
+++ infrun.c    24 May 2009 20:26:28 -0000
@@ -290,12 +290,12 @@ follow_fork (void)

   /* Copy user stepping state to the new inferior thread.  FIXME: the
      followed fork child thread should have a copy of most of the
-     parent thread structure's run control related fields, not just
-     these.  */
-  struct breakpoint *step_resume_breakpoint;
-  CORE_ADDR step_range_start;
-  CORE_ADDR step_range_end;
-  struct frame_id step_frame_id;
+     parent thread structure's run control related fields, not just these.
+     Initialized to avoid "may be used uninitialized" warnings from gcc.  */
+  struct breakpoint *step_resume_breakpoint = NULL;
+  CORE_ADDR step_range_start = 0;
+  CORE_ADDR step_range_end = 0;
+  struct frame_id step_frame_id = { 0 };

   if (!non_stop)
     {

Re: [RFA] Fix build failure [was Re: [RFC] fork in multi-threaded apps, per-thread pending_fork, other follow-fork cleanups.]

[Pedro Alves]

On Sunday 24 May 2009 21:27:30, Doug Evans wrote:

> gcc's not smart enough to detect it's actually ok.

Sigh...

> 2009-05-24  Doug Evans  <dje@google.com>
> 
>         * infrun.c (follow_fork): Initialize new step_* locals
>         to avoid "may be used uninitialized" warnings.

Ok, thanks.

-- 
Pedro Alves