Programming tips

So I finally fixed a bug which has been causing some people a bit of grief, bug 160284. This bug involves crashes due to a couple of different things:

  1. A memory address was casted to a larger sized type, which was not aligned properly for that size.
  2. In addition, there were issues handling memory which was mapped from a file using mmap(2).

What’s the deal with alignment you ask? It deals with computer architecture. Pretty much every major computer architecture I can think of requires memory accesses of a datatype of a certain size to be on a memory address that is a multiple of the datatype’s size. For instance, if you were trying to read a 32-bit integer, the CPU would expect a memory address to be divisible by 4, since the integer takes up 4 bytes in memory.

Note that this is a glossing-over. Just because a datatype is 4 bytes long doesn’t mean that as long as your memory address is divisible by 4 that it’s good. An architecture could require all memory access to happen on 8 or 16 byte boundaries for instance. In addition, the predominant architecture, x86, for the most part does not have these restrictions. Certain instructions do require aligned memory however, and unaligned memory access is slower even when it’s allowed.

The reason you never hear about this is that the compiler knows about the alignment restrictions for the architecture it’s building for, and takes care of it for you. Unless you try and go around it…

In our case, we had code to read in a file by mmap-ing it. The first 17 or so bytes were read to ensure that the file we were reading in is correct. Then the very next 4 bytes were cast to a 32-bit integer for a version check. This access is unaligned, but works for most systems since x86 does not crash, but instead is merely slower.

The solution is to add padding between the 17-byte magic string and the version integer. There are two obvious ways of doing that:

  1. Do it yourself. Calculate the next memory address that the CPU expects an integer access to fall on and insert padding until you get there. Unfortunately there is no easy way to determine alignment restrictions at build or run time that I can see.
  2. Let the compiler do it for you. What you need to do is make your header data a struct or class. By default each member will be correctly aligned, and better yet if you simply read and write a header at a time you won’t need to do tricky pointer arithmetic. The hard part is ensuring that the struct / class contains the character data and not merely a character pointer.

What I used in my fix was something like this:

static const char KPC_MAGIC[] = "KDE PIXMAP CACHE DEUX";
struct KPixmapCacheDataHeader
    KPixmapCacheDataHeader() :
        magic[0] = '\0'; // In case of inadvertent strcpy or something.

    // -1 from sizeof so we don't write out the trailing null.
    char    magic[sizeof(KPC_MAGIC) - 1];
    quint32 cacheVersion;
    quint32 size;

Other fun programming tips:

  • Dealing with endianness conversions? Don’t reimplement it yourself! Qt includes template functions to do it for you in QtGlobal.
  • Want to know the alignment of a datatype at compile time? Tom Lokovic has an interesting C++ solution.
  • Are you using QDataStream? Please make sure that unless your data types are the C++ basic types (i.e. basically the Qt integer types) that you are explicitly setting a binary version on your QDataStream and sticking with it. This is all described in QDataStream’s documentation. I’ve fallen afoul of this before which broke JuK in KDE 4.0 where you couldn’t load KDE 3.5 data, and I’ve seen a lot of non-trivial uses of QDataStream without setting the version. If you don’t set a version then Qt will pick one for you, and it’s likely to change without notice. It’s up to you to do it, Qt doesn’t/can’t encode QDataStream version info into the stream.
  • Although I wasn’t required to go this route, if you want to create a C++ object in shared memory (like a QMutex for shared mutual exclusion) then placement new might be for you. With placement new you handle allocating the memory, all new does is create the object where you tell it to. Of course, you’ll have to manually destruct the object too…
  • The corollary to the compiler handling alignment of structs is that if you make a struct intended to reflect a file header, you may have to tell the compiler to keep the struct “packed”. The flag to do so is implementation dependent however. Your program won’t crash if you do this as the compiler will insert code to fix up unaligned references as necessary. So if you were wondering why the sizeof a struct is different than what you expect, it could be due to alignment.

BTW, if you were annoyed at JuK’s track announcement popup going weird places, it’s fixed now.