FYI: Potential Memory Clobbering bug in 5.087 (and 5.085)

jeremiah · Joined: 20 Jul 2010 Posts: 1354

Just wanted to make folks aware since this was a pretty nasty surprise. I don't know the scope of the bug or the exact specific cause of it (I haven't had time to go through iterations of different option combinations, but the following code does something really *fun*. It's nonsense code, but reproduces the problem. The same issue occurs in correct slave SPI code running on this chip.

Ttelmah · Joined: 11 Mar 2010 Posts: 19538

I'd suspect that attempting to write to the SPI peripheral on a slave is the
problem. A slave can only physically 'write' data when it receives a
transfer. Does it do the same if you use spi_prewrite, which is the command
to pre load the data for the master to read?.
I'd agree on the 16bit mode 'core' to the problem. It looks as if the compiler
is only assigning an 8bit temporary register. What happens if you specify
bits=16 in the #USE as well as the XFER16?.
Specifying 'SPIx', always automatically does a 'FORCE_HW', as does
specifying 'slave' so the setup is rather OTT!. However that shouldn't matter.
Does it do the same on SPI1 or 2?.
Not at a computer with CCS today, so will have to leave having a look at
this till I get back!... :0

jeremiah · Joined: 20 Jul 2010 Posts: 1354

I'd have go get the code base from my coworker to see his actual code and try out other options This was just a contrived example that I could plop over to CCS in an email. Basically just keeping it minimal since it is a code generation bug.

Ttelmah · Joined: 11 Mar 2010 Posts: 19538

Just tested, and adding BITS=16 to the #USE SPI, fixes the problem.
It moves "dont_clobber_me" up to address 802.
It's interesting, since the default is meant to be 32bit max transfers.
Explicitly selecting BITS=32, has it allocating 4 bytes for the transfer.
So somehow the 'default' size is not being applied, resulting in only a
single byte buffer being allocated.
Went back to 5.074, and by default it uses a CLR.B, so doesn't clobber the
next byte. 5.080 though has the fault. In all cases though you have to
explicitly use BITS= to get more than one byte of storage allocated.

jeremiah · Joined: 20 Jul 2010 Posts: 1354

I checked back to 5.078 and it used CLR 800, so it broke somewhere between 5.074 and 5.078.

I checked my coworker's code. They are using spi_xfer even though it is in slave mode. Aside from the clobbering bug, it does correct enough SPI for them to talk to another device without any issues. The original fix was to #locate a ram variable right after the one CCS created to absorb the 16bit access. I'll update their bug report with the BITS=16 option, but since they aren't using the SPI in the traditional slave way, they'll probably want to do some bench testing to make sure it doesn't break something.