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

Jiang Jilin
2009-10-13T02:17:51Z

[-- 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,