Marshal.Copy/Write is no longer atomic?

Ages ago I did a science project where I was reading and writing registers from managed code.  It worked well and there is absolutely no reason that you shouldn’t be able to do this kind of thing.  Windows CE is an embedded platform, and affecting hardware is what we do in the embedded world.


Well, as with any code posted on the web (and in fact this got rolled into the Smart Device Framework), it got used in some actual shipping products.  Great!  i’m not the only one who thinks we should be able to do this stuff.


Well, over the past 6 months or so I had a few different people contact me saying that the code either didn’t work, or did work on their earlier hardware, but was now giving strange behavior.


Most telling was that if you hooked up a scope to the memory, you would see 4 write pulses when writing to a register.  Furthermore, all 4 bytes in the register ended up getting set to the last byte in the array you wanted to write.  For example, if you wrote 0x12345678 to a register, it would actually get set to 0x56565656.


Now I know that this code originally worked.  I wrote it using actual hardware (PXA255) so the behavior was new.  The fact that there were 4 strobes on the memory strongly suggested that the code was actually doing 4 individual writes for the 4 bytes, not a single, atomic 4-byte write.


I looked at the code and it couldn’t be more simple.  A write boiled down to this:


public void WriteInt32(int data)
{
   Marshal.WriteInt32(m_addressPointer, data);
}


A bit more investigation found that the behavior was fine under CF 1.0, but started failing in CF 2.0.  What that means is that Microsoft changed the underlying implementation of the Marshal class, and in a bad way.  Why would they do such a stupid, stupid thing?  The new mechanism is going to not only cause a break for the writes we’re looking at, but it’s also a lot slower.


Since I don’t know who did this, I can only guess as to what happened.  If you write a 4-byte value to an address that is not DWORD aligned (i.e. not evenly divisible by 4) then an ARM processor with throw a fault and puke on you (x86 throws, but handles it internally).  My bet is that some edge case got reported that the CF was throwing an unaligned exception, and some idiot developer decided that the heavy-handed solution of changing the write to happen byte-by-byte would be the solution.  Yes, it prevents the error, but it causes bugs and is bad, bad form.  Personally I’d like to slap the persone who made the change and the person who reviewed the change and thought it was ok.


So how do you get around this?  Well by doing what the CF itself should have done.  Instead of using Marshal, you use unsafe code and pointers, and check address alignnment before writing.  Something like this:


public unsafe void WriteInt32(int data, int offset)
{
  int baseAddr = (m_addressPointer.ToInt32() + offset);
  if (baseAddr % 4 == 0)
  {
    // dword aligned
    uint* pDest = (uint*)(baseAddr);
    *pDest = (uint)data;
  }
  else if (baseAddr % 2 == 0)
  {
    // word aligned
    ushort* pDest = (ushort*)(baseAddr);
    *pDest = (ushort)(data >> 0x10);
    pDest += 2;
    *pDest = (ushort)(data & 0xFFFF);
  }
  else
  {
    // byte aligned
    byte* pDest = (byte*)(baseAddr);
    foreach (byte b in BitConverter.GetBytes(data))
    {
      *pDest = b;
      pDest++;
    }
  }
}


This is a classic case of someone not understanding the problem they are solving.  This logic should have been done well below us – that’s the whole point of using managed code, right?  To simplify things.  Unfortunately it also allows many developers to write code without understanding what it’s really doing, and in my mind that’s crazy risky. 


Testing also shows that the Read and Copy methods are similarly broken and all of these bugs still exist in CF 3.5, and I strongly suspect it will remain broken in future versions.  So beware, if you’re using Marshal for moving data, you could probably get 4x performance improvement by using a pointer instead.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s