#4 gdb_write_memory occasionally fails to write whole buffer

closed-fixed
nobody
None
5
2000-09-10
2000-09-03
Edwin Olson
No

I found a nasty little bug in gdb_write_memory.

The bug occurs on oddly aligned and sized buffers like the following packet (actually occurred on my system, 2.95.2 based):

$M100512f,53:552329213051e3621372011e23afe3000961f3e403d809480b0009e000a00000097e106fe34f266ef668f6000b0009040003ff000901005270010052a0010051702f862f962fe64f227ffc6ef32e4261e2e201#64

Obviously 0x100512f isn't aligned on a word boundary, and the length of 83 (0x53) isn't either. In the gdbstubs, this situation results in only the first byte (0x55) being written to the memory and gdb_write_memory returns (reporting success.)

I have modified the function as below. I have made some other changes too which I think make more sense... In particular, I only worry about word/short transactions when the length field is a single word or short. In all other cases, I just blast each byte individually.

I have tested this on my system and it seems to work for uploading code, though I have not tested peripheral accesses yet.

void gdb_write_memory ( const char *hargs )
{
long addr = 0;
long len = 0;

/* parse address */
while( *hargs != ',' )
addr = ( addr << 4 ) + hex_to_long( *hargs++ );

/* skip ',' */
hargs++;

/* parse length */
while( *hargs != ':' )
len = ( len << 4 ) + hex_to_long( *hargs++ );

/* skip ':' */
hargs++;

/* if it is a single long word, make sure we write it in one transaction--
it might be a peripheral. */
if ( (addr%sizeof(long))==0 && len==sizeof(long) )
{
*(long*)addr = hexbuf_to_long( sizeof (long) * 2, hargs);
hargs += sizeof(long)*2;
addr += sizeof(long);
len -= sizeof(long);
}

/* the same applies to shorts. */
if ( (addr%sizeof(short))==0 && len==sizeof(short) )
{
*(short*)addr = hexbuf_to_long( sizeof (long) * 2, hargs);
hargs += sizeof(short)*2;
addr += sizeof(short);
len -= sizeof(short);
}

/* This is either a very lengthy or oddly sized region. Write it one byte at a time. */
while (len>0)
{
*(char*)addr=hexbuf_to_long(sizeof(char)*2,hargs);
hargs += sizeof(char)*2;
addr += sizeof(char);
len -= sizeof(char);
}

// gdb_putmsg( 0, "OK", 2 );
return;
}

Discussion

  • Edwin Olson

    Edwin Olson - 2000-09-03

    I see that bgat had previously reported a similar bug, but the code in CVS still exhibits the problem I indicated.

    I have not closely examined gdb_read_memory to see if changes are warranted there-- this might be a good thing to check.

     
  • Edwin Olson

    Edwin Olson - 2000-09-04

    After some additional thought, I have rewritten it again. This time it should "always do the right thing". I've tested this version a bit better (there was a typo in the original submission anyway.)

    Only the bottom part of the function has changed... I'd create a patch, but I don't seem to have permission to edit the repository.

    /* skip ':' */
    hargs++;

    /* keep writing bytes until we've written them all. Prefer larger writes
    over smaller writes since some peripherals with large registers need
    to be written in a single transaction. */

    while (len>0)
    {
    /* can we write an aligned long? */
    if (addr%sizeof(long)==0 && len>=sizeof(long))
    {
    *(long*)addr = hexbuf_to_long( sizeof (long) * 2, hargs);
    hargs += sizeof(long)*2; /* two characters consumed for each byte */
    addr += sizeof(long);
    len -= sizeof(long);
    continue;
    }

    /* we couldn't write an aligned long. Can we write an aligned short? */
    if ( (addr%sizeof(short))==0 && len>=sizeof(short) )
    {
    *(short*)addr = hexbuf_to_long( sizeof (short) * 2, hargs);
    hargs += sizeof(short)*2;
    addr += sizeof(short);
    len -= sizeof(short);
    continue;
    }

    /* all else has failed. Write a single byte. */
    *(char*)addr=hexbuf_to_long(sizeof(char)*2,hargs);
    hargs += sizeof(char)*2;
    addr += sizeof(char);
    len -= sizeof(char);

    }

    gdb_putmsg( 0, "OK", 2 );
    return;
    }

     
  • Edwin Olson

    Edwin Olson - 2000-09-10
    • status: open --> closed-fixed
     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

JavaScript is required for this form.





No, thanks