Page MenuHomePhabricator

Implement pthread_exit() interceptor for Thread sanitizer
ClosedPublic

Authored by yuri on Wed, Feb 6, 11:49 PM.

Details

Reviewers
dvyukov
Summary

This change is preparation for fiber support.

Diff Detail

Event Timeline

yuri created this revision.Wed, Feb 6, 11:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptWed, Feb 6, 11:49 PM
Herald added subscribers: Restricted Project, llvm-commits, jfb, kubamracek. · View Herald Transcript
dvyukov accepted this revision.Thu, Feb 7, 2:43 AM
This revision is now accepted and ready to land.Thu, Feb 7, 2:43 AM

http://lab.llvm.org:8011/builders/sanitizer-ppc64be-linux/builds/11338

: 'RUN: at line 1';      /home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/clang_build/./bin/clang  -fsanitize=thread -Wall  -m64  -gline-tables-only -I/home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm/projects/compiler-rt/test/tsan/../ -O1 /home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm/projects/compiler-rt/test/tsan/thread_exit.c -o /home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/clang_build/projects/compiler-rt/test/tsan/POWERPC64Config/Output/thread_exit.c.tmp &&  /home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/clang_build/projects/compiler-rt/test/tsan/POWERPC64Config/Output/thread_exit.c.tmp 2>&1 | FileCheck /home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm/projects/compiler-rt/test/tsan/thread_exit.c
--
Exit Code: 1

Command Output (stderr):
--
/home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm/projects/compiler-rt/test/tsan/thread_exit.c:25:11: error: CHECK: expected string not found in input
// CHECK: PASS
          ^
<stdin>:1:1: note: scanning from here
ThreadSanitizer:DEADLYSIGNAL
^
<stdin>:1:4: note: possible intended match here
ThreadSanitizer:DEADLYSIGNAL
   ^
yuri added a comment.Thu, Feb 7, 11:21 PM

Can someone with access to ppc64be system help to investigate the issue?

  1. Check if the test works without any sanitizers.
  2. What versions of gcc and glibc are installed on the host?
  3. Run test under gdb and collect stack trace at crash point.
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>

int var;

void *Thread(void *x) {
  pthread_exit(&var);
  return 0;
}

int main() {
  pthread_t t;
  pthread_create(&t, 0, Thread, 0);
  void *retval = 0;
  pthread_join(t, &retval);
  if (retval != &var) {
    fprintf(stderr, "Unexpected return value\n");
    exit(1);
  }
  fprintf(stderr, "PASS\n");
  return 0;
}
  • Forwarded message ---------

From: Alex L
Date: Fri, Feb 8, 2019 at 1:17 AM
Subject: Re: [compiler-rt] r353385 - tsan: Implement pthread_exit() interceptor for Thread sanitizer

Hi Dmitry,

It looks like your change has caused a failure of the Darwin/dispatch_main.mm TSAN test on our CI:

  • TEST 'ThreadSanitizer-x86_64-iossim :: Darwin/dispatch_main.mm' FAILED ****

Script:

: 'RUN: at line 4'; /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-expensive/llvm/projects/compiler-rt/test/sanitizer_common/ios_commands/iossim_compile.py /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-expensive/clang-build/./bin/clang -fsanitize=thread -Wall -arch x86_64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator11.4.sdk -gline-tables-only -I/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-expensive/llvm/projects/compiler-rt/test/tsan/../ /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-expensive/llvm/projects/compiler-rt/test/tsan/Darwin/dispatch_main.mm -o /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-expensive/clang-build/tools/clang/runtime/compiler-rt-bins/test/tsan/IOSSimX86_64Config/Darwin/Output/dispatch_main.mm.tmp -framework Foundation

: 'RUN: at line 5'; /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-expensive/llvm/projects/compiler-rt/test/sanitizer_common/ios_commands/iossim_run.py /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-expensive/clang-build/tools/clang/runtime/compiler-rt-bins/test/tsan/IOSSimX86_64Config/Darwin/Output/dispatch_main.mm.tmp 2>&1 | FileCheck /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-expensive/llvm/projects/compiler-rt/test/tsan/Darwin/dispatch_main.mm

Exit Code: 66


Here's the full log:

http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-expensive/13539/console

I narrowed it down to your commit.
You can reproduce it by running 'check-tsan' on macOS with the following CMake options "-DCMAKE_BUILD_TYPE=Release" "-DLLVM_ENABLE_ASSERTIONS=On" "-DLLVM_ENABLE_EXPENSIVE_CHECK=ON".

Could you please take a look?

Thanks,
Alex

One thing that I noticed is that pthread_exit is a noreturn function and SCOPED_TSAN_INTERCEPTOR creates a local object with destructor. At the very least this leads to confusing code where readers assumption is that the destructor will run, but it will not.
Perhaps we should try to remove the SCOPED_TSAN_INTERCEPTOR entirely as the CHECK that we want to add here does not need it.

yuri added a comment.Fri, Feb 8, 2:39 AM

It looks like your change has caused a failure of the Darwin/dispatch_main.mm TSAN test on our CI:

I will try to reproduce it myself and report my findings later

yuri added a comment.Fri, Feb 8, 2:40 AM

One thing that I noticed is that pthread_exit is a noreturn function and SCOPED_TSAN_INTERCEPTOR creates a local object with destructor. At the very least this leads to confusing code where readers assumption is that the destructor will run, but it will not.
Perhaps we should try to remove the SCOPED_TSAN_INTERCEPTOR entirely as the CHECK that we want to add here does not need it.

Let's fix after resolution of test failures

yuri added a comment.Fri, Feb 8, 10:44 AM

Fix for macOS is in D57963

vitalybuka reopened this revision.Sat, Feb 9, 2:00 AM

Removed interceptor in r353604

This revision is now accepted and ready to land.Sat, Feb 9, 2:00 AM

It still crashes even without interceptor

yuri added a comment.Sat, Feb 9, 9:31 AM

It still crashes even without interceptor

Looks like stack unwinding (pthread_exit depends on it) is completly broken on this platform.

I suggest to restore interceptor code and disable this test or mark it as expected failure on ppc64be.

I've seen some fixes has landed over the weekend. Is this fixed now? Or some breakages still remain?

yuri added a comment.Mon, Feb 11, 1:56 AM

I've seen some fixes has landed over the weekend. Is this fixed now? Or some breakages still remain?

macOS is fixed.
Test on ppc64 is disabled — pthread_exit does not work there without any interceptors.

I've seen some fixes has landed over the weekend. Is this fixed now? Or some breakages still remain?

macOS is fixed.
Test on ppc64 is disabled — pthread_exit does not work there without any interceptors.

Great! Thanks for taking care of this.