Re: [RFC] Add support of software single step to process record

[Pedro Alves <pedro@codesourcery.com>]

On Wednesday 23 December 2009 09:23:21, Hui Zhu wrote:
> +      struct gdbarch *gdbarch = target_thread_architecture (ptid);
> +
>        record_message (get_current_regcache (), signal);

>        record_beneath_to_resume (record_beneath_to_resume_ops, ptid, 1,
>                                  signal);

Why is this resume call still present?

> +
> +       if (gdbarch_software_single_step_p (gdbarch))
> +         {
> +           if (!inserted_single_step_breakpoint_p ())

Isn't this naming stale?  I thought you had renamed this.

> +             gdbarch_software_single_step (gdbarch, get_current_frame ());
> +           record_beneath_to_resume (record_beneath_to_resume_ops,
> +                                     ptid, step, signal);
> +           record_resume_step = 0;
> +         }
> +       else
> +         record_beneath_to_resume (record_beneath_to_resume_ops, ptid, 1,
> +                                   signal);
>      }
> 

You've got the predicates a bit mixed up.

 - gdbarch_software_single_step_p purpose is only "is there or
   not a gdbarch_software_single_step callback registered in
   this gdbarch"?  It returning true does not mean that
   software single-step should be used for that single-step.

 - gdbarch_software_single_step can return false, meaning,
   no software single-step needs to be used.

This is how stepping over atomic sequences is handled
currently (grep for deal_with_atomic_sequence):
gdbarch_software_single_step_p returns true, but
gdbarch_software_single_step returns false most
of the times.  See also infrun.c:maybe_software_singlestep.

I think you want this:

       if (!step
           && gdbarch_software_single_step_p (gdbarch)
           && !single_step_breakpoints_inserted ()
           && gdbarch_software_single_step (gdbarch, get_current_frame ()))
         record_resume_step = 0;
       else
         record_resume_step = 1;

       record_beneath_to_resume (record_beneath_to_resume_ops, ptid,
                                 record_resume_step, signal);

If `step' is true when record_resume is called, and so is
gdbarch_software_single_step_p, then it must be that infrun.c
already determined that gdbarch_software_single_step returns
false, otherwise, `step' would be false (maybe_software_singlestep).

If `step' is false (the user is requesting a continue), and
no single-step breakpoints are inserted yet, but,
gdbarch_software_single_step returns false, we have ourselves
an arch/target combo that only wants software single-stepping
for atomic sequences, e.g., MIPS (non-linux), or PPC.  If
so, we should force hardware single-step in the target
beneath (set record_resume_step to 1).

-- 
Pedro Alves