Merge pull request #35754 from devincoughlin/tsan-empty-types-5.4

[5.4] [TSan] Don't instrument inout accesses on known-empty types

Author: devincoughlin

lib/IRGen/GenBuiltin.cpp

@@ -120,6 +120,7 @@ getLoweredTypeAndTypeInfo(IRGenModule &IGM, Type unloweredType) {
 /// emitBuiltinCall - Emit a call to a builtin function.
 void irgen::emitBuiltinCall(IRGenFunction &IGF, const BuiltinInfo &Builtin,
                             Identifier FnId, SILType resultType,
+                            ArrayRef<SILType> argTypes,
                             Explosion &args, Explosion &out,
                             SubstitutionMap substitutions) {
   if (Builtin.ID == BuiltinValueKind::COWBufferForReading) {
@@ -1074,7 +1075,18 @@ if (Builtin.ID == BuiltinValueKind::id) { \
 
   if (Builtin.ID == BuiltinValueKind::TSanInoutAccess) {
     auto address = args.claimNext();
-    IGF.emitTSanInoutAccessCall(address);
+
+    // The tsanInoutAccess builtin takes a single argument, the address
+    // of the accessed storage
+    SILType accessedType = argTypes[0];
+
+    // Empty types (such as structs without stored properties) have a
+    // meaningless value for their address. We not should call into the
+    // TSan runtime to check for data races on accesses on such addresses.
+    if (!IGF.IGM.getTypeInfo(accessedType)
+        .isKnownEmpty(ResilienceExpansion::Maximal)) {
+      IGF.emitTSanInoutAccessCall(address);
+    }
     return;
   }
 

lib/IRGen/GenBuiltin.h

@@ -33,6 +33,7 @@ namespace irgen {
   /// Emit a call to a builtin function.
   void emitBuiltinCall(IRGenFunction &IGF, const BuiltinInfo &builtin,
                        Identifier fnId, SILType resultType,
+                       ArrayRef<SILType> argTypes,
                        Explosion &args, Explosion &result,
                        SubstitutionMap substitutions);
 

lib/IRGen/IRGenSIL.cpp

@@ -2680,18 +2680,21 @@ void IRGenSILFunction::visitBuiltinInst(swift::BuiltinInst *i) {
 
   auto argValues = i->getArguments();
   Explosion args;
+  SmallVector<SILType, 4> argTypes;
 
   for (auto idx : indices(argValues)) {
     auto argValue = argValues[idx];
 
     // Builtin arguments should never be substituted, so use the value's type
     // as the parameter type.
     emitApplyArgument(*this, argValue, argValue->getType(), args);
+
+    argTypes.push_back(argValue->getType());
   }
 
   Explosion result;
   emitBuiltinCall(*this, builtin, i->getName(), i->getType(),
-                  args, result, i->getSubstitutions());
+                  argTypes, args, result, i->getSubstitutions());
 
   setLoweredExplosion(i, result);
 }

test/SILGen/tsan_instrumentation.swift

@@ -1,5 +1,6 @@
 // REQUIRES: tsan_runtime
 // RUN: %target-swift-emit-silgen -sanitize=thread %s | %FileCheck %s
+// RUN: %target-swift-frontend -sanitize=thread -emit-ir -primary-file %s | %FileCheck --check-prefix=CHECK-LLVM-IR %s
 
 // TSan is only supported on 64 bit.
 // REQUIRES: PTRSIZE=64
@@ -63,3 +64,53 @@ func inoutGlobalClassStoredProperty() {
   // temporary buffer passed to takesInout().
   takesInout(&gClass.storedProperty)
 }
+
+// Known-empty types don't have storage, so there is no address
+// to pass to the TSan runtime to check for data races on inout accesses.
+// In this case, the instrumentation should skip the call to
+// __tsan_external_write()
+
+struct ZeroSizedStruct {
+  mutating
+  func mutate() { }
+}
+
+struct NonEmptyStruct {
+  var f: Int = 5
+  mutating
+  func mutate() { }
+}
+
+// CHECK-LLVM-IR-LABEL: testNoInstrumentZeroSizedStruct
+func testNoInstrumentZeroSizedStruct() {
+  var s = ZeroSizedStruct()
+
+// CHECK-LLVM-IR-NOT: tsan_external_write
+  s.mutate()
+}
+
+func takesInout<T>(_ p: inout T) { }
+
+// CHECK-LLVM-IR-LABEL: testNoInstrumentEmptyTuple
+func testNoInstrumentEmptyTuple() {
+  var t: Void = ()
+
+// CHECK-LLVM-IR-NOT: tsan_external_write
+  takesInout(&t)
+}
+
+// CHECK-LLVM-IR-LABEL: testNoInstrumentMutateInoutZeroSizedStruct
+func testNoInstrumentMutateInoutZeroSizedStruct(p: inout ZeroSizedStruct) {
+
+// CHECK-LLVM-IR-NOT: tsan_external_write
+  p.mutate()
+}
+
+// CHECK-LLVM-IR-LABEL: testInstrumentNonEmptyStruct
+func testInstrumentNonEmptyStruct() {
+  // Make sure we actually instrument accesses to non-empty structs.
+  var s = NonEmptyStruct()
+
+// CHECK-LLVM-IR: tsan_external_write
+  s.mutate()
+}