* [RFA] Resubmit process record and replay, 4/10
@ 2008-11-16 23:37 teawater
2008-11-20 4:21 ` Michael Snyder
0 siblings, 1 reply; 4+ messages in thread
From: teawater @ 2008-11-16 23:37 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 2402 bytes --]
The process record and replay target has six function pointers
record_beneath_to_resume, record_beneath_to_wait,
record_beneath_to_prepare_to_store, record_beneath_to_xfer_partial,
record_beneath_to_insert_breakpoint and
record_beneath_to_remove_breakpoint.
This patch add code to function "update_current_target" make these
pointers always point to the function of lower strata target than
process record and replay target.
When GDB in record mode, we need to call the really debug function in
low strata target because process record and replay target need call
this function to control the inferior.
Struct target_ops already has a pointer "beneath" point to low strata
target, but process record and replay target doesn't use it. Because
if low strata target doesn't set some function pointers, process
record and replay target will need to call the function pointers of
the target that is low strata target of this target. If this target
doesn't set them too, it will need to call anothers. So use "beneath"
is not a good choice and "multi-thread" target that need function
pointers of low strata target doesn't use "beneath" too.
2008-11-16 Hui Zhu <teawater@gmail.com>
* target.c (update_current_target): Set function points
record_beneath_to_resume,
record_beneath_to_wait,
record_beneath_to_prepare_to_store,
record_beneath_to_xfer_partial,
record_beneath_to_insert_breakpoint
and record_beneath_to_remove_breakpoint point to the function
of target that lower than process record and replay target.
target.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
Following is the diff with the previous patch:
@@ -8,7 +8,7 @@
static void target_info (char *, int);
-@@ -387,6 +388,12 @@ update_current_target (void)
+@@ -405,6 +406,12 @@
if (!current_target.FIELD) \
current_target.FIELD = (TARGET)->FIELD
@@ -21,12 +21,12 @@
for (t = target_stack; t; t = t->beneath)
{
INHERIT (to_shortname, t);
-@@ -476,6 +483,35 @@ update_current_target (void)
+@@ -494,6 +501,35 @@
/* Do not inherit to_memory_map. */
/* Do not inherit to_flash_erase. */
/* Do not inherit to_flash_done. */
+
-+ /* Set the real beneath function pointers. */
++ /* Set pointers to functions in the target beneath us. */
+ if (t != &record_ops)
+ {
+ if (!record_beneath_to_resume)
[-- Attachment #2: 4-target_record_beneath.txt --]
[-- Type: text/plain, Size: 1791 bytes --]
--- a/target.c
+++ b/target.c
@@ -41,6 +41,7 @@
#include "target-descriptions.h"
#include "gdbthread.h"
#include "solib.h"
+#include "record.h"
static void target_info (char *, int);
@@ -405,6 +406,12 @@
if (!current_target.FIELD) \
current_target.FIELD = (TARGET)->FIELD
+ record_beneath_to_resume = NULL;
+ record_beneath_to_store_registers = NULL;
+ record_beneath_to_xfer_partial = NULL;
+ record_beneath_to_insert_breakpoint = NULL;
+ record_beneath_to_remove_breakpoint = NULL;
+
for (t = target_stack; t; t = t->beneath)
{
INHERIT (to_shortname, t);
@@ -494,6 +501,35 @@
/* Do not inherit to_memory_map. */
/* Do not inherit to_flash_erase. */
/* Do not inherit to_flash_done. */
+
+ /* Set pointers to functions in the target beneath us. */
+ if (t != &record_ops)
+ {
+ if (!record_beneath_to_resume)
+ {
+ record_beneath_to_resume = t->to_resume;
+ }
+ if (!record_beneath_to_wait)
+ {
+ record_beneath_to_wait = t->to_wait;
+ }
+ if (!record_beneath_to_store_registers)
+ {
+ record_beneath_to_store_registers = t->to_store_registers;
+ }
+ if (!record_beneath_to_xfer_partial)
+ {
+ record_beneath_to_xfer_partial = t->to_xfer_partial;
+ }
+ if (!record_beneath_to_insert_breakpoint)
+ {
+ record_beneath_to_insert_breakpoint = t->to_insert_breakpoint;
+ }
+ if (!record_beneath_to_remove_breakpoint)
+ {
+ record_beneath_to_remove_breakpoint = t->to_remove_breakpoint;
+ }
+ }
}
#undef INHERIT
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [RFA] Resubmit process record and replay, 4/10
2008-11-16 23:37 [RFA] Resubmit process record and replay, 4/10 teawater
@ 2008-11-20 4:21 ` Michael Snyder
2008-11-20 4:25 ` Pedro Alves
0 siblings, 1 reply; 4+ messages in thread
From: Michael Snyder @ 2008-11-20 4:21 UTC (permalink / raw)
To: teawater; +Cc: gdb-patches
Seems like a couple of people had some doubts about this
particular part of the implementation.
Does anyone have any alternative suggestions?
teawater wrote:
> The process record and replay target has six function pointers
> record_beneath_to_resume, record_beneath_to_wait,
> record_beneath_to_prepare_to_store, record_beneath_to_xfer_partial,
> record_beneath_to_insert_breakpoint and
> record_beneath_to_remove_breakpoint.
> This patch add code to function "update_current_target" make these
> pointers always point to the function of lower strata target than
> process record and replay target.
>
> When GDB in record mode, we need to call the really debug function in
> low strata target because process record and replay target need call
> this function to control the inferior.
>
> Struct target_ops already has a pointer "beneath" point to low strata
> target, but process record and replay target doesn't use it. Because
> if low strata target doesn't set some function pointers, process
> record and replay target will need to call the function pointers of
> the target that is low strata target of this target. If this target
> doesn't set them too, it will need to call anothers. So use "beneath"
> is not a good choice and "multi-thread" target that need function
> pointers of low strata target doesn't use "beneath" too.
>
> 2008-11-16 Hui Zhu <teawater@gmail.com>
>
> * target.c (update_current_target): Set function points
> record_beneath_to_resume,
> record_beneath_to_wait,
> record_beneath_to_prepare_to_store,
> record_beneath_to_xfer_partial,
> record_beneath_to_insert_breakpoint
> and record_beneath_to_remove_breakpoint point to the function
> of target that lower than process record and replay target.
>
> target.c | 36 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> Following is the diff with the previous patch:
> @@ -8,7 +8,7 @@
>
> static void target_info (char *, int);
>
> -@@ -387,6 +388,12 @@ update_current_target (void)
> +@@ -405,6 +406,12 @@
> if (!current_target.FIELD) \
> current_target.FIELD = (TARGET)->FIELD
>
> @@ -21,12 +21,12 @@
> for (t = target_stack; t; t = t->beneath)
> {
> INHERIT (to_shortname, t);
> -@@ -476,6 +483,35 @@ update_current_target (void)
> +@@ -494,6 +501,35 @@
> /* Do not inherit to_memory_map. */
> /* Do not inherit to_flash_erase. */
> /* Do not inherit to_flash_done. */
> +
> -+ /* Set the real beneath function pointers. */
> ++ /* Set pointers to functions in the target beneath us. */
> + if (t != &record_ops)
> + {
> + if (!record_beneath_to_resume)
>
>
> ------------------------------------------------------------------------
>
> --- a/target.c
> +++ b/target.c
> @@ -41,6 +41,7 @@
> #include "target-descriptions.h"
> #include "gdbthread.h"
> #include "solib.h"
> +#include "record.h"
>
> static void target_info (char *, int);
>
> @@ -405,6 +406,12 @@
> if (!current_target.FIELD) \
> current_target.FIELD = (TARGET)->FIELD
>
> + record_beneath_to_resume = NULL;
> + record_beneath_to_store_registers = NULL;
> + record_beneath_to_xfer_partial = NULL;
> + record_beneath_to_insert_breakpoint = NULL;
> + record_beneath_to_remove_breakpoint = NULL;
> +
> for (t = target_stack; t; t = t->beneath)
> {
> INHERIT (to_shortname, t);
> @@ -494,6 +501,35 @@
> /* Do not inherit to_memory_map. */
> /* Do not inherit to_flash_erase. */
> /* Do not inherit to_flash_done. */
> +
> + /* Set pointers to functions in the target beneath us. */
> + if (t != &record_ops)
> + {
> + if (!record_beneath_to_resume)
> + {
> + record_beneath_to_resume = t->to_resume;
> + }
> + if (!record_beneath_to_wait)
> + {
> + record_beneath_to_wait = t->to_wait;
> + }
> + if (!record_beneath_to_store_registers)
> + {
> + record_beneath_to_store_registers = t->to_store_registers;
> + }
> + if (!record_beneath_to_xfer_partial)
> + {
> + record_beneath_to_xfer_partial = t->to_xfer_partial;
> + }
> + if (!record_beneath_to_insert_breakpoint)
> + {
> + record_beneath_to_insert_breakpoint = t->to_insert_breakpoint;
> + }
> + if (!record_beneath_to_remove_breakpoint)
> + {
> + record_beneath_to_remove_breakpoint = t->to_remove_breakpoint;
> + }
> + }
> }
> #undef INHERIT
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [RFA] Resubmit process record and replay, 4/10
2008-11-20 4:21 ` Michael Snyder
@ 2008-11-20 4:25 ` Pedro Alves
2008-11-20 6:42 ` Michael Snyder
0 siblings, 1 reply; 4+ messages in thread
From: Pedro Alves @ 2008-11-20 4:25 UTC (permalink / raw)
To: gdb-patches; +Cc: Michael Snyder, teawater
A Thursday 20 November 2008 01:46:05, Michael Snyder escreveu:
> Seems like a couple of people had some doubts about this
> particular part of the implementation.
>
> Does anyone have any alternative suggestions?
>
Sounds like a deficiency of the target interface, as most target_ops
functions don't take a "this" pointer and do the "beneath"
lookup themselves.
Would doing something like this work here? :
for (t = current_target.beneath; t != NULL; t = t->beneath)
if (t->to_xfer_partial != NULL)
{
t->to_xfer_partial (...);
return;
}
etc., that is don't stop looking at the immediate
target beneath, but look until you find one that suits instead.
I'd prefer that the core side isn't special cased for
the record target. If need be, we can add, change or adapt
the target interfaces.
--
Pedro Alves
> teawater wrote:
> > The process record and replay target has six function pointers
> > record_beneath_to_resume, record_beneath_to_wait,
> > record_beneath_to_prepare_to_store, record_beneath_to_xfer_partial,
> > record_beneath_to_insert_breakpoint and
> > record_beneath_to_remove_breakpoint.
> > This patch add code to function "update_current_target" make these
> > pointers always point to the function of lower strata target than
> > process record and replay target.
> >
> > When GDB in record mode, we need to call the really debug function in
> > low strata target because process record and replay target need call
> > this function to control the inferior.
> >
> > Struct target_ops already has a pointer "beneath" point to low strata
> > target, but process record and replay target doesn't use it. Because
> > if low strata target doesn't set some function pointers, process
> > record and replay target will need to call the function pointers of
> > the target that is low strata target of this target. If this target
> > doesn't set them too, it will need to call anothers. So use "beneath"
> > is not a good choice and "multi-thread" target that need function
> > pointers of low strata target doesn't use "beneath" too.
> >
> > 2008-11-16 Hui Zhu <teawater@gmail.com>
> >
> > * target.c (update_current_target): Set function points
> > record_beneath_to_resume,
> > record_beneath_to_wait,
> > record_beneath_to_prepare_to_store,
> > record_beneath_to_xfer_partial,
> > record_beneath_to_insert_breakpoint
> > and record_beneath_to_remove_breakpoint point to the function
> > of target that lower than process record and replay target.
> >
> > target.c | 36 ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 36 insertions(+)
> >
> > Following is the diff with the previous patch:
> > @@ -8,7 +8,7 @@
> >
> > static void target_info (char *, int);
> >
> > -@@ -387,6 +388,12 @@ update_current_target (void)
> > +@@ -405,6 +406,12 @@
> > if (!current_target.FIELD) \
> > current_target.FIELD = (TARGET)->FIELD
> >
> > @@ -21,12 +21,12 @@
> > for (t = target_stack; t; t = t->beneath)
> > {
> > INHERIT (to_shortname, t);
> > -@@ -476,6 +483,35 @@ update_current_target (void)
> > +@@ -494,6 +501,35 @@
> > /* Do not inherit to_memory_map. */
> > /* Do not inherit to_flash_erase. */
> > /* Do not inherit to_flash_done. */
> > +
> > -+ /* Set the real beneath function pointers. */
> > ++ /* Set pointers to functions in the target beneath us. */
> > + if (t != &record_ops)
> > + {
> > + if (!record_beneath_to_resume)
> >
> >
> > ------------------------------------------------------------------------
> >
> > --- a/target.c
> > +++ b/target.c
> > @@ -41,6 +41,7 @@
> > #include "target-descriptions.h"
> > #include "gdbthread.h"
> > #include "solib.h"
> > +#include "record.h"
> >
> > static void target_info (char *, int);
> >
> > @@ -405,6 +406,12 @@
> > if (!current_target.FIELD) \
> > current_target.FIELD = (TARGET)->FIELD
> >
> > + record_beneath_to_resume = NULL;
> > + record_beneath_to_store_registers = NULL;
> > + record_beneath_to_xfer_partial = NULL;
> > + record_beneath_to_insert_breakpoint = NULL;
> > + record_beneath_to_remove_breakpoint = NULL;
> > +
> > for (t = target_stack; t; t = t->beneath)
> > {
> > INHERIT (to_shortname, t);
> > @@ -494,6 +501,35 @@
> > /* Do not inherit to_memory_map. */
> > /* Do not inherit to_flash_erase. */
> > /* Do not inherit to_flash_done. */
> > +
> > + /* Set pointers to functions in the target beneath us. */
> > + if (t != &record_ops)
> > + {
> > + if (!record_beneath_to_resume)
> > + {
> > + record_beneath_to_resume = t->to_resume;
> > + }
> > + if (!record_beneath_to_wait)
> > + {
> > + record_beneath_to_wait = t->to_wait;
> > + }
> > + if (!record_beneath_to_store_registers)
> > + {
> > + record_beneath_to_store_registers = t->to_store_registers;
> > + }
> > + if (!record_beneath_to_xfer_partial)
> > + {
> > + record_beneath_to_xfer_partial = t->to_xfer_partial;
> > + }
> > + if (!record_beneath_to_insert_breakpoint)
> > + {
> > + record_beneath_to_insert_breakpoint = t->to_insert_breakpoint;
> > + }
> > + if (!record_beneath_to_remove_breakpoint)
> > + {
> > + record_beneath_to_remove_breakpoint = t->to_remove_breakpoint;
> > + }
> > + }
> > }
> > #undef INHERIT
> >
>
>
--
Pedro Alves
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [RFA] Resubmit process record and replay, 4/10
2008-11-20 4:25 ` Pedro Alves
@ 2008-11-20 6:42 ` Michael Snyder
0 siblings, 0 replies; 4+ messages in thread
From: Michael Snyder @ 2008-11-20 6:42 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, teawater
Pedro Alves wrote:
> A Thursday 20 November 2008 01:46:05, Michael Snyder escreveu:
>> Seems like a couple of people had some doubts about this
>> particular part of the implementation.
>>
>> Does anyone have any alternative suggestions?
>>
>
> Sounds like a deficiency of the target interface, as most target_ops
> functions don't take a "this" pointer and do the "beneath"
> lookup themselves.
>
> Would doing something like this work here? :
>
> for (t = current_target.beneath; t != NULL; t = t->beneath)
> if (t->to_xfer_partial != NULL)
> {
> t->to_xfer_partial (...);
> return;
> }
>
> etc., that is don't stop looking at the immediate
> target beneath, but look until you find one that suits instead.
>
> I'd prefer that the core side isn't special cased for
> the record target. If need be, we can add, change or adapt
> the target interfaces.
>
I think that you're right that the "target beneath" functionality
is incomplete as it is.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-11-20 2:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-11-16 23:37 [RFA] Resubmit process record and replay, 4/10 teawater
2008-11-20 4:21 ` Michael Snyder
2008-11-20 4:25 ` Pedro Alves
2008-11-20 6:42 ` Michael Snyder
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox