This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Jiang Jilin wrote:
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.

I understand your first point (about caller vs. callee), but I don't understand what you mean about "handling" set_record_insn_max_num. In what way does it need handling?

There is still a user command to set and show that variable:


add_setshow_zinteger_cmd ("insn-number-max", no_class, &record_insn_max_num, _("Set record/replay buffer limit."), _("Show record/replay buffer limit."), _("\ Set the maximum number of instructions to be stored in the\n\ record/replay buffer. Zero means unlimited. Default is 200000."),



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,


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]