On 14 November 2014 11:38, Christophe Lyon <christophe.l...@linaro.org> wrote:
> 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.
>
Hi again,
Looking at the results on ARM platforms, I've noticed regressions when
the compiler generates code in Thumb mode (as opposed to ARM mode).
The backtraces are incomplete, thus making the tests fail.
For instance, in heap-overflow-1.c, we had:
#0 0x408c2207 in __interceptor_malloc
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libsanitizer/asan/asan_malloc_linux.cc:38^M
#1 0x888b in main
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/c-c++-common/asan/heap-overflow-1.c:19^M
and now:
#0 0x408c1eff in __interceptor_malloc
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libsanitizer/asan/asan_malloc_linux.cc:38^M
#1 0x407fcb83 (<unknown module>)^M
I'm not familiar with LLVM+sanitizer testing: what kind of tests are
performed upstream before committing changes?
Do both ARM & Thumb modes pass LLVM+sanitizer tests? Do they check
backtraces as in GCC testsuite?
>>
>>>
>>> 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
