address-sanitizer round 2
For the upcoming 5.16 I decided to check our code again with address-sanitizer, google's open-source memory checker.
At the first round address-sanitizer was still a bit immature, I had to use a black list for false positives. With the current versions all the false positives has been fixed and clang 3.1 has address-sanitizer included.
address-sanitizer is a memory checker similar to mudflap, but superior to valgrind or coverity and others. It catches more error types, esp. invalid access to globals and to stack addresses, and use after free and use after return. It does so by using shadow memory maps for all pointers and instrumenting the accesses. This is fast, but needs more memory. ASan (short for address-sanitizer) hashes the pointer maps, so it needs much less memory than a full old-style fence checker, which use insane amounts of memory. valgrind can be used to catch memory leaks (i.e. if you are writing daemons) but should not be used to catch pointer errors.
It's slows down code by factor 1.5-2 compared to valgrind which is 10-20x slower. It can compile code with full optimizations and without -g. I use asan with -DEBUGGING similar to my usage of -DDEBUG_LEAKING_SCALARS. Using valgrind hurts, using asan is straightforward and transparent, you are not hurt by it at all. Note that Perl was checked with coverity and valgrind extensively before.
With the first attempt I had about 20 problems identified, and the very first appeared to be a real bug, the rest looked like false posivites, but I was not sure about it.
In the second round I have found 4 core bugs, and I'm now checking CPAN XS code. I'm checking blead with DEBUGGING, threaded and unthreaded.
Core: No invalid stack access, only globals and even one heap bug, undetected by valgrind.
#111594: 0ffb95f Socket.xs heap-buffer-overflow with abstract AF_UNIX paths
#111586: sdbm.c: fix off-by-one access to global ".dir"
#72700: Copy&paste List::Util BOOT bug, reading global past 2 bytes
#111610: XS::APItest::clone_with_stack heap-use-after-free on PL_curcop (without patch, I guess it's just a bad test)
Build instructions
See http://code.google.com/p/address-sanitizer/wiki/HowToBuild
cd ~
svn co http://llvm.org/svn/llvm-project/llvm/trunk llvm
cd llvm
R=$(svn info | grep Revision: | awk '{print $2}')
# I tested with r152121 and r152199
(cd tools && svn co -r $R \
http://llvm.org/svn/llvm-project/cfe/trunk clang)
(cd projects && svn co -r $R \
http://llvm.org/svn/llvm-project/compiler-rt/trunk compiler-rt)
mkdir build
Move away any existing clang, clang++ and llvm-gcc as older versions will not be able to compile llvm.
(cd build && ../configure --enable-optimized && make -j 10)
# Build and test asan run-time library
cd projects/compiler-rt/lib/asan/
make -f Makefile.old get_third_party
This might need to patch the shebang of the python tools from /usr/bin/python2.4 to /usr/bin/python
make -f Makefile.old test -j 10
# Install clang and asan run-time into a separate directory
# ../asan_clang_linux
make -f Makefile.old install
cd <perl-git>
patch perl with my three asan fixes:
- 111594: 0ffb95f Socket.xs heap-buffer-overflow with abstract AF_UNIX paths
- 111586: sdbm.c: fix off-by-one access to global ".dir"
- 72700: Copy&paste List::Util BOOT bug, reading global past 2 bytes
build perl with:
./Configure -de -Dusedevel -DEBUGGING -Doptimize=-g3 \
-Dcc=~/llvm/projects/compiler-rt/lib/asan_clang_linux/bin/clang \
-Accflags=-faddress-sanitizer -Aldflags=-faddress-sanitizer \
-Alddlflags=-faddress-sanitizer
CPAN samples
Interestingly the first CPAN errors were all in modules maintained by me. B-Flags-0.06 and B-Generate-1.44 are the new fixed versions. Yes, I'm a lisp programmer :), but to my defense, the wrong code was not written by me.
B-Generate
static SV *specialsv_list[6];
...
specialsv_list[6] = (SV*)pWARN_STD; // asan warned here
Fix:
static SV *specialsv_list[7];
=================================================================
==27781== ERROR: AddressSanitizer global-buffer-overflow on address 0x2b176c37a730 at pc 0x2b176c2833a7 bp 0x7fff58c4dfb0 sp 0x7fff58c4dfa8
WRITE of size 8 at 0x2b176c37a730 thread T0
0x2b176c37a730 is located 0 bytes to the right of global variable 'specialsv_list (Generate.c)' (0x2b176c37a700) of size 48
B-Flags
==19039== ERROR: AddressSanitizer heap-buffer-overflow on address 0x2b0995e2d47f at pc 0x2b09965e709f bp 0x7fff150fcfb0 sp 0x7fff150fcfa8
READ of size 1 at 0x2b0995e2d47f thread T0
The fix is here:
- if (*(SvEND(RETVAL) - 1) == ',') {
+ if (SvCUR(RETVAL) && (*(SvEND(RETVAL) - 1) == ',')) {
RETVAL
was an empty string in this case, and checking SvEND - 1
for a zero size string is invalid.
I submitted a YAPC::US talk proposal titled "Forget valgrind, use address-sanitizer"
Tuning tips
Perl is compared to other bigger apps like chromium and mozilla a malloc hog. The measured slowdown is 3x, compared to 2x in the others.
Kosta (asan dev.): "According to our measurements 400.perlbench suffers the greatest slowdown from asan compared to all other benchmarks -- 2.5x.
And that is measured with stack unwinding turned off (i.e. heap-related warnings will be reported w/o malloc/free stack traces). With stack traces enabled the 400.perlbench's slowdown is way over 3x.
This is easy to explain -- perl is very malloc intensive, allocates small
chunks and mostly reads single bytes
You may want to try env var ASAN_OPTIONS=malloc_context_size=1
to speed
up things."
To defend my beloved Valgrind Memcheck,
I should say that it is still unbeatable in finding uninitialized memory reads (asan doesn't do that).
Also, Valgrind may find addressability bugs in libraries you don't compile (asan does that only for a small subset of libc functions).
You already mentioned http://code.google.com/p/address-sanitizer/wiki/ComparisonOfMemoryTools which provides more comparison.
Can you prod me about these on IRC at some point I appear to be free? I now have the power to fix two of them (Socket, List::Util). Thanks :)
kcc: You are right, I forgot that. I tend to be too unfair.
But valgrind only works for heap pointers, not for globals or stack pointers.