Monday, June 17, 2019

Dexter Sony LDP-1450 bug finally found and fixed!

Some of you know that I had to roll back a Dexter firmware upgrade a while ago due to that game Crime Patrol not working correctly with the update.

Well, I've been working behind the scenes to figure out what the bug was and pulling my hair out in frustration looking at my code.

It turns out that the bug had nothing to do with my changes that caused the problem but instead was a buffer overflow bug in some other code that hadn't been a problem by sheer luck :)

Once I added some new serial port code, this buffer overflow was overwriting my memory.  I finally had to attach an AVR Dragon (AVR programmer and debugger) and set a memory breakpoint to catch the problem.

Here is the code that caused the problem.  It's part of the LDP-1000/1450 interpreter:

uint16_t g_ldp1000i_tx_buf[12]; // this should be small enough so as to not waste space but big enough to hold things like current frame number
uint16_t *g_ldp1000i_pTxBufStart = 0;
uint16_t *g_ldp1000i_pTxBufEnd = 0;
const uint16_t *g_ldp1000i_pTxBufLastGoodPtr = (g_ldp1000i_tx_buf + sizeof(g_ldp1000i_tx_buf) - 1);

The fix is to change the last line to:

const uint16_t *g_ldp1000i_pTxBufLastGoodPtr = (g_ldp1000i_tx_buf + (sizeof(g_ldp1000i_tx_buf)/sizeof(uint16_t)) - 1);

The problem with working in C is that these types of mistakes are easy to make and the compiler often won't even warn you about it.  This is the price of working with lower level languages :o

Using the sizeof keyword is kind of dangerous because it always returns a value in bytes which doesn't work when your pointer is a uint16_t :)  I still use sizeof all over the place, though, because most of my pointers are uint8_t's :)

For future reference, when adding to a pointer, if you are going to use sizeof, you need to divide by the size (in bytes) of the element that makes up the array.  In this case, the TX buffer is 12 2-byte elements, so sizeof would return a 24 (12*2 is 24).  But when adding, I actually wanted to just add 12, not 24, because I wanted to add the number of elements, not the number of bytes.  So I needed to divide by the number of bytes per element (2 in this case).

After some testing, I expect to finally release some new firmware for Dexter in preparation for officially supporting the European version of Dragon's Lair (woohoo!).