[compiler-rt] r350139 - Add support for background thread on NetBSD in ASan

Kamil Rytarowski via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 7 08:13:12 PST 2019


On 07.01.2019 16:38, Yvan Roux wrote:
> Kamil, Diana,
> 
> On Mon, 7 Jan 2019 at 15:43, Kamil Rytarowski via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>>
>> I've already replied to Yvan Roux 4 days ago:
> 
> Sorry, I was preempted I didn't had time to investigate further.
> 
>> It's not possible to guess what's wrong.
>>  - Is the background thread started at all?
>>  - Is __local_asan_dyninit optimized away or not executed?
>>  - Is there a timing issue that test needs more time?
> 
> Given that these test cases were OK in Thumbv8 build #1576 and the
> commits involved into build #1577, my guess is that the issue is
> related to __local_asan_dyninit.
> 

Can you debug it or give me remote access to a machine with this setup?

> build #1576 ninja check logs:
> http://lab.llvm.org:8011/builders/clang-cmake-thumbv8-full-sh/builds/1576/steps/ninja%20check%202/logs/stdio
> 
>> I don't have access to setup to reproduce it myself.
>>
>> The patch fixed Android setup and enabled NetBSD.
>>
>> I think that the problem is not caused by my change, but it's a side
>> effect of other bug/optimization. I can mark the test as XFAIL for your
>> setup or add ifdef for your Linux configuration to start the background
>> thread in the same way as before.
>>
>>
>> On 07.01.2019 10:34, Diana Picus wrote:
>>> Hi Kamil,
>>>
>>> I think this is breaking our Thumb buildbots:
>>> http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-full-sh/builds/1263
>>> http://lab.llvm.org:8011/builders/clang-cmake-thumbv8-full-sh/builds/1577
>>>
>>> Can you please have a look?
>>>
>>> Thanks,
>>> Diana
>>>
>>> On Sat, 29 Dec 2018 at 01:35, Kamil Rytarowski via llvm-commits
>>> <llvm-commits at lists.llvm.org> wrote:
>>>>
>>>> Author: kamil
>>>> Date: Fri Dec 28 16:32:07 2018
>>>> New Revision: 350139
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=350139&view=rev
>>>> Log:
>>>> Add support for background thread on NetBSD in ASan
>>>>
>>>> Summary:
>>>> Change the point of calling MaybeStartBackgroudThread() from AsanInitInternal()
>>>> that is too early on NetBSD to a constructor (with aid of C++11 lambda construct).
>>>>
>>>> Enable the code for background thread as is for NetBSD.
>>>>
>>>> Rename test/sanitizer_common/TestCases/Linux/hard_rss_limit_mb_test.cc
>>>> to test/sanitizer_common/TestCases/hard_rss_limit_mb_test.cc and allow runs
>>>> on NetBSD. This tests passes correctly.
>>>>
>>>> Reviewers: vitalybuka, joerg, eugenis
>>>>
>>>> Reviewed By: eugenis
>>>>
>>>> Subscribers: eugenis, kubamracek, fedor.sergeev, llvm-commits, mgorny, #sanitizers
>>>>
>>>> Tags: #sanitizers
>>>>
>>>> Differential Revision: https://reviews.llvm.org/D55887
>>>>
>>>> Added:
>>>>     compiler-rt/trunk/test/sanitizer_common/TestCases/hard_rss_limit_mb_test.cc
>>>>       - copied, changed from r350138, compiler-rt/trunk/test/sanitizer_common/TestCases/Linux/hard_rss_limit_mb_test.cc
>>>> Removed:
>>>>     compiler-rt/trunk/test/sanitizer_common/TestCases/Linux/hard_rss_limit_mb_test.cc
>>>> Modified:
>>>>     compiler-rt/trunk/lib/asan/asan_rtl.cc
>>>>     compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_libcdep.cc
>>>>
>>>> Modified: compiler-rt/trunk/lib/asan/asan_rtl.cc
>>>> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_rtl.cc?rev=350139&r1=350138&r2=350139&view=diff
>>>> ==============================================================================
>>>> --- compiler-rt/trunk/lib/asan/asan_rtl.cc (original)
>>>> +++ compiler-rt/trunk/lib/asan/asan_rtl.cc Fri Dec 28 16:32:07 2018
>>>> @@ -383,6 +383,13 @@ void PrintAddressSpaceLayout() {
>>>>            kHighShadowBeg > kMidMemEnd);
>>>>  }
>>>>
>>>> +static bool UNUSED __local_asan_dyninit = [] {
>>>> +  MaybeStartBackgroudThread();
>>>> +  SetSoftRssLimitExceededCallback(AsanSoftRssLimitExceededCallback);
>>>> +
>>>> +  return false;
>>>> +}();
>>>> +
>>>>  static void AsanInitInternal() {
>>>>    if (LIKELY(asan_inited)) return;
>>>>    SanitizerToolName = "AddressSanitizer";
>>>> @@ -457,9 +464,6 @@ static void AsanInitInternal() {
>>>>    allocator_options.SetFrom(flags(), common_flags());
>>>>    InitializeAllocator(allocator_options);
>>>>
>>>> -  MaybeStartBackgroudThread();
>>>> -  SetSoftRssLimitExceededCallback(AsanSoftRssLimitExceededCallback);
>>>> -
>>>>    // On Linux AsanThread::ThreadStart() calls malloc() that's why asan_inited
>>>>    // should be set to 1 prior to initializing the threads.
>>>>    asan_inited = 1;
>>>>
>>>> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_libcdep.cc
>>>> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_libcdep.cc?rev=350139&r1=350138&r2=350139&view=diff
>>>> ==============================================================================
>>>> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_libcdep.cc (original)
>>>> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_libcdep.cc Fri Dec 28 16:32:07 2018
>>>> @@ -25,7 +25,7 @@ void SetSoftRssLimitExceededCallback(voi
>>>>    SoftRssLimitExceededCallback = Callback;
>>>>  }
>>>>
>>>> -#if SANITIZER_LINUX && !SANITIZER_GO
>>>> +#if (SANITIZER_LINUX || SANITIZER_NETBSD) && !SANITIZER_GO
>>>>  // Weak default implementation for when sanitizer_stackdepot is not linked in.
>>>>  SANITIZER_WEAK_ATTRIBUTE StackDepotStats *StackDepotGetStats() {
>>>>    return nullptr;
>>>> @@ -114,7 +114,7 @@ void WriteToSyslog(const char *msg) {
>>>>  }
>>>>
>>>>  void MaybeStartBackgroudThread() {
>>>> -#if SANITIZER_LINUX && \
>>>> +#if (SANITIZER_LINUX || SANITIZER_NETBSD) && \
>>>>      !SANITIZER_GO  // Need to implement/test on other platforms.
>>>>    // Start the background thread if one of the rss limits is given.
>>>>    if (!common_flags()->hard_rss_limit_mb &&
>>>>
>>>> Removed: compiler-rt/trunk/test/sanitizer_common/TestCases/Linux/hard_rss_limit_mb_test.cc
>>>> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/sanitizer_common/TestCases/Linux/hard_rss_limit_mb_test.cc?rev=350138&view=auto
>>>> ==============================================================================
>>>> --- compiler-rt/trunk/test/sanitizer_common/TestCases/Linux/hard_rss_limit_mb_test.cc (original)
>>>> +++ compiler-rt/trunk/test/sanitizer_common/TestCases/Linux/hard_rss_limit_mb_test.cc (removed)
>>>> @@ -1,44 +0,0 @@
>>>> -// Check hard_rss_limit_mb. Not all sanitizers implement it yet.
>>>> -// RUN: %clangxx -O2 %s -o %t
>>>> -//
>>>> -// Run with limit should fail:
>>>> -// RUN: %env_tool_opts=hard_rss_limit_mb=100                           not %run %t 2>&1 | FileCheck %s
>>>> -// This run uses getrusage:
>>>> -// RUN: %env_tool_opts=hard_rss_limit_mb=100:can_use_proc_maps_statm=0 not %run %t 2>&1 | FileCheck %s
>>>> -//
>>>> -// Run w/o limit or with a large enough limit should pass:
>>>> -// RUN: %env_tool_opts=hard_rss_limit_mb=1000 %run %t
>>>> -// RUN: %run %t
>>>> -//
>>>> -// FIXME: make it work for other sanitizers.
>>>> -// XFAIL: lsan
>>>> -// XFAIL: tsan
>>>> -// XFAIL: msan
>>>> -// XFAIL: ubsan
>>>> -
>>>> -// https://github.com/google/sanitizers/issues/981
>>>> -// UNSUPPORTED: android-26
>>>> -
>>>> -#include <string.h>
>>>> -#include <stdio.h>
>>>> -#include <unistd.h>
>>>> -
>>>> -const int kNumAllocs = 200 * 1000;
>>>> -const int kAllocSize = 1000;
>>>> -volatile char *sink[kNumAllocs];
>>>> -
>>>> -int main(int argc, char **argv) {
>>>> -  for (int i = 0; i < kNumAllocs; i++) {
>>>> -    if ((i % 1000) == 0) {
>>>> -      // Don't write to stderr! Doing that triggers a kernel race condition
>>>> -      // between this thread and the rss-limit thread, and may lose part of the
>>>> -      // output. See https://lkml.org/lkml/2014/2/17/324.
>>>> -      printf("[%d]\n", i);
>>>> -    }
>>>> -    char *x = new char[kAllocSize];
>>>> -    memset(x, 0, kAllocSize);
>>>> -    sink[i] = x;
>>>> -  }
>>>> -  sleep(1);  // Make sure the background thread has time to kill the process.
>>>> -// CHECK: hard rss limit exhausted
>>>> -}
>>>>
>>>> Copied: compiler-rt/trunk/test/sanitizer_common/TestCases/hard_rss_limit_mb_test.cc (from r350138, compiler-rt/trunk/test/sanitizer_common/TestCases/Linux/hard_rss_limit_mb_test.cc)
>>>> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/sanitizer_common/TestCases/hard_rss_limit_mb_test.cc?p2=compiler-rt/trunk/test/sanitizer_common/TestCases/hard_rss_limit_mb_test.cc&p1=compiler-rt/trunk/test/sanitizer_common/TestCases/Linux/hard_rss_limit_mb_test.cc&r1=350138&r2=350139&rev=350139&view=diff
>>>> ==============================================================================
>>>> --- compiler-rt/trunk/test/sanitizer_common/TestCases/Linux/hard_rss_limit_mb_test.cc (original)
>>>> +++ compiler-rt/trunk/test/sanitizer_common/TestCases/hard_rss_limit_mb_test.cc Fri Dec 28 16:32:07 2018
>>>> @@ -19,6 +19,8 @@
>>>>  // https://github.com/google/sanitizers/issues/981
>>>>  // UNSUPPORTED: android-26
>>>>
>>>> +// UNSUPPORTED: freebsd, solaris, darwin
>>>> +
>>>>  #include <string.h>
>>>>  #include <stdio.h>
>>>>  #include <unistd.h>
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 850 bytes
Desc: OpenPGP digital signature
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190107/80d4b01d/attachment.sig>


More information about the llvm-commits mailing list