Re: Visual C++ aliasing issue
On Tue, 21 Apr 2009 12:01:06 -0700 (PDT), "{}" <satol@yahoo.com> wrote:
Hello,
Here is an example, where Visual Studio 2008 produces a code, that
does not work right when compiled with the default Release
configuration. The code works correctly in Debug.
Looks like SP1 has the same problem.
The code was working in VS2003.
The program copies an array of BGR elements into an array of BGRA
elements in place backwards (starting from the last element).
The variable tmp in the loop is introduced specially to prevent the
aliasing issue. The compiler optimizes the variable out however.
#include <malloc.h>
Should be <stdlib.h>.
struct Pixel3 {unsigned char b, g, r;};
struct Pixel4 {unsigned char b, g, r, a;};
unsigned char* volatile pBufTmp = 0; //volatile to prevent unnecessary
optimizations here
volatile int uiWidth = 3;
Get rid of both volatiles.
int main(int argc, char* argv[])
{
pBufTmp = (unsigned char*)malloc(4 * uiWidth);
Read up on the sizeof operator. (You're implicitly relying on sizeof below
every time you cast a pointer and index it as an array.)
for (int k = 0; k < uiWidth; k++)
{
((Pixel3*)pBufTmp)[k].b = 0x30;
((Pixel3*)pBufTmp)[k].g = 0x20;
((Pixel3*)pBufTmp)[k].r = 0x10;
}
int iSrcBytesPP = 3;
int iDstBytesPP = 4;
Read up on the sizeof operator.
unsigned char* pBufSrc = pBufTmp + (iSrcBytesPP * (uiWidth - 1));
unsigned char* pBufDst = pBufTmp + (iDstBytesPP * (uiWidth - 1));
for (int k = 0, cc = uiWidth; k < cc; k++, pBufSrc -= iSrcBytesPP,
pBufDst -= iDstBytesPP)
{
Pixel3 tmp = *(Pixel3*)pBufSrc; //copy to temp variable
*(Pixel3*)pBufDst = tmp;
((Pixel4*)pBufDst)->a = 255;
}
return 0;
}
Expected that the pBufTmp should contain elements 0x30, 0x20, 0x10,
0xFF, instead only the first element is the correct one, the rest is
set to 0x30, 0x20, 0x20, 0xFF.
If you look at the assembly code (compile with /FAs), you'll see the
compiler has optimized the struct assignment in the last loop into a
word/byte move:
mov si, WORD PTR [edx]
mov WORD PTR [ecx], si
mov bl, BYTE PTR [edx+2]
mov BYTE PTR [ecx+2], bl
Unfortunately, it's doing this from low address to high address, which is
destructive to your overlapping arrays. This is definitely a bug, since
your use of the temporary variable should eliminate this concern. You could
fix it by using memmove or by copying member by member in reverse order.
You can report the bug here:
http://connect.microsoft.com/feedback/default.aspx?SiteID=210
That said, your code is very hard to understand and would benefit from a
rewrite! You should be using sizeof, and almost all of those casts should
be unnecessary in well-written code.
--
Doug Harrison
Visual C++ MVP