Struct packing, source listings and the Zen of preprocessed code.
It seems that no matter how good at people (definitely including me) are at writing buggy code they are much less adept at creating good reproduction cases or using their existing tools to figure out what’s going on.
A recent issue I saw involved a piece of C++ code where a struct was being sent over a socket and was arriving corrupted.
I was giving a laundry list of what they had tried. Debugging the send and receive through all the layers of the stack, network analysis to verify the packet came through each hop uncorrupted, debugging both sides of the connection extensively. What they concluded was that there was a compiler bug causing bogus code to be generated on one side of the connection.
Compiler do break sometimes. But this block of code was a series of integral assignments (int = int, char = char, etc). This was also a shipping version of VC – not some random developer desktop build. I was highly skeptical of their claim.
So I asked to see the struct. It looked something like this:
#ifdef PLATFORM_SPECIFIC_VALUE
#pragma pack (push, 2)
#else
#pragma pack (push, 4)
#endif
struct Foo
{
char Character;
int Integer;
short Short;
long Long;
};
#pragma pack(pop)
See the problem(s) yet?
So imagine a function that looks like this:
void SetFoo(Foo *foo)
{
foo->Character = 'a';
foo->Integer = 42;
foo->Short = 128;
foo->Long = 99999;
}
This is where I pulled out “compiler bug reproduction tool #1” (in my book anyway) – Generate Preprocessed File (/P).
People spend a lot of time assuming (i.e. guessing) what their code looks like once the maze of #ifdef and preprocessor goo are complete. But sometimes people make mistakes so to check you use /P.
They recompiled the source file SetFoo was defined in using /P and found this (some whitespace removed):
#line 1 "c:\\temp\\genrepro\\worker.cpp"
#line 1 "c:\\temp\\genrepro\\juice.h"
#pragma once
#pragma pack (push, 4)
#line 8 "c:\\temp\\genrepro\\juice.h"
struct Foo
{
char Character;
int Integer;
short Short;
long Long;
};
#pragma pack(pop)
#line 2 "c:\\temp\\genrepro\\worker.cpp"
void SetFoo(Foo *foo)
{
foo->Character = 'a';
foo->Integer = 42;
foo->Short = 128;
foo->Long = 99999;
}
So they are using 4 byte packing.
Why is this a problem? Well – let’s look at the generated asm to set those values in SetFoo:
; 5 : foo->Character = 'a';
00000 8b 44 24 04 mov eax, DWORD PTR _foo$[esp-4]
00004 c6 00 61 mov BYTE PTR [eax], 97 ; 00000061H
; 6 : foo->Integer = 42;
00007 c7 40 04 2a 00
00 00 mov DWORD PTR [eax+4], 42 ; 0000002aH
; 7 : foo->Short = 128;
0000e 66 c7 40 08 80
00 mov WORD PTR [eax+8], 128 ; 00000080H
; 8 : foo->Long = 99999;
00014 c7 40 0c 9f 86
01 00 mov DWORD PTR [eax+12], 99999 ; 0001869fH
Notice the offsets? 0, 4, 8, and 12? 4 byte packing.
Now let’s look at the other side setting those same values:
0001e 50 push eax
0001f c6 00 61 mov BYTE PTR [eax], 97 ; 00000061H
00022 c7 40 02 2a 00
00 00 mov DWORD PTR [eax+2], 42 ; 0000002aH
00029 66 c7 40 06 80
00 mov WORD PTR [eax+6], 128 ; 00000080H
0002f c7 40 08 9f 86
01 00 mov DWORD PTR [eax+8], 99999 ; 0001869fH
Offsets 0, 2, 6 and 8. 2 byte packing.
What do you think sizeof(Foo) returns on each platform? It’s 12 (for 2 byte packing) and 16 (4 byte packing).
(Hey – you too can generate your own asm listing using the Assembly Listing /Fx flags – I generated these using /FAcs).
Really there is nothing fancy about this whole exercise. A lot of you are probably reading this and saying “Well Duh!” but walk around the office and ask a handful of people how to generate an assembly listing of their code or how to see the preprocessed results for a source file and a surprising amount may not even know you could.
Once they had fixed the packing problem we began talking about their other problem.
Do you see it?