Skip to content

Commit

Permalink
[tsan] Do not report races coming from deinitializers and _Block_release
Browse files Browse the repository at this point in the history
TSan does not observe the guaranteed syncronization between the ref
count drop to zero and object destruction. This can lead to false positive
reports.

This patch adds an attribute to deinitializers to ignore memory accesses
at run time. It also moves the logic to add sanitizer attributes from
IRGenFunction to IRGenSILFunction, which means that the automatically
generated code such as _Block_release handler will not be instrumented
and the accesses made in them will be invisible to TSan.

Solves a problem similar to what's addressed in clang commit:
https://reviews.llvm.org/D25857
  • Loading branch information
AnnaZaks committed Jan 5, 2017
1 parent 5cff812 commit 18c10c5
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 10 deletions.
1 change: 1 addition & 0 deletions lib/IRGen/GenFunc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1490,6 +1490,7 @@ static llvm::Function *emitBlockDisposeHelper(IRGenModule &IGM,
IGM.getModule());
func->setAttributes(IGM.constructInitialAttributes());
IRGenFunction IGF(IGM, func);
assert(!func->hasFnAttribute(llvm::Attribute::SanitizeThread));
if (IGM.DebugInfo)
IGM.DebugInfo->emitArtificialFunction(IGF, func);

Expand Down
8 changes: 0 additions & 8 deletions lib/IRGen/IRGenFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,6 @@ IRGenFunction::IRGenFunction(IRGenModule &IGM, llvm::Function *Fn,
IGM.DebugInfo->pushLoc();
}

// Apply sanitizer attributes to the function.
// TODO: Check if the function is ASan black listed either in the external
// file or via annotations.
if (IGM.IRGen.Opts.Sanitize == SanitizerKind::Address)
Fn->addFnAttr(llvm::Attribute::SanitizeAddress);
if (IGM.IRGen.Opts.Sanitize == SanitizerKind::Thread)
Fn->addFnAttr(llvm::Attribute::SanitizeThread);

emitPrologue();
}

Expand Down
18 changes: 16 additions & 2 deletions lib/IRGen/IRGenSIL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1018,8 +1018,22 @@ IRGenSILFunction::IRGenSILFunction(IRGenModule &IGM,
SILFunction *f)
: IRGenFunction(IGM, IGM.getAddrOfSILFunction(f, ForDefinition),
f->getDebugScope(), f->getLocation()),
CurSILFn(f)
{}
CurSILFn(f) {
// Apply sanitizer attributes to the function.
// TODO: Check if the function is ASan black listed either in the external
// file or via annotations.
if (IGM.IRGen.Opts.Sanitize == SanitizerKind::Address)
CurFn->addFnAttr(llvm::Attribute::SanitizeAddress);
if (IGM.IRGen.Opts.Sanitize == SanitizerKind::Thread) {
if (dyn_cast_or_null<DestructorDecl>(f->getDeclContext()))
// Do not report races in deinit and anything called from it
// because TSan does not observe synchronization between retain
// count dropping to '0' and the object deinitialization.
CurFn->addFnAttr("sanitize_thread_no_checking_at_run_time");
else
CurFn->addFnAttr(llvm::Attribute::SanitizeThread);
}
}

IRGenSILFunction::~IRGenSILFunction() {
assert(Builder.hasPostTerminatorIP() && "did not terminate BB?!");
Expand Down
40 changes: 40 additions & 0 deletions test/Sanitizers/tsan-norace-block-release.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// RUN: %target-swiftc_driver %s -g -sanitize=thread -o %t_tsan-binary
// RUN: env TSAN_OPTIONS=abort_on_error=0:ignore_interceptors_accesses=1 %target-run %t_tsan-binary 2>&1 | %FileCheck %s
// REQUIRES: executable_test
// REQUIRES: objc_interop
// REQUIRES: CPU=x86_64
// REQUIRES: tsan_runtime
// XFAIL: linux

// Test that we do not report a race on block release operation.
import Foundation

public class Sad : NSObject {
private var _source: DispatchSourceTimer?
public override init() {
_source = DispatchSource.makeTimerSource()

// If this line is commented out no data race.
_source?.setEventHandler(handler: globalFuncHandler)

super.init()
_source?.resume()
}
deinit {
_source?.cancel()
}
}

func globalFuncHandler() {
}

func dotest() {
_ = Sad()
}

dotest()
sleep(1)
print("Done.")

// CHECK: Done.
// CHECK-NOT: ThreadSanitizer: data race
52 changes: 52 additions & 0 deletions test/Sanitizers/tsan-norace-deinit-run-time.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// RUN: %target-swiftc_driver %s -g -sanitize=thread -o %t_tsan-binary
// RUN: env TSAN_OPTIONS=abort_on_error=0:ignore_interceptors_accesses=1 %target-run %t_tsan-binary 2>&1 | %FileCheck %s
// REQUIRES: executable_test
// REQUIRES: objc_interop
// REQUIRES: CPU=x86_64
// REQUIRES: tsan_runtime
// XFAIL: linux

// Test that we do not report a race on deinit; the synchronization is guaranteed by runtime.
import Foundation

public class TestDeallocObject : NSObject {
public var v : Int
public override init() {
v = 1
}

@_semantics("optimize.sil.never")
func unoptimize(_ input : Int) -> Int {
return input
}

func accessMember() {
var local : Int = unoptimize(v)
local += 1
}

deinit {
v = 0
}
}

if (true) {
var tdo : TestDeallocObject = TestDeallocObject()
tdo.accessMember()

// Read the value from a different thread.
let concurrentQueue = DispatchQueue(label: "queuename", attributes: .concurrent)
concurrentQueue.async {
tdo.accessMember()
}
// Read the value from this thread.
tdo.accessMember()
sleep(1)

// Deinit the value.
}

print("Done.")

// CHECK: Done.
// CHECK-NOT: ThreadSanitizer: data race

0 comments on commit 18c10c5

Please sign in to comment.