+eugenis (what kind of testing on ARM are we doing upstream?)
On Fri, Nov 14, 2014 at 7:44 AM, Christophe Lyon
<christophe.l...@linaro.org> wrote:
> 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?
Evgenyi can answer that, I hope.
Christophe, I encourage you to add more ARM testing to LLVM build bots.
> 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
