C++ enums historically gave me some headaches due to the fact that compilers are free to choose the size of the type for whatever criteria they think. This makes particularly hard to write portable code among compilers from ABI point of view (Visual C++ vs C++ Builder, anyone?).

Some days ago I was debugging a problem with dali-toolkit and the enum story scalated to another level.

Let me give you some background: dali-toolkit loads textures using a background worker thread that is responsible to load the image files to memory. I was looking why some textures didn’t load in Windows. To make the asynchronous loading possible it implements a small state machine and it keeps all information about the texture in a type called TextureInfo.

Here is a partial definition of the structure:

struct TextureInfo {
		// ....
    LoadState loadState:4;         ///< The load state showing the load progress of the Texture
    FittingMode::Type fittingMode:3; ///< The requested FittingMode
    Dali::SamplingMode::Type samplingMode:3; ///< The requested SamplingMode
    StorageType storageType:2;     ///< CPU storage / GPU upload;
		// ....
};

All these custom types are enums defined elsewhere. Yes, you read right, enums + bit fields, what possibly can go wrong? To illustrate the root cause of the bug, let’s write a toy sample:

enum Animal {
    Cat,
    Dog,
    Horse,
    Turtle,
};

enum Breed {
    BorderCollie,
    GoldenRetriever,
    GermanShepard,
    Husky,
};

enum Adopted {
    No,
    Yes,
};

struct AnimalInfo {
    Animal animal:2;
    Breed breed:2;
    Adopted adopted:1;

    AnimalInfo(Animal a, Breed b, Adopted ad)
        : animal(a), breed(b), adopted(ad) {}
};

int main(void) {
    AnimalInfo animal(Animal::Dog, Breed::GermanShepard, Adopted::Yes);

    auto breed = animal.breed;

    return sizeof(AnimalInfo);
}

Now, using godbolt, we can view the assembly code generated by different compilers. In particular, let’s look at the assembly code for the main function. Firstly, the the gcc generated code. I added comments to the parts interesting to us:

main:
        push    rbp
        mov     rbp, rsp
        sub     rsp, 16
        lea     rax, [rbp-8]
        mov     ecx, 1
        mov     edx, 2
        mov     esi, 1
        mov     rdi, rax
        call    AnimalInfo::AnimalInfo(Animal, Breed, Adopted)
        movzx   eax, BYTE PTR [rbp-8] // load the information
        shr     al, 2   	// breed is in the bits 2 and 3, so shift right 2 bits
        and     eax, 3  	// breed is only two bits long
        movzx   eax, al  	// eax = breed value
        mov     DWORD PTR [rbp-4], eax // store it in our variable
        mov     eax, 4    // AnimalInfo is 4 bytes long
        leave
        ret

Here is clang. It uses different registers but is essentially the same code:

main:
        push    rbp
        mov     rbp, rsp
        sub     rsp, 16
        mov     dword ptr [rbp - 4], 0
        lea     rdi, [rbp - 8]
        mov     eax, 1
        mov     esi, eax
        mov     edx, 2
        mov     ecx, eax
        call    AnimalInfo::AnimalInfo(Animal, Breed, Adopted)
        mov     r8b, byte ptr [rbp - 8]
        shr     r8b, 2
        and     r8b, 3
        movzx   eax, r8b
        mov     dword ptr [rbp - 12], eax
        mov     eax, 4
        add     rsp, 16
        pop     rbp
        ret

Now the msvc compiler:

main PROC
$LN3:
  sub rsp, 56 ; 00000038H
  mov r9d, 1
  mov r8d, 2
  mov edx, 1
  lea rcx, QWORD PTR animal$[rsp]
  call AnimalInfo::AnimalInfo(Animal,Breed,Adopted) ; AnimalInfo::AnimalInfo
  mov eax, DWORD PTR animal$[rsp]
  shl eax, 28
  sar eax, 30 // OH NO! SIGNED SHIFT!!!
  mov DWORD PTR breed$[rsp], eax
  mov eax, 4
  add rsp, 56 ; 00000038H
  ret 0
main ENDP

Before commenting about the bug generated by msvc, let me praise the compiler optimizer, it got the breed value in two instructions, while clang and gcc needed three. Very smart! Too bad that code is wrong. I think it is better to undestand the issue if we view the bit layout of the word holded by AnimalInfo:

	-------------------------------------
	|  31-5  |    4    |  3-2  |  1-0   |
	-------------------------------------
	| Unused | Adopted | Breed | Animal |
	-------------------------------------

So, the code shifts 28 bits to left, moving Breed to the bits 31 and 30, then it shifts 30 bits to the right, but it does a signed shift, filling the shifted bits with the value of the most significant bit. The Breed value is GermanShepard, which is 2. So, when we shift left, the bit 31 will hold the value 1, and when we shift right, we end up with the final value 0xfffffffe! The reason this happens is because msvc assumes the enum is a signed integer by default and, combined with the use of bit fields, yields the bug. This is precisely what happened in dali-toolkit.

Once we understood the problem, we can solve it easily by forcing the enum to an unsigned integer:

enum class Animal: unsigned int {
    Cat,
    Dog,
    Horse,
    Turtle,
};

enum class Breed: unsigned int {
    BorderCollie,
    GoldenRetriever,
    GermanShepard,
    Husky,
};

enum class Adopted: unsigned int {
    No,
    Yes,
};

struct AnimalInfo {
    Animal animal:2;
    Breed breed:2;
    Adopted adopted:1;

    AnimalInfo(Animal a, Breed b, Adopted ad)
        : animal(a), breed(b), adopted(ad) {}
};

int main(void) {
    AnimalInfo animal(Animal::Dog, Breed::GermanShepard, Adopted::Yes);

    auto breed = animal.breed;

    return sizeof(AnimalInfo);
}

Now the generated code does what it is supposed to do:

main PROC
$LN3:
  sub rsp, 56 ; 00000038H
  mov r9d, 1
  mov r8d, 2
  mov edx, 1
  lea rcx, QWORD PTR animal$[rsp]
  call AnimalInfo::AnimalInfo(Animal,Breed,Adopted) ; AnimalInfo::AnimalInfo
  mov eax, DWORD PTR animal$[rsp]
  shr eax, 2
  and eax, 3
  mov DWORD PTR breed$[rsp], eax
  mov eax, 4
  add rsp, 56 ; 00000038H
  ret 0
main ENDP