Re: [RFA] Fix off-by-one error in record.c (record_list_release_first)

[Jiang Jilin <freephp@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2043 bytes --]

On Tue, Oct 13, 2009 at 12:35 AM, Michael Snyder <msnyder@vmware.com> wrote:
> Hui,
>
> My "info record" patch helped me to find a bug.
> I found that, once we start calling record_list_release_first,
> we start adding two instructions to the log for every one
> instruction we remove.  Therefore the log continues to grow,
> even though it is supposed to remain constant in size.
>
> This is because record_insn_num is not incremented if we
> call record_list_release_first -- but record_list_release_first
> does decrement it.

Hi Michael,

I've noticed this bug(maybe) earlier, but I prefer fixing the caller
functions, not the callee function record_list_release_first. And
your patch doesn't handle set_record_insn_max_num.

Thanks!

2009-10-13  Michael Snyder  <msnyder@vmware.com>
        Jiang Jilin  <freephp@gmail.com>

        * record.c (record_message): Increase record_insn_num after
        record_list_release_first.
        (record_registers_change): Ditto.
        (record_xfer_partial): Ditto.

diff --git a/gdb/record.c b/gdb/record.c
index 8b56010..0208dac 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -438,8 +438,8 @@ record_message (void *args)

   if (record_insn_num == record_insn_max_num && record_insn_max_num)
     record_list_release_first ();
-  else
-    record_insn_num++;
+
+  record_insn_num++;

   return 1;
 }
@@ -1008,8 +1008,8 @@ record_registers_change (struct regcache
*regcache, int regnum)

   if (record_insn_num == record_insn_max_num && record_insn_max_num)
     record_list_release_first ();
-  else
-    record_insn_num++;
+
+  record_insn_num++;
 }

 static void
@@ -1121,8 +1121,8 @@ record_xfer_partial (struct target_ops *ops,
enum target_object object,

       if (record_insn_num == record_insn_max_num && record_insn_max_num)
        record_list_release_first ();
-      else
-       record_insn_num++;
+
+      record_insn_num++;
     }

   return record_beneath_to_xfer_partial (record_beneath_to_xfer_partial_ops,

[-- Attachment #2: 0001-Fix-off-by-one-error-in-record.c-record_list_releas.patch.txt --]
[-- Type: text/plain, Size: 955 bytes --]

diff --git a/gdb/record.c b/gdb/record.c
index 8b56010..0208dac 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -438,8 +438,8 @@ record_message (void *args)
 
   if (record_insn_num == record_insn_max_num && record_insn_max_num)
     record_list_release_first ();
-  else
-    record_insn_num++;
+
+  record_insn_num++;
 
   return 1;
 }
@@ -1008,8 +1008,8 @@ record_registers_change (struct regcache *regcache, int regnum)
 
   if (record_insn_num == record_insn_max_num && record_insn_max_num)
     record_list_release_first ();
-  else
-    record_insn_num++;
+
+  record_insn_num++;
 }
 
 static void
@@ -1121,8 +1121,8 @@ record_xfer_partial (struct target_ops *ops, enum target_object object,
 
       if (record_insn_num == record_insn_max_num && record_insn_max_num)
 	record_list_release_first ();
-      else
-	record_insn_num++;
+
+      record_insn_num++;
     }
 
   return record_beneath_to_xfer_partial (record_beneath_to_xfer_partial_ops,