It was roughly this
read-from-p;
if(p != NULL) write-to-p;
since read-from-p is undefined if p is null, gcc could (and did) legally optimize out the NULL check, so you could end up writing to NULL.
[edit] I noticed that this case of bug is actually mentioned in the regehr article that is linked.
#define TRACE_FOO(foo_ptr) TRACE_INT((foo_ptr)->x)
...
TRACE_ENTER(TRACE_INT(x) TRACE_FOO(foo_ptr));
if(!foo_ptr) return NULL;
use foo-ptr
int x = *p;
if (p == NULL) {
// p can't be NULL without having triggered
// undefined behavior in the first line, so
// this code is removed by the compiler.
return;
}
// ...
if (p == NULL) {
// Moving the check before the dereference
// avoids undefined behavior and resolves
// the issue.
return;
}
int x = *p;
// ...This is actually guaranteed by the standard (although the language used is not "2s complement", but "repeatedly adding or subtracting one more than the maximum value that can be stored in the type until the value is in range").
C requires that these sorts of type conversions happen through unions: using pointer casts is not correct and undefined behavior results.
Actually, loading a member of a union other than the one most recently stored to is undefined behaviour too - it's just that using a union in this way falls into the "...both Clang and GCC nail down a few behaviors that the C standard leaves undefined." category. (And most other compilers besides - there is much historical usage of this).
Otherwise, a great read.
PS– Is it just me or is LLVM coming up more and more these days?
- http://lkml.org/lkml/2009/7/17/187
- http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6...
I've put a gist file demonstrating the problem: https://gist.github.com/968723
#include <stdio.h>
struct agnx_priv {
char demo;
};
struct ieee80211_hw {
struct agnx_priv *priv;
};
struct pci_dev {
struct ieee80211_hw* dev;
};
struct ieee80211_hw* pci_get_drvdata(struct pci_dev *pdev) {
if(!pdev)
return NULL;
return pdev->dev;
}
static void agnx_pci_remove (struct pci_dev *pdev)
{
struct ieee80211_hw *dev = pci_get_drvdata(pdev);
struct agnx_priv *priv = dev->priv;
if (!dev) return;
// This will segfault if the previous if is optimized
printf("%c\n", priv->demo);
}
int main(int argc, char **argv)
{
// (struct pci_dev *)argv[1] is just to avoid compiler optimisations
// =~ NULL if no args are passed.
struct pci_dev * pdev = (struct pci_dev *)argv[1];
agnx_pci_remove(pdev);
return 0;
}
gcc -O2 -ggdb -Wall -Wundef -fno-strict-aliasing \
-fno-common -fdelete-null-pointer-checks test-deref.c \
-o test-deref
gcc -O2 -ggdb -Wall -Wundef -fno-strict-aliasing \
-fno-common -fno-delete-null-pointer-checks test-deref.c \
-o test-derefedit: seems Blogger turned their -O knob to eleven http://status.blogger.com/
"Raises an invalid opcode exception in all operating modes."
What is here undefined? LLVM must not generate such instructions except it it really wants such a exception. (Like Linux's panic() does on x86)