On Thu, Nov 13, 2014 at 1:16 AM, Jakub Jelinek <ja...@redhat.com> wrote:
> On Wed, Nov 12, 2014 at 05:35:48PM -0800, Konstantin Serebryany wrote:
>> Here is one more merge of libsanitizer (last one was in Sept).
>>
>> Tested on x86_64 Ubuntu 14.04 like this:
>> rm -rf */{*/,}libsanitizer && make -j 50
>> make -j 40 -C gcc check-g{cc,++}
>> RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} asan.exp' && \
>> make -j 40 -C gcc check-g{cc,++}
>> RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} tsan.exp' && \
>> make -j 40 -C gcc check
>> RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} ubsan.exp' && \
>> echo PASS
>>
>> Expected ChangeLog entry:
>>
>> 2014-11-12 Kostya Serebryany <k...@google.com>
>>
>> * All source files: Merge from upstream r221802.
>> * sanitizer_common/sanitizer_symbolizer_libbacktrace.cc
>> (LibbacktraceSymbolizer::SymbolizeData): replace 'address'
>> with 'start' to follow the new interface.
>
> Capital R in Replace. All lines are indented by single tab, not tab
> and two spaces.
>
>> * asan/Makefile.am (AM_CXXFLAGS): added -std=c++11.
>
> Capital A in Added. Also, I wonder if we shouldn't use -std=gnu++11
> instead. As the sources are compiled by newly built compiler, it should be
> generally fine to use extensions in there.
in llvm we use -std=c++11, so I use it here for consistency.
>
>> * interception/Makefile.am (AM_CXXFLAGS): added -std=c++11.
>> * libbacktrace/Makefile.am (AM_CXXFLAGS): added -std=c++11.
>> * lsan/Makefile.am (AM_CXXFLAGS): added -std=c++11.
>> * sanitizer_common/Makefile.am (sanitizer_common_files): Added new
>> files.
>> (AM_CXXFLAGS): added -std=c++11.
>> * tsan/Makefile.am (AM_CXXFLAGS): added -std=c++11.
>> * ubsan/Makefile.am (AM_CXXFLAGS): added -std=c++11.
>
> Ditto.
>
>> * asan/Makefile.in: Regenerate.
>> * interception/Makefile.in: Regenerate.
>> * libbacktrace/Makefile.in: Regenerate.
>> * lsan/Makefile.in: Regenerate.
>> * sanitizer_common/Makefile.in: Regenerate.
>> * tsan/Makefile.in: Regenerate.
>> * ubsan/Makefile.in: Regenerate.
>
> Other than that, it looks good to me, I've bootstrapped/regtested
> it on x86_64-linux and i686-linux too. So, with those changes ok for trunk
> (how do you decide about c++11 vs. gnu++11 I'll leave to you).
Fixed all, committed. r217518.
>
> A few questions regarding possible changes on the compiler side:
> 1) is __asan_poison_intra_object_redzone/__asan_unpoison_intra_object_redzone
> just for the ABI incompatible putting of red zones in between fields
> in structures? How do you handle whole struct copying in that case?
This is all highly experimental:
https://code.google.com/p/address-sanitizer/wiki/IntraObjectOverflow
Currently we apply this instrumentation only to C++ classes that are
a) non-standard-layout, i.e. we are allowed by the standard to
reshuffle the fields and add paddings.
b) have a DTOR, where we can do the unpoison.
Even with this strict limitation we hit lots of failures where users
make assumptions about the layout or size of non-standard-layout
types.
We do find juicy bugs in this mode so we'll likely continue the
investigation and try to reduce the current limitations.
> Could it be done without changing ABI for a subset of structs
> which have natural padding in them?
Quite likely. But we will need to figure out where to unpoison the paddings.
> 2) regarding the tsan memory layout changes, is it now possible to support
> non-pie binaries? If yes, we should probably remove the:
> %{!pie:%{!shared:%e-fsanitize=thread linking must be done with -pie or
> -shared}}}\
> and add testcases that would test that.
Yes, that was one of the reasons for the change.
But let's hear from Dmitry if he is ready to remove -pie now or wants
to do some more testing.
--kcc
>
> Jakub
