RFC: Run length encoding bug in remote.c?

[Orjan Friberg <orjan dot friberg at axis dot com>]

I've had problems for a long time printing the task_struct pointer
"current" in our Linux kernel port.  The gdb stub in the kernel does
run-length encoding of the data it sends, which is used heavily when
printing the current struct since it contains lots of zeros.

After receiving the first g packet from the stub, host-gdb asks for
memory chunks of that size.  For my target, REGISTER_SIZE is 106 bytes,
which is 0x6a.  With 2 hex chars per byte that amounts to 212 bytes.

If I turn off run-length encoding in the stub, the first read of the
current struct results in the following communication:

Sending packet: $m600f6060,6a#f8...Ack
Packet received:
000000000f00000000000000000000000000000000000000000000000000000032000000000000000000000000000000706d0e60000000000000000000000000000000000040176000c0166000801660004016600000166000c01f6000001f6000000000000000000000

The packet received is 212 bytes like it should be.

Turning on run-length encoding the following happens:

Sending packet: $m600f6060,6a#f8...Ack
Repeat count 20 too large for buffer:
000000000f000000000000000000000000000000000000000000000000000000370000000000000000000000000000008c6d0e60000000000000000000000000000000000040176000c0166000801660004016600000166000c01f6000001f60

What is missing is the last 20 zeros from the previous example.  So, in
effect, what gdb claims doesn't fit in the buffer fits perfectly in
fact.

To me there seems to be a off-by-one error in the following snippet of
code (read_frame() in remote.c):

/* The character before ``*'' is repeated. */

if (repeat > 0 && repeat <= 255 
    && bc > 0
    && bc + repeat < sizeof_buf - 1)
  {
    memset (&buf[bc], buf[bc - 1], repeat);
    bc += repeat;
    continue;
  }

I suspect that the (bc + repeat) in the condition should in fact be (bc
+ repeat - 1) since the memset begins already at position bc in buf,
which means that the last position in buf to be memsetted is at position
(bc + repeat - 1).  *That* position must be < sizeof_buf - 1 to allow
room for null-termination of buf.

The patch below makes that change.  Though this works (tested with
"print *current" in gdb) and looks sensible to me, I have a feeling that
this error should show up quite often for other people as well, since it
would happen every time the response to an 'm' packet ends with
run-length encoding of the data.

Comments?


2001-08-08  Orjan Friberg  <orjanf@axis.com>

        * remote.c (read_frame): Correct off-by-one error in condition.


Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.58
diff -c -3 -p -r1.58 remote.c
*** remote.c    2001/07/15 20:34:14     1.58
--- remote.c    2001/08/08 13:26:00
*************** read_frame (char *buf,
*** 4189,4195 ****
  
            if (repeat > 0 && repeat <= 255 
                && bc > 0
!                 && bc + repeat < sizeof_buf - 1)
              {
                memset (&buf[bc], buf[bc - 1], repeat);
                bc += repeat;
--- 4189,4195 ----
  
            if (repeat > 0 && repeat <= 255 
                && bc > 0
!                 && bc + repeat - 1 < sizeof_buf - 1)
              {
                memset (&buf[bc], buf[bc - 1], repeat);
                bc += repeat;

-- 
Orjan Friberg              E-mail: orjan.friberg@axis.com
Axis Communications AB     Phone:  +46 46 272 17 68