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

[Pedro Alves <pedro at codesourcery dot 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