If you’re C programmer, does this code look OK to you?
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int main(int argc, char* argv[])
{
char szBuffer[80];
strcpy(szBuffer, "abcdefghijklmnopqrstuvwxyz");
printf("Before: %s\n", szBuffer);
strcpy(szBuffer, szBuffer+2);
printf(" After: **%s\n", szBuffer);
return 0;
}
Here is the output on my server, a Core i7 running Debian 6:
Before: abcdefghijklmnopqrstuvwxyz
After: **cdefghijklmnopqrstuvwzyz
What the program does is dropping two characters from a text string in a buffer, moving the rest of it left by two characters. You expect the moved characters to stay in sequence, but if you compare the last three characters of the output you see that that isn’t the case. The ‘x’ has been obliterated by a duplicate ‘z’. The code is broken.
It’s a bug, and not a straightforward one, as I’ll explain.
I first came across it a couple of months ago, as I was moving some code of mine from an Athlon 64 Linux server to a new Intel Core i7 server. Subsequently I observed strange corruption in data it produced. I tracked it down to strcpy() calls that looked perfectly innocent to me, but when I recoded them as in-line loops doing the same job the bug went away.
Yesterday I came across the same problem on a CentOS 6 server (also a Core i7, x86_64) and figured out what the problem really was.
Most C programmers are aware that overlapping block moves using strcpy or memcpy can cause problems, but assume they’re OK as long as the destination lies outside (e.g. below) the source block. If you read the small print in the strcpy documentation, it warns that results for overlapping moves are unpredicable, but most of us don’t take that at face value and think we’ll get away with it as long as we observe the above caveat.
That is no longer the case with the current version of the GNU C compiler on 64-bit Linux and the latest CPUs. The current strcpy implementation uses super-fast SSE block operation that only reliably work as expected if the source and destination don’t overlap at all. Depending on alignment and block length they may still work in some cases, but you can’t rely on it any more. The same caveat theoretically applies to memcpy (which is subject to the same warnings and technically very similar), though I haven’t observed the problem with it yet.
If you do need to remove characters from the middle of a NUL terminated char array, instead of strcpy use your own function based on the memmove and strlen library functions, for example something like this:
void myStrCpy(char* d, const char* s)
{
memmove(d, s, strlen(s)+1);
}
...
char szBuffer[80];
...
// remove n characters i characters into the buffer:
myStrCpy(szBuffer+i, szBuffer+i+n);
I don’t know how much existing code the “optimzed” strcpy library function broke in the name of performance, but I imagine there are many programmers out there that got caught by it like I was.
See also: