Merge pull request #17100 from devincoughlin/tsan-exclusivity-interaction

Author: swift-ci

include/swift/SIL/InstructionUtils.h

@@ -111,6 +111,10 @@ bool isIncidentalUse(SILInstruction *user);
 /// only used in recognizable patterns without otherwise "escaping".
 bool onlyAffectsRefCount(SILInstruction *user);
 
+/// Return true when the instruction represents added instrumentation for
+/// run-time sanitizers.
+bool isSanitizerInstrumentation(SILInstruction *Instruction);
+
 /// If V is a convert_function or convert_escape_to_noescape return its operand
 /// recursively.
 SILValue stripConvertFunctions(SILValue V);

lib/SIL/InstructionUtils.cpp

@@ -299,6 +299,18 @@ bool swift::onlyAffectsRefCount(SILInstruction *user) {
   }
 }
 
+bool swift::isSanitizerInstrumentation(SILInstruction *Instruction) {
+  auto *BI = dyn_cast<BuiltinInst>(Instruction);
+  if (!BI)
+    return false;
+
+  Identifier Name = BI->getName();
+  if (Name == BI->getModule().getASTContext().getIdentifier("tsanInoutAccess"))
+    return true;
+
+  return false;
+}
+
 SILValue swift::stripConvertFunctions(SILValue V) {
   while (true) {
     if (auto CFI = dyn_cast<ConvertFunctionInst>(V)) {

lib/SILOptimizer/Analysis/AccessSummaryAnalysis.cpp

@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #define DEBUG_TYPE "sil-access-summary-analysis"
+#include "swift/SIL/InstructionUtils.h"
 #include "swift/SIL/SILArgument.h"
 #include "swift/SILOptimizer/Analysis/AccessSummaryAnalysis.h"
 #include "swift/SILOptimizer/Analysis/FunctionOrder.h"
@@ -489,6 +490,12 @@ getSingleAddressProjectionUser(SingleValueInstruction *I) {
     if (isa<BeginAccessInst>(I) && isa<EndAccessInst>(User))
       continue;
 
+    // Ignore sanitizer instrumentation when looking for a single projection
+    // user. This ensures that we're able to find a single projection subpath
+    // even when sanitization is enabled.
+    if (isSanitizerInstrumentation(User))
+      continue;
+
     // We have more than a single user so bail.
     if (SingleUser)
       return std::make_pair(nullptr, 0);

lib/SILOptimizer/Mandatory/DIMemoryUseCollectorOwnership.cpp

@@ -683,21 +683,6 @@ static SILValue getAccessedPointer(SILValue Pointer) {
   return Pointer;
 }
 
-/// Returns true when the instruction represents added instrumentation for
-/// run-time sanitizers.
-static bool isSanitizerInstrumentation(SILInstruction *Instruction,
-                                       ASTContext &Ctx) {
-  auto *BI = dyn_cast<BuiltinInst>(Instruction);
-  if (!BI)
-    return false;
-
-  Identifier Name = BI->getName();
-  if (Name == Ctx.getIdentifier("tsanInoutAccess"))
-    return true;
-
-  return false;
-}
-
 void ElementUseCollector::collectUses(SILValue Pointer, unsigned BaseEltNo) {
   assert(Pointer->getType().isAddress() &&
          "Walked through the pointer to the value?");
@@ -954,7 +939,7 @@ void ElementUseCollector::collectUses(SILValue Pointer, unsigned BaseEltNo) {
 
     // Sanitizer instrumentation is not user visible, so it should not
     // count as a use and must not affect compile-time diagnostics.
-    if (isSanitizerInstrumentation(User, Module.getASTContext()))
+    if (isSanitizerInstrumentation(User))
       continue;
 
     // Otherwise, the use is something complicated, it escapes.

lib/SILOptimizer/Mandatory/PMOMemoryUseCollector.cpp

@@ -13,6 +13,7 @@
 #define DEBUG_TYPE "definite-init"
 #include "PMOMemoryUseCollector.h"
 #include "swift/AST/Expr.h"
+#include "swift/SIL/InstructionUtils.h"
 #include "swift/SIL/SILArgument.h"
 #include "swift/SIL/SILBuilder.h"
 #include "llvm/ADT/StringExtras.h"
@@ -364,21 +365,6 @@ bool ElementUseCollector::collectContainerUses(AllocBoxInst *ABI) {
   return true;
 }
 
-// Returns true when the instruction represents added instrumentation for
-/// run-time sanitizers.
-static bool isSanitizerInstrumentation(SILInstruction *Instruction,
-                                       ASTContext &Ctx) {
-  auto *BI = dyn_cast<BuiltinInst>(Instruction);
-  if (!BI)
-    return false;
-
-  Identifier Name = BI->getName();
-  if (Name == Ctx.getIdentifier("tsanInoutAccess"))
-    return true;
-
-  return false;
-}
-
 bool ElementUseCollector::collectUses(SILValue Pointer, unsigned BaseEltNo) {
   assert(Pointer->getType().isAddress() &&
          "Walked through the pointer to the value?");
@@ -630,7 +616,7 @@ bool ElementUseCollector::collectUses(SILValue Pointer, unsigned BaseEltNo) {
 
     // Sanitizer instrumentation is not user visible, so it should not
     // count as a use and must not affect compile-time diagnostics.
-    if (isSanitizerInstrumentation(User, Module.getASTContext()))
+    if (isSanitizerInstrumentation(User))
       continue;
 
     // Otherwise, the use is something complicated, it escapes.

test/SILOptimizer/access_summary_analysis.sil

@@ -174,6 +174,24 @@ bb0(%0 : @trivial $*StructWithStoredProperties):
   return %7 : $()
 }
 
+// CHECK-LABEL: @accessSeparateStoredPropertiesOfSameCaptureWithTSanInstrumentation
+// CHECK-NEXT: ([.f modify, .g read])
+sil private @accessSeparateStoredPropertiesOfSameCaptureWithTSanInstrumentation : $@convention(thin) (@inout_aliasable StructWithStoredProperties) -> () {
+bb0(%0 : @trivial $*StructWithStoredProperties):
+  %1 = begin_access [modify] [unknown] %0: $*StructWithStoredProperties
+  %2 = builtin "tsanInoutAccess"(%1 : $*StructWithStoredProperties) : $()
+  %3 = struct_element_addr %1 : $*StructWithStoredProperties, #StructWithStoredProperties.f
+  %4 = builtin "tsanInoutAccess"(%3 : $*Int) : $()
+  end_access %1 : $*StructWithStoredProperties
+  %6 = begin_access [read] [unknown] %0: $*StructWithStoredProperties
+  %7 = builtin "tsanInoutAccess"(%6 : $*StructWithStoredProperties) : $()
+  %8 = struct_element_addr %6 : $*StructWithStoredProperties, #StructWithStoredProperties.g
+  %9 = builtin "tsanInoutAccess"(%8 : $*Int) : $()
+  end_access %6 : $*StructWithStoredProperties
+  %11 = tuple ()
+  return %11 : $()
+}
+
 // CHECK-LABEL: @addressToPointerOfStructElementAddr
 // CHECK-NEXT: ([])
 // This mirrors the code pattern for materializeForSet on a struct stored

test/Sanitizers/tsan-static-exclusivity.swift

@@ -0,0 +1,28 @@
+// RUN: %target-swift-frontend -enforce-exclusivity=checked -sanitize=thread -emit-sil -primary-file %s -o /dev/null -verify
+// REQUIRES: tsan_runtime
+
+
+struct OtherStruct {
+  mutating
+  func mutableTakingClosure(_ c: () -> Void) { }
+}
+
+struct StructAndInt {
+    var s = OtherStruct()
+    var f = 42
+}
+
+func testStoredPropertyRelaxationInClosure() {
+    var l = StructAndInt()
+    l.s.mutableTakingClosure {
+        _ = l.f // no-diagnostic
+    }
+}
+
+func takesInouts(_ p1: inout Int, _ p2: inout OtherStruct) { }
+
+func testStoredPropertyRelaxationInInout() {
+    var l = StructAndInt()
+    takesInouts(&l.f, &l.s) // no-diagnostic
+}
+