libsanitizer mege from upstream r171973

libsanitizer mege from upstream r171973

[Konstantin Serebryany]

Hi,

The attached patch is the libsanitizer merge from upstream r171973.

Lots of changes. Among other things:
  - slow CFI-based unwinder (off by default for now)
  - more interceptors in asan (read, pread, etc)
  - precise handling of memset/memcpy/etc (detects first overrun byte)
  - new experimental asan allocator (off by default)
  - asan now detects new/free, malloc/delete and new/delete[] mismatches
  - tsan now detects more races IO operations

Patch for libsanitizer is automatically generated by libsanitizer/merge.sh
Tested with
rm -rf */{*/,}libsanitizer \
  && make -j 50 \
  && make -C gcc check-g{cc,++}
RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} asan.exp'

Our internal LLVM bots (Linux, Mac and Android) are also green, but
since the changes are large something may potentially break on other
platforms.

Ok to commit?

--kcc

Re: libsanitizer mege from upstream r171973

[Konstantin Serebryany]

[-- Attachment #1: Type: text/plain, Size: 1209 bytes --]

Sending gzip-ed patch (otherwise it's too large)

On Thu, Jan 10, 2013 at 11:04 AM, Konstantin Serebryany
<konstantin.s.serebryany@gmail.com> wrote:
> Actually attaching the patch.
>
> On Thu, Jan 10, 2013 at 11:01 AM, Konstantin Serebryany
> <konstantin.s.serebryany@gmail.com> wrote:
>> Hi,
>>
>> The attached patch is the libsanitizer merge from upstream r171973.
>>
>> Lots of changes. Among other things:
>>   - slow CFI-based unwinder (off by default for now)
>>   - more interceptors in asan (read, pread, etc)
>>   - precise handling of memset/memcpy/etc (detects first overrun byte)
>>   - new experimental asan allocator (off by default)
>>   - asan now detects new/free, malloc/delete and new/delete[] mismatches
>>   - tsan now detects more races IO operations
>>
>> Patch for libsanitizer is automatically generated by libsanitizer/merge.sh
>> Tested with
>> rm -rf */{*/,}libsanitizer \
>>   && make -j 50 \
>>   && make -C gcc check-g{cc,++}
>> RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} asan.exp'
>>
>> Our internal LLVM bots (Linux, Mac and Android) are also green, but
>> since the changes are large something may potentially break on other
>> platforms.
>>
>> Ok to commit?
>>
>> --kcc

[-- Attachment #2: libsanitizer-171973.patch.gz --]
[-- Type: application/x-gzip, Size: 72125 bytes --]

Re: libsanitizer mege from upstream r171973

[Jakub Jelinek]

On Thu, Jan 10, 2013 at 11:07:26AM +0400, Konstantin Serebryany wrote:
> >> Our internal LLVM bots (Linux, Mac and Android) are also green, but
> >> since the changes are large something may potentially break on other
> >> platforms.
> >>
> >> Ok to commit?

Ok, but it would be nice if the prctl stuff in asan_test.cc could be made
more robust.  Not all Linux kernels support prctl (PR_SET_NAME, ...) (only
2.6.9+), and not all prctl.h headers define PR_SET_NAME.
So, I'd guard the prctl call with additional
#ifdef PR_SET_NAME
#endif
guards, and set some bool variable if the prctl failed (or if it wasn't
#called at all), and if that global bool variable was true, used a different
expected error message (.* in the place of the expected error string).
Or perhaps just have TryToSetThreadName return whether it succeeded or not,
also call TryToSetThreadName in ThreadNamesTest directly and just return
it it failed.

BTW, I had a look at your qsort overflow testcase, and I get:
$ ./xg++ -B ./ -O2 -fsanitize=address -o overflow-in-qsort overflow-in-qsort.cc -B ../x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs/ -B ../x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/ -Wl,-rpath,../x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/ -fno-omit-frame-pointer -g
$ ASAN_OPTIONS=fast_unwind_on_fatal=1 ./overflow-in-qsort 2>&1 | /usr/src/llvm/projects/compiler-rt/lib/asan/scripts/asan_symbolize.py 
=================================================================
==24758== ERROR: AddressSanitizer: global-buffer-overflow on address 0x000000601448 at pc 0x400acf bp 0x7fffb9caee90 sp 0x7fffb9caee88
WRITE of size 4 at 0x000000601448 thread T0
    #0 0x400ace in QsortCallback /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:19
    #1 0x3216c37bb1 in ?? ??:0
0x000000601448 is located 0 bytes to the right of global variable 'global_array (overflow-in-qsort.cc)' (0x601420) of size 40
...
while with
$ ASAN_OPTIONS=fast_unwind_on_fatal=0 ./overflow-in-qsort 2>&1 | /usr/src/llvm/projects/compiler-rt/lib/asan/scripts/asan_symbolize.py 
=================================================================
==24763== ERROR: AddressSanitizer: global-buffer-overflow on address 0x000000601448 at pc 0x400acf bp 0x7ffff72db050 sp 0x7ffff72db048
WRITE of size 4 at 0x000000601448 thread T0
    #0 0x400ace in QsortCallback /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:19
    #1 0x3216c37bb1 in ?? ??:0
    #2 0x3216c3809b in ?? ??:0
    #3 0x400b1b in MyQsort /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:26
    #4 0x4008c9 in main /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:33
    #5 0x3216c21734 in ?? ??:0
    #6 0x400944 in _start ??:0
0x000000601448 is located 0 bytes to the right of global variable 'global_array (overflow-in-qsort.cc)' (0x601420) of size 40
...
which is much better (Fedora 17 x86_64).

Similarly for 32-bit:

$ ./xg++ -B ./ -O2 -fsanitize=address -o overflow-in-qsort overflow-in-qsort.cc -B ../x86_64-unknown-linux-gnu/32/libstdc++-v3/src/.libs/ -B ../x86_64-unknown-linux-gnu/32/libsanitizer/asan/.libs/ -Wl,-rpath,../x86_64-unknown-linux-gnu/32/libsanitizer/asan/.libs/ -fno-omit-frame-pointer -g -m32
$ ASAN_OPTIONS=fast_unwind_on_fatal=1 ./overflow-in-qsort 2>&1 | /usr/src/llvm/projects/compiler-rt/lib/asan/scripts/asan_symbolize.py 
=================================================================
==24774== ERROR: AddressSanitizer: global-buffer-overflow on address 0x08049fc8 at pc 0x80488fe bp 0xffd03548 sp 0xffd0353c
WRITE of size 4 at 0x08049fc8 thread T0
    #0 0x80488fd in QsortCallback /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:19
    #1 0x4e00fcb2 in ?? ??:0
addr2line: '': No such file
    #2 0x27ffffff in
0x08049fc8 is located 0 bytes to the right of global variable 'global_array (overflow-in-qsort.cc)' (0x8049fa0) of size 40
...

vs.

ASAN_OPTIONS=fast_unwind_on_fatal=0 ./overflow-in-qsort 2>&1 | /usr/src/llvm/projects/compiler-rt/lib/asan/scripts/asan_symbolize.py 
=================================================================
==24780== ERROR: AddressSanitizer: global-buffer-overflow on address 0x08049fc8 at pc 0x80488fe bp 0xffbfcf88 sp 0xffbfcf7c
WRITE of size 4 at 0x08049fc8 thread T0
    #0 0x80488fd in QsortCallback /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:19
    #1 0x4e00fcb2 in ?? ??:0
    #2 0x4e010206 in ?? ??:0
    #3 0x4e0102ee in ?? ??:0
    #4 0x8048955 in MyQsort /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:26
    #5 0x804870c in main /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:33
    #6 0x4dff8634 in ?? ??:0
    #7 0x8048794 in _start ??:0
0x08049fc8 is located 0 bytes to the right of global variable 'global_array (overflow-in-qsort.cc)' (0x8049fa0) of size 40

Clearly asan_symbolize.py doesn't cope with debug info stripped to separate file (I have glibc-debuginfo installed), but AFAIK
iant's libbacktrace doesn't yet either (nor understand dwz DWARF extensions, but Fedora 17 still doesn't use them, only
Fedora 18+ does).  Anyway, I've yet to see a testcase where the fast unwinder results in better backtrace with default options
(sure, one can compile with -fno-asynchronous-unwind-tables).

	Jakub

Re: libsanitizer mege from upstream r171973

[Konstantin Serebryany]

On Thu, Jan 10, 2013 at 2:59 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Jan 10, 2013 at 11:07:26AM +0400, Konstantin Serebryany wrote:
>> >> Our internal LLVM bots (Linux, Mac and Android) are also green, but
>> >> since the changes are large something may potentially break on other
>> >> platforms.
>> >>
>> >> Ok to commit?
>
> Ok, but it would be nice if the prctl stuff in asan_test.cc could be made
> more robust.  Not all Linux kernels support prctl (PR_SET_NAME, ...) (only
> 2.6.9+), and not all prctl.h headers define PR_SET_NAME.



> So, I'd guard the prctl call with additional
> #ifdef PR_SET_NAME

Ok.
I'd prefer to guard the whole test with #if defined(__linux__) &&
defined(PR_SET_NAME).

> #endif
> guards, and set some bool variable if the prctl failed (or if it wasn't
> #called at all), and if that global bool variable was true, used a different
> expected error message (.* in the place of the expected error string).
> Or perhaps just have TryToSetThreadName return whether it succeeded or not,
> also call TryToSetThreadName in ThreadNamesTest directly and just return
> it it failed.
>
> BTW, I had a look at your qsort overflow testcase, and I get:
> $ ./xg++ -B ./ -O2 -fsanitize=address -o overflow-in-qsort overflow-in-qsort.cc -B ../x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs/ -B ../x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/ -Wl,-rpath,../x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/ -fno-omit-frame-pointer -g
> $ ASAN_OPTIONS=fast_unwind_on_fatal=1 ./overflow-in-qsort 2>&1 | /usr/src/llvm/projects/compiler-rt/lib/asan/scripts/asan_symbolize.py
> =================================================================
> ==24758== ERROR: AddressSanitizer: global-buffer-overflow on address 0x000000601448 at pc 0x400acf bp 0x7fffb9caee90 sp 0x7fffb9caee88
> WRITE of size 4 at 0x000000601448 thread T0
>     #0 0x400ace in QsortCallback /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:19
>     #1 0x3216c37bb1 in ?? ??:0
> 0x000000601448 is located 0 bytes to the right of global variable 'global_array (overflow-in-qsort.cc)' (0x601420) of size 40
> ...
> while with
> $ ASAN_OPTIONS=fast_unwind_on_fatal=0 ./overflow-in-qsort 2>&1 | /usr/src/llvm/projects/compiler-rt/lib/asan/scripts/asan_symbolize.py
> =================================================================
> ==24763== ERROR: AddressSanitizer: global-buffer-overflow on address 0x000000601448 at pc 0x400acf bp 0x7ffff72db050 sp 0x7ffff72db048
> WRITE of size 4 at 0x000000601448 thread T0
>     #0 0x400ace in QsortCallback /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:19
>     #1 0x3216c37bb1 in ?? ??:0
>     #2 0x3216c3809b in ?? ??:0
>     #3 0x400b1b in MyQsort /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:26
>     #4 0x4008c9 in main /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:33
>     #5 0x3216c21734 in ?? ??:0
>     #6 0x400944 in _start ??:0
> 0x000000601448 is located 0 bytes to the right of global variable 'global_array (overflow-in-qsort.cc)' (0x601420) of size 40
> ...
> which is much better (Fedora 17 x86_64).

Good!

>
> Similarly for 32-bit:
>
> $ ./xg++ -B ./ -O2 -fsanitize=address -o overflow-in-qsort overflow-in-qsort.cc -B ../x86_64-unknown-linux-gnu/32/libstdc++-v3/src/.libs/ -B ../x86_64-unknown-linux-gnu/32/libsanitizer/asan/.libs/ -Wl,-rpath,../x86_64-unknown-linux-gnu/32/libsanitizer/asan/.libs/ -fno-omit-frame-pointer -g -m32
> $ ASAN_OPTIONS=fast_unwind_on_fatal=1 ./overflow-in-qsort 2>&1 | /usr/src/llvm/projects/compiler-rt/lib/asan/scripts/asan_symbolize.py
> =================================================================
> ==24774== ERROR: AddressSanitizer: global-buffer-overflow on address 0x08049fc8 at pc 0x80488fe bp 0xffd03548 sp 0xffd0353c
> WRITE of size 4 at 0x08049fc8 thread T0
>     #0 0x80488fd in QsortCallback /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:19
>     #1 0x4e00fcb2 in ?? ??:0
> addr2line: '': No such file
>     #2 0x27ffffff in
> 0x08049fc8 is located 0 bytes to the right of global variable 'global_array (overflow-in-qsort.cc)' (0x8049fa0) of size 40
> ...
>
> vs.
>
> ASAN_OPTIONS=fast_unwind_on_fatal=0 ./overflow-in-qsort 2>&1 | /usr/src/llvm/projects/compiler-rt/lib/asan/scripts/asan_symbolize.py
> =================================================================
> ==24780== ERROR: AddressSanitizer: global-buffer-overflow on address 0x08049fc8 at pc 0x80488fe bp 0xffbfcf88 sp 0xffbfcf7c
> WRITE of size 4 at 0x08049fc8 thread T0
>     #0 0x80488fd in QsortCallback /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:19
>     #1 0x4e00fcb2 in ?? ??:0
>     #2 0x4e010206 in ?? ??:0
>     #3 0x4e0102ee in ?? ??:0
>     #4 0x8048955 in MyQsort /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:26
>     #5 0x804870c in main /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:33
>     #6 0x4dff8634 in ?? ??:0
>     #7 0x8048794 in _start ??:0
> 0x08049fc8 is located 0 bytes to the right of global variable 'global_array (overflow-in-qsort.cc)' (0x8049fa0) of size 40
>
> Clearly asan_symbolize.py doesn't cope with debug info stripped to separate file (I have glibc-debuginfo installed), but AFAIK
> iant's libbacktrace doesn't yet either (nor understand dwz DWARF extensions, but Fedora 17 still doesn't use them, only
> Fedora 18+ does).  Anyway, I've yet to see a testcase where the fast unwinder results in better backtrace with default options
> (sure, one can compile with -fno-asynchronous-unwind-tables).

Yesterday I've compared the two unwinders on chromium and saw no such
case (there were few where CFI-based one is better, just like with
qsort).
It is very likely that I will flip fast_unwind_on_fatal to 0 next week.

--kcc


>
>         Jakub

Re: libsanitizer mege from upstream r171973

[Jakub Jelinek]

On Thu, Jan 10, 2013 at 03:27:41PM +0400, Konstantin Serebryany wrote:
> > So, I'd guard the prctl call with additional
> > #ifdef PR_SET_NAME
> 
> Ok.
> I'd prefer to guard the whole test with #if defined(__linux__) &&
> defined(PR_SET_NAME).

That is not sufficient.  You can have PR_SET_NAME defined in the headers,
but still the underlying kernel doesn't need to handle it.

> It is very likely that I will flip fast_unwind_on_fatal to 0 next week.

Perfect, thanks.

	Jakub

Re: libsanitizer mege from upstream r171973

[Konstantin Serebryany]

> That is not sufficient.  You can have PR_SET_NAME defined in the headers,
> but still the underlying kernel doesn't need to handle it.
>
Like this?
http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/tests/asan_test.cc?r1=172048&r2=172059&pathrev=172059&diff_format=h

Re: libsanitizer mege from upstream r171973

[Jakub Jelinek]

On Thu, Jan 10, 2013 at 03:57:44PM +0400, Konstantin Serebryany wrote:
> > That is not sufficient.  You can have PR_SET_NAME defined in the headers,
> > but still the underlying kernel doesn't need to handle it.
> >
> Like this?
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/tests/asan_test.cc?r1=172048&r2=172059&pathrev=172059&diff_format=h

Yeah.  I wouldn't create a separate thread just to call prctl, I believe it
should work even in the initial thread, doesn't it?  But perhaps you don't
want to affect the name of the original testcase when running under gtest
native.

	Jakub

Re: libsanitizer mege from upstream r171973

[Konstantin Serebryany]

On Thu, Jan 10, 2013 at 4:23 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Jan 10, 2013 at 03:57:44PM +0400, Konstantin Serebryany wrote:
>> > That is not sufficient.  You can have PR_SET_NAME defined in the headers,
>> > but still the underlying kernel doesn't need to handle it.
>> >
>> Like this?
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/tests/asan_test.cc?r1=172048&r2=172059&pathrev=172059&diff_format=h
>
> Yeah.  I wouldn't create a separate thread just to call prctl, I believe it
> should work even in the initial thread, doesn't it?  But perhaps you don't
> want to affect the name of the original testcase when running under gtest
> native.
Exactly.

Thanks!

--kcc

>
>         Jakub

Re: libsanitizer mege from upstream r171973

[Dmitry Vyukov]

On Thu, Jan 10, 2013 at 11:01 AM, Konstantin Serebryany
<konstantin.s.serebryany@gmail.com> wrote:
> Hi,
>
> The attached patch is the libsanitizer merge from upstream r171973.
>
> Lots of changes. Among other things:
>   - slow CFI-based unwinder (off by default for now)
>   - more interceptors in asan (read, pread, etc)
>   - precise handling of memset/memcpy/etc (detects first overrun byte)
>   - new experimental asan allocator (off by default)
>   - asan now detects new/free, malloc/delete and new/delete[] mismatches
>   - tsan now detects more races IO operations
>
> Patch for libsanitizer is automatically generated by libsanitizer/merge.sh
> Tested with
> rm -rf */{*/,}libsanitizer \
>   && make -j 50 \
>   && make -C gcc check-g{cc,++}
> RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} asan.exp'
>
> Our internal LLVM bots (Linux, Mac and Android) are also green, but
> since the changes are large something may potentially break on other
> platforms.
>
> Ok to commit?

Rubberstamp LGTM (already reviewed upstream)

Re: libsanitizer mege from upstream r171973

[Jack Howarth]

On Thu, Jan 10, 2013 at 11:01:30AM +0400, Konstantin Serebryany wrote:
> Hi,
> 
> The attached patch is the libsanitizer merge from upstream r171973.
> 
> Lots of changes. Among other things:
>   - slow CFI-based unwinder (off by default for now)
>   - more interceptors in asan (read, pread, etc)
>   - precise handling of memset/memcpy/etc (detects first overrun byte)
>   - new experimental asan allocator (off by default)
>   - asan now detects new/free, malloc/delete and new/delete[] mismatches
>   - tsan now detects more races IO operations
> 
> Patch for libsanitizer is automatically generated by libsanitizer/merge.sh
> Tested with
> rm -rf */{*/,}libsanitizer \
>   && make -j 50 \
>   && make -C gcc check-g{cc,++}
> RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} asan.exp'
> 
> Our internal LLVM bots (Linux, Mac and Android) are also green, but
> since the changes are large something may potentially break on other
> platforms.
> 
> Ok to commit?
> 
> --kcc

kcc,
    Any chance that we can get http://llvm-reviews.chandlerc.com/D216 approved and committed
into llvm so that the libsanitizer merge can also eliminate the need for the Core Foundation
framework? It appears that you are assigned as the reviewer for that patch. Unfortunately
diff #518 has bit-rotted against current compiler-rt svn so Alexander will need to correct 
the patch for that.
              Jack