On 13 November 2014 21:44, Konstantin Serebryany
<konstantin.s.serebry...@gmail.com> wrote:
> 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.
>
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
