A "Problem" Caused by GCC Optimization中文EN

This was originally posted as a long Weibo post. But since it's been a while since I updated the blog, I'm moving it over here — just to pad the count, so bear with me 🙂

I wasted nearly 2 hours on this and I have to vent:

It all started around 5pm today. I was debugging an extension, running a routine check with valgrind (valgrind-3.8.1), and unfortunately valgrind reported an invalid read:

After attaching with gdb, I found that the reported error location was:

In PHP NG (PHP New Generation), a new string structure is used to hold strings, namely zend_string:

After investigating for a long time, I confirmed that this op was properly initialized. So where was the problem?

Suddenly I noticed that op was a string of length 1, "0", and it occurred to me: earlier we made a very "fine-grained" optimization. For the struct above, on a 64-bit system, sizeof it — due to padding — would actually be larger than 8 + 8 + 4 + 1 (21), coming out to 8 + 8 + 8 = 24.

So instead of the usual approach:

str = malloc(sizeof(str) + len + 1)

to allocate memory for a string of length len, we use something like:

str = malloc ((int)((str*)0)->val) + len + 1)

So for "0", we actually allocate 22 bytes.

But what could go wrong with that? Let's attach with gdb again and disassemble to see exactly what's happening:

Well, the problem is on the line at f3b5. GCC reads a word-sized value at 0x10(%rdx); %rdx is the pointer to the zend_string op, and the 0x10 offset is str->len. It turns out GCC cleverly optimized

if (str->len == 1 && str->val[0] == '0')

into a single instruction comparing against the value 0x3000000001....

So, as explained above, because this str is only 22 bytes, when it tries to read 8 bytes starting at offset 16, we actually read 3 bytes past the end of the str struct...... and thus the invalid read.

The problem is clear: a clever GCC optimization caused a harmless report (and 0xffffffffff)............ So I wasted my time.... (Of course, it's still best to fix it. The fix I have in mind now is to allocate at least 24 bytes.)