Re: libsanitizer merge from upstream r221802

[Konstantin Serebryany <konstantin.s.serebryany@gmail.com>]

Let's continue the discussion there, we can do another merge quickly
or do a cherry pick to GCC once we have a solution.
So far I don't see one. (other than not supporting the old kernels, of course)

On Fri, Nov 14, 2014 at 2:38 AM, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
> On 13 November 2014 21:44, Konstantin Serebryany
> <konstantin.s.serebryany@gmail.com> wrote:
>> On Thu, Nov 13, 2014 at 1:16 AM, Jakub Jelinek <jakub@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  <kcc@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.
>>
>
> Hmm
> So as already reported on the llvm lists, this has the side effect of
> breaking the build for aarch64 when using "old" kernel headers.
> I wish the discussion at
> http://reviews.llvm.org/D6026
> had converged before merging incorrect things into GCC.
>
>>
>>>
>>> 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