Skip to content

Commit be60d9b

Browse files
committed
[Exclusivity] Teach separate-stored-property relaxation about TSan instrumentation
The compile-time exclusivity diagnostics explicitly allow conflicting accesses to a struct when it can prove that the two accesses are used to project addresses for separate stored properties. Unfortunately, the logic that detects this special case gets confused by Thread Sanitizer's SIL-level instrumentation. This causes the exclusivity diagnostics to have false positives when TSan is enabled. To fix this, teach the AccessSummaryAnalysis to ignore TSan builtins when determining whether an access has a single projected subpath. rdar://problem/40455335
1 parent c50ca98 commit be60d9b

File tree

3 files changed

+53
-0
lines changed

3 files changed

+53
-0
lines changed

lib/SILOptimizer/Analysis/AccessSummaryAnalysis.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
//===----------------------------------------------------------------------===//
1212

1313
#define DEBUG_TYPE "sil-access-summary-analysis"
14+
#include "swift/SIL/InstructionUtils.h"
1415
#include "swift/SIL/SILArgument.h"
1516
#include "swift/SILOptimizer/Analysis/AccessSummaryAnalysis.h"
1617
#include "swift/SILOptimizer/Analysis/FunctionOrder.h"
@@ -489,6 +490,12 @@ getSingleAddressProjectionUser(SingleValueInstruction *I) {
489490
if (isa<BeginAccessInst>(I) && isa<EndAccessInst>(User))
490491
continue;
491492

493+
// Ignore sanitizer instrumentation when looking for a single projection
494+
// user. This ensures that we're able to find a single projection subpath
495+
// even when sanitization is enabled.
496+
if (isSanitizerInstrumentation(User))
497+
continue;
498+
492499
// We have more than a single user so bail.
493500
if (SingleUser)
494501
return std::make_pair(nullptr, 0);

test/SILOptimizer/access_summary_analysis.sil

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,24 @@ bb0(%0 : @trivial $*StructWithStoredProperties):
174174
return %7 : $()
175175
}
176176

177+
// CHECK-LABEL: @accessSeparateStoredPropertiesOfSameCaptureWithTSanInstrumentation
178+
// CHECK-NEXT: ([.f modify, .g read])
179+
sil private @accessSeparateStoredPropertiesOfSameCaptureWithTSanInstrumentation : $@convention(thin) (@inout_aliasable StructWithStoredProperties) -> () {
180+
bb0(%0 : @trivial $*StructWithStoredProperties):
181+
%1 = begin_access [modify] [unknown] %0: $*StructWithStoredProperties
182+
%2 = builtin "tsanInoutAccess"(%1 : $*StructWithStoredProperties) : $()
183+
%3 = struct_element_addr %1 : $*StructWithStoredProperties, #StructWithStoredProperties.f
184+
%4 = builtin "tsanInoutAccess"(%3 : $*Int) : $()
185+
end_access %1 : $*StructWithStoredProperties
186+
%6 = begin_access [read] [unknown] %0: $*StructWithStoredProperties
187+
%7 = builtin "tsanInoutAccess"(%6 : $*StructWithStoredProperties) : $()
188+
%8 = struct_element_addr %6 : $*StructWithStoredProperties, #StructWithStoredProperties.g
189+
%9 = builtin "tsanInoutAccess"(%8 : $*Int) : $()
190+
end_access %6 : $*StructWithStoredProperties
191+
%11 = tuple ()
192+
return %11 : $()
193+
}
194+
177195
// CHECK-LABEL: @addressToPointerOfStructElementAddr
178196
// CHECK-NEXT: ([])
179197
// This mirrors the code pattern for materializeForSet on a struct stored
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// RUN: %target-swift-frontend -enforce-exclusivity=checked -sanitize=thread -emit-sil -primary-file %s -o /dev/null -verify
2+
// REQUIRES: tsan_runtime
3+
4+
5+
struct OtherStruct {
6+
mutating
7+
func mutableTakingClosure(_ c: () -> Void) { }
8+
}
9+
10+
struct StructAndInt {
11+
var s = OtherStruct()
12+
var f = 42
13+
}
14+
15+
func testStoredPropertyRelaxationInClosure() {
16+
var l = StructAndInt()
17+
l.s.mutableTakingClosure {
18+
_ = l.f // no-diagnostic
19+
}
20+
}
21+
22+
func takesInouts(_ p1: inout Int, _ p2: inout OtherStruct) { }
23+
24+
func testStoredPropertyRelaxationInInout() {
25+
var l = StructAndInt()
26+
takesInouts(&l.f, &l.s) // no-diagnostic
27+
}
28+

0 commit comments

Comments
 (0)