From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30501 invoked by alias); 19 Sep 2008 03:37:01 -0000 Received: (qmail 30489 invoked by uid 22791); 19 Sep 2008 03:36:59 -0000 X-Spam-Check-By: sourceware.org Received: from ti-out-0910.google.com (HELO ti-out-0910.google.com) (209.85.142.185) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 19 Sep 2008 03:36:25 +0000 Received: by ti-out-0910.google.com with SMTP id d10so110909tib.12 for ; Thu, 18 Sep 2008 20:36:22 -0700 (PDT) Received: by 10.110.103.5 with SMTP id a5mr6320592tic.2.1221795381943; Thu, 18 Sep 2008 20:36:21 -0700 (PDT) Received: by 10.110.42.9 with HTTP; Thu, 18 Sep 2008 20:36:21 -0700 (PDT) Message-ID: Date: Fri, 19 Sep 2008 03:37:00 -0000 From: teawater To: "Michael Snyder" Subject: Re: [reverse RFC] Fix some bugs Cc: "gdb-patches@sourceware.org" In-Reply-To: <48D2A09F.30203@vmware.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <48D2A09F.30203@vmware.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-09/txt/msg00405.txt.bz2 On Fri, Sep 19, 2008 at 02:40, Michael Snyder wrote: > teawater wrote: >> >> Thanks for the test and bug report of Michael. It help me a lot. >> >> 2008-09-19 Hui Zhu >> >> * 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 >> + >> + * 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 >> * 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); >> > >