Re: [RFA] Submit process record and replay third time, 3/9

[teawater <teawater@gmail.com>]

I had remove the "TARGET_IS_PROCESS_RECORD".

Thanks,
Hui

On Wed, Mar 11, 2009 at 02:00, Pedro Alves <pedro@codesourcery.com> wrote:
> On Tuesday 10 March 2009 17:02:46, teawater wrote:
>> >> +#define TARGET_IS_PROCESS_RECORD   \
>> >> +     (current_target.beneath == &record_ops)
>> >
>> > Sorry, but I repeat the request I've made several times already.  This is
>> > not the right way to do this.  You need to add a new target_ops method or
>> > property that the core of GDB checks on.  It is not correct that make
>> > the core of GDB reference record_ops directly.  To come up with
>> > the target callback's name, at each call site of TARGET_IS_PROCESS_RECORD,
>> > consider what is the property of the current target that GDB needs to
>> > know about the current target.  Is it something like:
>> >
>> >  target_is_recording () ?
>> >  target_is_replaying () ?
>> >  target_is_read_only () ?
>> >
>> > etc.
>> >
>>
>> I forget a process record has special strata "record_stratum".
>>
>> What about delete "TARGET_IS_PROCESS_RECORD" and add
>> #define target_is_record (target) (target->to_stratum == record_stratum)
>> to target.h?
>
> No, that's not a new callback...
>
> If we in some hypothetical future end up layering yet another target
> on top of record, then that check will fail.
>
> What I'm saying is, TARGET_IS_PROCESS_RECORD is just as wrong
> as TARGET_IS_LINUX_NAT_C, or TARGET_IS_WINDOWS_NAT_C.
>
> E.g., in places in the common code that we want to check if the
> current target has execution, we call target_has_execution, doesn't
> matter which target it is, as long as it has execution.  If we want to
> check that the target is in asynchronous more, we check for
> target_is_async_p, again, doesn't matter which target it is.
>
> In your case, imagine that you implemented all of record.c in
> gdbserver instead of on GDB.  Say, imagine that there's no record.c in
> GDB at all.  Then, when you connected to gdbserver, and told it to
> start recording, the topmost pushed target on the GDB side would still be
> the remote target (process stratum).  GDB wouldn't know how gdbserver
> was implementing the recording feature, only that the remote side supports
> the recording feature.  Now, see, here's a property we'd possibly
> want to expose through a target method --- e.g., target_can_record_p().
>
> If you look at the places in the core of GDB where you are
> currently checking for TARGET_IS_PROCESS_RECORD, and you wanted those
> checks to also return true in the case of precord being implemented
> on the remote server, clearly, you'd need to check for some
> target property other than the target name, or its stratum.
>
> Why is it that you need to call TARGET_IS_PROCESS_RECORD in the
> first place?   That is the key here.  Again, is it because the
> target is recording, and you can't do some things in that case?
> Is it because the target is replaying?  Or perhaps it's a more
> fundamental state --- is the target in a read only state?  When you
> find out which *state(s)* you're interesting in checking, we can
> easily add target method(s) for it/them, and make the record
> target implement them (say, returning true), and making the
> default return false.  Or, we may even come to the conclusion
> that is not a target method we want --- we may end up with
> a global variable, similar to `execution_direction'.
>
> --
> Pedro Alves
>