Variable-Length Array

19 Mar 2015

Recently, I have discovered a hidden bug in a computer program, which caused the program to crash under certain circumstances. The code looks like the following:

void Tools::blobToString(
        std::string&        out_string,
        const unsigned char *in_blob,
        const int           in_size)
{
    char tmpBlob[in_size];

    for (int i = 0; i < in_size; i++)
    {
        tmpBlob[i] = (char)in_blob[i];
    }

    out_string.assign(tmpBlob, in_size);
}

The program crashed when tmpBlob is being written inside the for-loop. But why?

Maybe a bit more background information first: the function is used to copy an image blob into a C++ string which will be used later in the program; the size of the image is passed in as in_size parameter.

When I debugged the program, I found the image that caused the crash has in_size around 70MB and in_blob was filled and not empty. I suspected the large image size could be the root cause but I wasn’t sure. Then I found the memory for tmpBlob was not allocated at the moment the program crashed, since the memory of that array was not accessible in the debugger. So … ah-ha!!!!! There it is!

tmpBlob is a statically allocated variable-length character array, which means it is usually allocated on statck. If the array length is too long (or the available remaining space on stack is not enough), which was my case, then stack overflow will occur and allocation will fail. And no, not the stackoverflow website.

But wait, how do you know for sure the array is bigger than your stack? how big is your stack? Try ulimit:

$ ulimit -a
core file size          (blocks, -c) 0
data seg size           (kbytes, -d) unlimited
scheduling priority             (-e) 0
file size               (blocks, -f) unlimited
pending signals                 (-i) 63086
max locked memory       (kbytes, -l) 64
max memory size         (kbytes, -m) unlimited
open files                      (-n) 1024
pipe size            (512 bytes, -p) 8
POSIX message queues     (bytes, -q) 819200
real-time priority              (-r) 0
stack size              (kbytes, -s) 8192
cpu time               (seconds, -t) unlimited
max user processes              (-u) 63086
virtual memory          (kbytes, -v) unlimited
file locks                      (-x) unlimited

Looks like normal stack size is only a few mega bytes. Therefore, 70MB is no doubt way to big. So, the array space was not created, but the program still tried to write data to a non-existing memory space, segfault occured, mystery solved.

This type of error is one of the ‘NO-NOs’ your teacher taught you to avoid at school, but yet it happens in real-life, production code (how embarrassed). The rule of thumb is, you shouldn’t allocate variable-length array on stack if the size cannot be determined at compile-time; in such cases, you should allocate memory (using malloc) on heap. Stack is better-suited for fixed-size data structures.

However, we can also imagine in a reasonable-sized software, the need for dynamic memory allocation is quite large. And if we keep using malloc all over the place, the code will look like spaghetti and causes memory fragmentation, which might drag down your software performance; in other words, this is not an efficient way to manage your memory EITHER. The more structural solution is to implement a memory pool.

ADDENDUM

My colleauge provided an alternative solution to the code above, which prevents copying twice (and creating allocation issues):

void Tools::blobToString(
        std::string&        out_string,
        const unsigned char *in_blob,
        const unsigned int  in_size)
{
    out_string.assign(reinterpret_cast<const std::string::value_type*>(in_blob), in_size);
}

Much better and simpler isn’t it? But why C++ has to make things so complicated in the first place so that the first person who wrote the original code didn’t figure out?

Also, the rule of thumb is to never use a C array in C++. Use vector instead if you know the size in advance (which is the case in the example shown above - a known fixed length at run time), and vector is always allocated on stack which also solves our problem mentioned in this post.