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

Mon Jan 7 07:38:22 PST 2019

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.

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