* [reverse RFC] Fix some bugs
@ 2008-09-18 17:17 teawater
2008-09-18 18:43 ` Michael Snyder
0 siblings, 1 reply; 3+ messages in thread
From: teawater @ 2008-09-18 17:17 UTC (permalink / raw)
To: Michael Snyder, gdb-patches
[-- Attachment #1: Type: text/plain, Size: 317 bytes --]
Thanks for the test and bug report of Michael. It help me a lot.
2008-09-19 Hui Zhu <teawater@gmail.com>
* record.c (record_wait): Fix the bug of forware step after
reverse step.
Make debug message more clean.
Remove variable record_list_status.
* i386-tdep.c (i386_record): Fix the bug of "0x80 ... 0x83".
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 20080919.patch --]
[-- Type: text/x-diff; name=20080919.patch, Size: 5026 bytes --]
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2008-09-19 Hui Zhu <teawater@gmail.com>
+
+ * record.c (record_wait): Fix the bug of forware step after
+ reverse step.
+ Make debug message more clean.
+ Remove variable record_list_status.
+
+ * i386-tdep.c (i386_record): Fix the bug of "0x80 ... 0x83".
+
2008-09-15 Michael Snyder <msnyder@vmware.com>
* infrun.c (proceed): No need to singlestep over a breakpoint
--- a/i386-tdep.c
+++ b/i386-tdep.c
@@ -3037,11 +3037,7 @@ reswitch:
}
else
{
- if (ot == OT_BYTE)
- {
- reg &= 0x3;
- }
- if (record_arch_list_add_reg (reg))
+ if (record_arch_list_add_reg (rm))
{
return (-1);
}
--- a/record.c
+++ b/record.c
@@ -33,7 +33,6 @@ int record_debug = 0;
record_t record_first;
record_t *record_list = &record_first;
-int record_list_status = 1; /* 0 normal 1 to the begin 2 to the end */
record_t *record_arch_list_head = NULL;
record_t *record_arch_list_tail = NULL;
struct regcache *record_regcache = NULL;
@@ -446,7 +445,6 @@ record_open (char *name, int from_tty)
/* Reset */
record_insn_num = 0;
- record_list_status = 1;
record_execdir = EXEC_FORWARD;
record_list = &record_first;
record_list->next = NULL;
@@ -546,30 +544,31 @@ record_wait (ptid_t ptid, struct target_
Then set it to terminal_ours to make GDB get the signal. */
target_terminal_ours ();
- /* Loop over the record log, looking for the next place to stop. */
+ /* In EXEC_FORWARD mode,, record_list point to the tail of prev
+ instruction. */
+ if (record_execdir == EXEC_FORWARD && record_list->next)
+ {
+ record_list = record_list->next;
+ }
+
+ /* Loop over the record_list, looking for the next place to stop. */
do
{
/* check state */
- if ((record_execdir == EXEC_REVERSE && !record_list->prev
- && record_list_status == 1)
- || (record_execdir != EXEC_REVERSE
- && !record_list->next
- && record_list_status == 2))
+ if (record_execdir == EXEC_REVERSE && record_list == &record_first)
{
- if (record_list_status == 2)
- {
- fprintf_unfiltered (gdb_stdlog,
- "Record: running to the end of record list.\n");
- }
- else if (record_list_status == 1)
- {
- fprintf_unfiltered (gdb_stdlog,
- "Record: running to the begin of record list.\n");
- }
+ fprintf_unfiltered (gdb_stdlog,
+ "Record: running to the begin of record list.\n");
+ stop_soon = STOP_QUIETLY;
+ break;
+ }
+ if (record_execdir != EXEC_REVERSE && !record_list->next)
+ {
+ fprintf_unfiltered (gdb_stdlog,
+ "Record: running to the end of record list.\n");
stop_soon = STOP_QUIETLY;
break;
}
- record_list_status = 0;
/* set ptid, register and memory according to record_list */
if (record_list->type == record_reg)
@@ -579,7 +578,8 @@ record_wait (ptid_t ptid, struct target_
if (record_debug > 1)
{
fprintf_unfiltered (gdb_stdlog,
- "Record: record_reg to inferior num = %d.\n",
+ "Record: record_reg 0x%s to inferior num = %d.\n",
+ paddr_nz ((CORE_ADDR)record_list),
record_list->u.reg.num);
}
regcache_cooked_read (regcache, record_list->u.reg.num, reg);
@@ -594,7 +594,8 @@ record_wait (ptid_t ptid, struct target_
if (record_debug > 1)
{
fprintf_unfiltered (gdb_stdlog,
- "Record: record_mem to inferior addr = 0x%s len = %d.\n",
+ "Record: record_mem 0x%s to inferior addr = 0x%s len = %d.\n",
+ paddr_nz ((CORE_ADDR)record_list),
paddr_nz (record_list->u.mem.addr),
record_list->u.mem.len);
}
@@ -625,19 +626,13 @@ record_wait (ptid_t ptid, struct target_
if (record_debug > 1)
{
fprintf_unfiltered (gdb_stdlog,
- "Record: record_end to inferior need_dasm = %d.\n",
+ "Record: record_end 0x%s to inferior need_dasm = %d.\n",
+ paddr_nz ((CORE_ADDR)record_list),
record_list->u.need_dasm);
}
if (record_execdir == EXEC_FORWARD)
{
- if (record_list == &record_first)
- {
- /* The first record_t, not a really record_t.
- Goto next record_t. */
- goto next;
- }
-
need_dasm = record_list->u.need_dasm;
}
if (need_dasm)
@@ -706,21 +701,19 @@ record_wait (ptid_t ptid, struct target_
}
next:
- if (record_execdir == EXEC_REVERSE)
- {
- if (record_list->prev && continue_flag)
- record_list = record_list->prev;
- else
- record_list_status = 1;
- }
- else
+ if (continue_flag)
{
- if (record_list->next)
- record_list = record_list->next;
+ if (record_execdir == EXEC_REVERSE)
+ {
+ if (record_list->prev)
+ record_list = record_list->prev;
+ }
else
- record_list_status = 2;
+ {
+ if (record_list->next)
+ record_list = record_list->next;
+ }
}
-
}
while (continue_flag);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [reverse RFC] Fix some bugs
2008-09-18 17:17 [reverse RFC] Fix some bugs teawater
@ 2008-09-18 18:43 ` Michael Snyder
2008-09-19 3:37 ` teawater
0 siblings, 1 reply; 3+ messages in thread
From: Michael Snyder @ 2008-09-18 18:43 UTC (permalink / raw)
To: teawater; +Cc: gdb-patches
teawater wrote:
> Thanks for the test and bug report of Michael. It help me a lot.
>
> 2008-09-19 Hui Zhu <teawater@gmail.com>
>
> * record.c (record_wait): Fix the bug of forware step after
> reverse step.
> Make debug message more clean.
> Remove variable record_list_status.
>
> * i386-tdep.c (i386_record): Fix the bug of "0x80 ... 0x83".
Hui, are all these changes related?
It looks a little like you might have combined several
smaller changes into one patch. In order to make review
easier, we like to have unrelated patches submitted separately.
If they are in fact related, could you explain please?
Maybe add a few more comments?
Thanks a lot!
> ------------------------------------------------------------------------
>
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,12 @@
> +2008-09-19 Hui Zhu <teawater@gmail.com>
> +
> + * record.c (record_wait): Fix the bug of forware step after
> + reverse step.
> + Make debug message more clean.
> + Remove variable record_list_status.
> +
> + * i386-tdep.c (i386_record): Fix the bug of "0x80 ... 0x83".
> +
> 2008-09-15 Michael Snyder <msnyder@vmware.com>
>
> * infrun.c (proceed): No need to singlestep over a breakpoint
> --- a/i386-tdep.c
> +++ b/i386-tdep.c
> @@ -3037,11 +3037,7 @@ reswitch:
> }
> else
> {
> - if (ot == OT_BYTE)
> - {
> - reg &= 0x3;
> - }
> - if (record_arch_list_add_reg (reg))
> + if (record_arch_list_add_reg (rm))
> {
> return (-1);
> }
> --- a/record.c
> +++ b/record.c
> @@ -33,7 +33,6 @@ int record_debug = 0;
>
> record_t record_first;
> record_t *record_list = &record_first;
> -int record_list_status = 1; /* 0 normal 1 to the begin 2 to the end */
> record_t *record_arch_list_head = NULL;
> record_t *record_arch_list_tail = NULL;
> struct regcache *record_regcache = NULL;
> @@ -446,7 +445,6 @@ record_open (char *name, int from_tty)
>
> /* Reset */
> record_insn_num = 0;
> - record_list_status = 1;
> record_execdir = EXEC_FORWARD;
> record_list = &record_first;
> record_list->next = NULL;
> @@ -546,30 +544,31 @@ record_wait (ptid_t ptid, struct target_
> Then set it to terminal_ours to make GDB get the signal. */
> target_terminal_ours ();
>
> - /* Loop over the record log, looking for the next place to stop. */
> + /* In EXEC_FORWARD mode,, record_list point to the tail of prev
> + instruction. */
> + if (record_execdir == EXEC_FORWARD && record_list->next)
> + {
> + record_list = record_list->next;
> + }
> +
> + /* Loop over the record_list, looking for the next place to stop. */
> do
> {
> /* check state */
> - if ((record_execdir == EXEC_REVERSE && !record_list->prev
> - && record_list_status == 1)
> - || (record_execdir != EXEC_REVERSE
> - && !record_list->next
> - && record_list_status == 2))
> + if (record_execdir == EXEC_REVERSE && record_list == &record_first)
> {
> - if (record_list_status == 2)
> - {
> - fprintf_unfiltered (gdb_stdlog,
> - "Record: running to the end of record list.\n");
> - }
> - else if (record_list_status == 1)
> - {
> - fprintf_unfiltered (gdb_stdlog,
> - "Record: running to the begin of record list.\n");
> - }
> + fprintf_unfiltered (gdb_stdlog,
> + "Record: running to the begin of record list.\n");
> + stop_soon = STOP_QUIETLY;
> + break;
> + }
> + if (record_execdir != EXEC_REVERSE && !record_list->next)
> + {
> + fprintf_unfiltered (gdb_stdlog,
> + "Record: running to the end of record list.\n");
> stop_soon = STOP_QUIETLY;
> break;
> }
> - record_list_status = 0;
>
> /* set ptid, register and memory according to record_list */
> if (record_list->type == record_reg)
> @@ -579,7 +578,8 @@ record_wait (ptid_t ptid, struct target_
> if (record_debug > 1)
> {
> fprintf_unfiltered (gdb_stdlog,
> - "Record: record_reg to inferior num = %d.\n",
> + "Record: record_reg 0x%s to inferior num = %d.\n",
> + paddr_nz ((CORE_ADDR)record_list),
> record_list->u.reg.num);
> }
> regcache_cooked_read (regcache, record_list->u.reg.num, reg);
> @@ -594,7 +594,8 @@ record_wait (ptid_t ptid, struct target_
> if (record_debug > 1)
> {
> fprintf_unfiltered (gdb_stdlog,
> - "Record: record_mem to inferior addr = 0x%s len = %d.\n",
> + "Record: record_mem 0x%s to inferior addr = 0x%s len = %d.\n",
> + paddr_nz ((CORE_ADDR)record_list),
> paddr_nz (record_list->u.mem.addr),
> record_list->u.mem.len);
> }
> @@ -625,19 +626,13 @@ record_wait (ptid_t ptid, struct target_
> if (record_debug > 1)
> {
> fprintf_unfiltered (gdb_stdlog,
> - "Record: record_end to inferior need_dasm = %d.\n",
> + "Record: record_end 0x%s to inferior need_dasm = %d.\n",
> + paddr_nz ((CORE_ADDR)record_list),
> record_list->u.need_dasm);
> }
>
> if (record_execdir == EXEC_FORWARD)
> {
> - if (record_list == &record_first)
> - {
> - /* The first record_t, not a really record_t.
> - Goto next record_t. */
> - goto next;
> - }
> -
> need_dasm = record_list->u.need_dasm;
> }
> if (need_dasm)
> @@ -706,21 +701,19 @@ record_wait (ptid_t ptid, struct target_
> }
>
> next:
> - if (record_execdir == EXEC_REVERSE)
> - {
> - if (record_list->prev && continue_flag)
> - record_list = record_list->prev;
> - else
> - record_list_status = 1;
> - }
> - else
> + if (continue_flag)
> {
> - if (record_list->next)
> - record_list = record_list->next;
> + if (record_execdir == EXEC_REVERSE)
> + {
> + if (record_list->prev)
> + record_list = record_list->prev;
> + }
> else
> - record_list_status = 2;
> + {
> + if (record_list->next)
> + record_list = record_list->next;
> + }
> }
> -
> }
> while (continue_flag);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [reverse RFC] Fix some bugs
2008-09-18 18:43 ` Michael Snyder
@ 2008-09-19 3:37 ` teawater
0 siblings, 0 replies; 3+ messages in thread
From: teawater @ 2008-09-19 3:37 UTC (permalink / raw)
To: Michael Snyder; +Cc: gdb-patches
On Fri, Sep 19, 2008 at 02:40, Michael Snyder <msnyder@vmware.com> wrote:
> teawater wrote:
>>
>> Thanks for the test and bug report of Michael. It help me a lot.
>>
>> 2008-09-19 Hui Zhu <teawater@gmail.com>
>>
>> * record.c (record_wait): Fix the bug of forware step after
>> reverse step.
>> Make debug message more clean.
>> Remove variable record_list_status.
>>
>> * i386-tdep.c (i386_record): Fix the bug of "0x80 ... 0x83".
>
> Hui, are all these changes related?
> It looks a little like you might have combined several
> smaller changes into one patch. In order to make review
> easier, we like to have unrelated patches submitted separately.
>
> If they are in fact related, could you explain please?
> Maybe add a few more comments?
Sorry for it. I will add some comments in this function.
Thanks,
Hui
>
> Thanks a lot!
>
>
>> ------------------------------------------------------------------------
>>
>> --- a/ChangeLog
>> +++ b/ChangeLog
>> @@ -1,3 +1,12 @@
>> +2008-09-19 Hui Zhu <teawater@gmail.com>
>> +
>> + * record.c (record_wait): Fix the bug of forware step after
>> + reverse step.
>> + Make debug message more clean.
>> + Remove variable record_list_status.
>> +
>> + * i386-tdep.c (i386_record): Fix the bug of "0x80 ... 0x83".
>> +
>> 2008-09-15 Michael Snyder <msnyder@vmware.com>
>> * infrun.c (proceed): No need to singlestep over a breakpoint
>> --- a/i386-tdep.c
>> +++ b/i386-tdep.c
>> @@ -3037,11 +3037,7 @@ reswitch:
>> }
>> else
>> {
>> - if (ot == OT_BYTE)
>> - {
>> - reg &= 0x3;
>> - }
>> - if (record_arch_list_add_reg (reg))
>> + if (record_arch_list_add_reg (rm))
>> {
>> return (-1);
>> }
>> --- a/record.c
>> +++ b/record.c
>> @@ -33,7 +33,6 @@ int record_debug = 0;
>> record_t record_first;
>> record_t *record_list = &record_first;
>> -int record_list_status = 1; /* 0 normal 1 to the begin 2 to the end */
>> record_t *record_arch_list_head = NULL;
>> record_t *record_arch_list_tail = NULL;
>> struct regcache *record_regcache = NULL;
>> @@ -446,7 +445,6 @@ record_open (char *name, int from_tty)
>> /* Reset */
>> record_insn_num = 0;
>> - record_list_status = 1;
>> record_execdir = EXEC_FORWARD;
>> record_list = &record_first;
>> record_list->next = NULL;
>> @@ -546,30 +544,31 @@ record_wait (ptid_t ptid, struct target_
>> Then set it to terminal_ours to make GDB get the signal. */
>> target_terminal_ours ();
>> - /* Loop over the record log, looking for the next place to stop.
>> */
>> + /* In EXEC_FORWARD mode,, record_list point to the tail of prev
>> + instruction. */
>> + if (record_execdir == EXEC_FORWARD && record_list->next)
>> + {
>> + record_list = record_list->next;
>> + }
>> +
>> + /* Loop over the record_list, looking for the next place to stop.
>> */
>> do
>> {
>> /* check state */
>> - if ((record_execdir == EXEC_REVERSE && !record_list->prev
>> - && record_list_status == 1) - ||
>> (record_execdir != EXEC_REVERSE
>> - && !record_list->next
>> - && record_list_status == 2))
>> + if (record_execdir == EXEC_REVERSE && record_list ==
>> &record_first)
>> {
>> - if (record_list_status == 2)
>> - {
>> - fprintf_unfiltered (gdb_stdlog,
>> - "Record: running to the end of
>> record list.\n");
>> - }
>> - else if (record_list_status == 1)
>> - {
>> - fprintf_unfiltered (gdb_stdlog,
>> - "Record: running to the begin of
>> record list.\n");
>> - }
>> + fprintf_unfiltered (gdb_stdlog,
>> + "Record: running to the begin of record
>> list.\n");
>> + stop_soon = STOP_QUIETLY;
>> + break;
>> + }
>> + if (record_execdir != EXEC_REVERSE && !record_list->next)
>> + {
>> + fprintf_unfiltered (gdb_stdlog,
>> + "Record: running to the end of record
>> list.\n");
>> stop_soon = STOP_QUIETLY;
>> break;
>> }
>> - record_list_status = 0;
>> /* set ptid, register and memory according to record_list */
>> if (record_list->type == record_reg)
>> @@ -579,7 +578,8 @@ record_wait (ptid_t ptid, struct target_
>> if (record_debug > 1)
>> {
>> fprintf_unfiltered (gdb_stdlog,
>> - "Record: record_reg to inferior num
>> = %d.\n",
>> + "Record: record_reg 0x%s to inferior
>> num = %d.\n",
>> + paddr_nz ((CORE_ADDR)record_list),
>> record_list->u.reg.num);
>> }
>> regcache_cooked_read (regcache, record_list->u.reg.num, reg);
>> @@ -594,7 +594,8 @@ record_wait (ptid_t ptid, struct target_
>> if (record_debug > 1)
>> {
>> fprintf_unfiltered (gdb_stdlog,
>> - "Record: record_mem to inferior addr
>> = 0x%s len = %d.\n",
>> + "Record: record_mem 0x%s to inferior
>> addr = 0x%s len = %d.\n",
>> + paddr_nz ((CORE_ADDR)record_list),
>> paddr_nz (record_list->u.mem.addr),
>> record_list->u.mem.len);
>> }
>> @@ -625,19 +626,13 @@ record_wait (ptid_t ptid, struct target_
>> if (record_debug > 1)
>> {
>> fprintf_unfiltered (gdb_stdlog,
>> - "Record: record_end to inferior
>> need_dasm = %d.\n",
>> + "Record: record_end 0x%s to inferior
>> need_dasm = %d.\n",
>> + paddr_nz ((CORE_ADDR)record_list),
>> record_list->u.need_dasm);
>> }
>> if (record_execdir == EXEC_FORWARD)
>> {
>> - if (record_list == &record_first)
>> - {
>> - /* The first record_t, not a really record_t.
>> - Goto next record_t. */
>> - goto next;
>> - }
>> -
>> need_dasm = record_list->u.need_dasm;
>> }
>> if (need_dasm)
>> @@ -706,21 +701,19 @@ record_wait (ptid_t ptid, struct target_
>> }
>> next:
>> - if (record_execdir == EXEC_REVERSE)
>> - {
>> - if (record_list->prev && continue_flag)
>> - record_list = record_list->prev;
>> - else
>> - record_list_status = 1;
>> - }
>> - else
>> + if (continue_flag)
>> {
>> - if (record_list->next)
>> - record_list = record_list->next;
>> + if (record_execdir == EXEC_REVERSE)
>> + {
>> + if (record_list->prev)
>> + record_list = record_list->prev;
>> + }
>> else
>> - record_list_status = 2;
>> + {
>> + if (record_list->next)
>> + record_list = record_list->next;
>> + }
>> }
>> -
>> }
>> while (continue_flag);
>>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-09-19 3:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-09-18 17:17 [reverse RFC] Fix some bugs teawater
2008-09-18 18:43 ` Michael Snyder
2008-09-19 3:37 ` teawater
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox