Let's debug: missing RBI data race diagnostics

the following is a compilation of notes and thoughts from researching code that should produce RBI diagnostics, but is not currently doing so. it is somewhat long and meandering, but can hopefully serve as documentation for any future efforts to investigate similar problems. i don't claim to have a great mental model of everything going on here, so do feel free to correct any mistakes i've made!


Context

i've been looking into a class of issues with region based isolation (RBI) diagnostics that seem to admit code with data races. here's an example that is fairly representative of the problem:

@MainActor // *
func mutableLocalCaptureDataRace() {
  var x = 0

  Task.detached { x = 1 }

  x = 2
}
// * note: the @MainActor annotation may not actually be necessary to
// reproduce this problem, but it was present during my investigation

this currently compiles without a concurrency error or warning in the 6.0 (and 6.1) compilers, under both 5 & 6 language modes.

using the RBI logging[1], we can inspect the output of the analysis to get a better sense of what's going on. here are the 'interesting' bits of the output for the mutableLocalCaptureDataRace function (note: this excludes the Task closure as it is analyzed independently).

per my current understanding of the relevant code, the initial step of the analysis processes the SILGen instructions to determine the 'trackable' values required for the dataflow phase. additionally, each instruction is translated from SIL into zero or more 'partition operations', depending on the specific instruction and translation semantics. the 'trackable state' collected during this phase also records various pieces of information about elements, like whether it represents a non-sendable value, the isolation region it's in, etc.

here's a portion of the pre-dataflow phase log output where the partial_apply instruction for the Task closure is handled:

// SIL:
  // function_ref closure #1 in mutableLocalCaptureDataRace()
  %11 = function_ref @$s6output27mutableLocalCaptureDataRaceyyFyyYacfU_ : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %14
  %12 = copy_value %1 : ${ var Int }              // user: %14
  %13 = enum $Optional<any Actor>, #Optional.none!enumelt // user: %14
  %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16

// RBI:
Visiting:   // function_ref closure #1 in mutableLocalCaptureDataRace()
  %11 = function_ref @$s6output27mutableLocalCaptureDataRaceyyFyyYacfU_ : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %14
    Semantics: assign_fresh
Visiting:   %12 = copy_value %1 : ${ var Int }              // user: %14
    Semantics: look_through
Visiting:   %13 = enum $Optional<any Actor>, #Optional.none!enumelt // user: %14
    Semantics: assign_fresh
Visiting:   %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
    Semantics: special
 โ”Œโ”€โ”ฌโ”€โ•ผ  %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
 โ”‚ โ””โ”€โ•ผ  line:5:17
 โ”œโ”€โ”€โ”€โ”€โ”€โ•ผ require %%1:   %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
 โ”‚    โ””โ•ผ assign %%10 = %%1:   %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
 โ””โ”€โ”€โ”€โ”€โ”€โ•ผ Used Values
          โ””โ•ผ State: %%1. TrackableValueState[id: 1][is_no_alias: no][is_sendable: no][region_value_kind: disconnected].
             Rep Value:   %1 = begin_borrow [var_decl] %0 : ${ var Int }  // users: %27, %12, %2
             Type: ${ var Int }
          โ””โ•ผ State: %%10. TrackableValueState[id: 10][is_no_alias: no][is_sendable: no][region_value_kind: disconnected].
             Rep Value:   %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
             Type: $@isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <()>

the logs indicate that handling the isolated partial_apply has 'special' translation semantics, and in this case the instruction is translated into both the 'require' and 'assign' partition operations. the 'require' is for the tracked value with id %%1, which in this instance is the mutable local variable x. IIUC, a 'require' here means that during the dataflow, the tracked value's region will be required to not have been marked as 'sent' when this partition operation is encountered. if that's not the case, a diagnostic should (generally) be produced.

then we have the output from the call to Task.detached which looks like:

//SIL:
  // function_ref static Task<>.detached(priority:operation:)
  %15 = function_ref @$sScTss5NeverORs_rlE8detached8priority9operationScTyxABGScPSg_xyYaYAcntFZ : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %16
  %16 = apply %15<(), Never>(%9, %14, %8) : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %19

// RBI:
Visiting:   // function_ref static Task<>.detached(priority:operation:)
  %15 = function_ref @$sScTss5NeverORs_rlE8detached8priority9operationScTyxABGScPSg_xyYaYAcntFZ : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %16
    Semantics: assign_fresh
Visiting:   %16 = apply %15<(), Never>(%9, %14, %8) : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %19
    Semantics: apply
 โ”Œโ”€โ”ฌโ”€โ•ผ  %16 = apply %15<(), Never>(%9, %14, %8) : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %19
 โ”‚ โ””โ”€โ•ผ  line:5:8
 โ”œโ”€โ”€โ”€โ”€โ”€โ•ผ require %%10:   %16 = apply %15<(), Never>(%9, %14, %8) : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %19
 โ”‚    โ””โ•ผ send %%10:   %16 = apply %15<(), Never>(%9, %14, %8) : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %19
 โ””โ”€โ”€โ”€โ”€โ”€โ•ผ Used Values
          โ””โ•ผ State: %%10. TrackableValueState[id: 10][is_no_alias: no][is_sendable: no][region_value_kind: disconnected].
             Rep Value:   %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
             Type: $@isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <()>

we can see that in the translation of the apply instruction, both a 'require' and 'send' partition operation are created. the 'send' will mark all trackable state values in the same region as %%10 to become marked as 'sent', and so subsequent 'require' operations on any of them should typically produce diagnostics.

now if we focus on the logging around the subsequent assignment x = 2, we have from the pre-dataflow logs:

// SIL:
  %24 = begin_access [modify] [dynamic] %2 : $*Int // users: %25, %26
  store %23 to [trivial] %24 : $*Int              // id: %25
  end_access %24 : $*Int                          // id: %26

// RBI:
Visiting:   %24 = begin_access [modify] [dynamic] %2 : $*Int // users: %25, %26
    Semantics: look_through
Visiting:   store %23 to [trivial] %24 : $*Int              // id: %25
    Semantics: store
Visiting:   end_access %24 : $*Int                          // id: %26
    Semantics: ignored

the logging indicates that the SIL store instruction has 'store' semantics, and if we look at the logic for this translation, it appears that a store to a non-sendable destination should be translated into either an assign or merge partition operation, both of which should eventually add some additional 'require' partition operations if trackable state is involved. i believe this means that in the dataflow portion of the analysis, we would expect to see a 'require' operation processed after the earlier 'send' produced for the Task.detached instructions.

however, if we inspect the output of the dataflow logs, they appear to end with the 'send' operation associated with the Task.detached call:

// Relevant trackable state:
%%0: TrackableValue. State: TrackableValueState[id: 0][is_no_alias: no][is_sendable: no][region_value_kind: disconnected].
    Rep Value:   %0 = alloc_box ${ var Int }, var, name "x"      // users: %28, %1
%%1: TrackableValue. State: TrackableValueState[id: 1][is_no_alias: no][is_sendable: no][region_value_kind: disconnected].
    Rep Value:   %1 = begin_borrow [var_decl] %0 : ${ var Int }  // users: %27, %12, %2
%%10: TrackableValue. State: TrackableValueState[id: 10][is_no_alias: no][is_sendable: no][region_value_kind: disconnected].
    Rep Value:   %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16

// Dataflow:
Block: bb0
    Visiting Preds!
Applying: assign_fresh %%0:   %0 = alloc_box ${ var Int }, var, name "x"      // users: %28, %1
    Before: []
    After:  [(0)]
Applying: require %%0:   %1 = begin_borrow [var_decl] %0 : ${ var Int }  // users: %27, %12, %2
    Before: [(0)]
    After:  [(0)]
Applying: assign %%1 = %%0:   %1 = begin_borrow [var_decl] %0 : ${ var Int }  // users: %27, %12, %2
    Before: [(0)]
    After:  [(0 1)]
Applying: require %%1:   %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
    Before: [(0 1)]
    After:  [(0 1)]
Applying: assign %%10 = %%1:   %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
    Before: [(0 1)]
    After:  [(0 1 10)]
Applying: require %%10:   %16 = apply %15<(), Never>(%9, %14, %8) : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %19
    Before: [(0 1 10)]
    After:  [(0 1 10)]
Applying: send %%10:   %16 = apply %15<(), Never>(%9, %14, %8) : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %19
    Before: [(0 1 10)]
    After:  [{0 1 10}]
    Working Partition: [{0 1 10}]
    Exit Partition: [{0 1 10}]
    Updated Partition: yes
// โ‰๏ธ where's the `require` for the write to `x`?

Potential explanation

after digging into the implementation[2], this is what i think is going on:

  1. during the initial pass through the SIL instructions to collect 'trackable state', when we reach the begin_access instruction for the second write to x:
    %24 = begin_access [modify] [dynamic] %2 : $*Int // users: %25, %26
    
    we enter the method for translating instructions with 'lookthrough' semantics.
  2. the 'destination' value, which in this case is the begin_access instruction, is attempted to be tracked.
  3. after a few method hops, this logic ends up in getUnderlyingTrackedValueHelper
  4. it fails the test to check for non-sendability here (first goes here, and then here). this seems to be because the type of the destination value resolves to Int, which is sendable.
  5. since the value appears to be sendable, a new 'underlying tracked value' is created and returned. this eventually shows up in the log output as:
    %%18: TrackableValue. State: TrackableValueState[id: 18][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   %24 = begin_access [modify] [dynamic] %2 : $*Int // users: %25, %26
    
    which indicates that the is_sendable flag is true.
  6. now, when we get to the translation logic for the store instruction, we check if the store's destination is trackable state (destination in this case is the begin_access inst), and we fail because the state is marked as is_sendable.
  7. the store is thus ignored, and produces no partition operations that would validate x isn't used after being sent

Possible remediation

i think it's odd that the result of translating an instruction with 'look through' semantics would produce a new underlying trackable value. it would seem to me that a begin_access should generally track whatever backs the accessed value. there is logic to this effect for handling ProjectBoxInst, for example.

adding an additional condition to the underlying value helper early exit check that requires the instruction not be a look through instruction, still does not produce a diagnostic. however, this change does enable the translateSILStore() logic to end up in translateSILMerge, which seems like an improvement.

it seems that in the merge translation logic, we currently only add a partition 'require' for the destination value if both it and at least one of its operands are 'trackable'. if we change the code to always add a 'require' for the destination value as long as it's trackable, then we get a diagnostic to the effect of:

18 |   var x = 0
19 | 
20 |   Task.detached { x = 1 }
   |        |- error: sending value of non-Sendable type '() async -> ()' risks causing data races
   |        `- note: Passing value of non-Sendable type '() async -> ()' as a 'sending' argument to static method 'detached(priority:operation:)' risks causing races in between local and caller code
21 | 
22 |   x = 2
   |     `- note: access can happen concurrently
23 | }
24 | 

i have not yet attempted drafting a PR or looked too deeply into what might break with such a change, but i'm curious if a fix along the lines proposed seems reasonable to those more familiar with the code, or any interested parties (cc @Michael_Gottesman, @hborla). i will also note that @Nickolas_Pohilets recently put together a draft PR to address a similar issue, which takes a slightly different approach to addressing the problem (changing the sendability check logic vs altering 'look through' tracking semantics).

References

  1. example code in godbolt. note that at the time of writing, it is using a compiler from the current 6.1 dev branch so that the RBI logs will show up.

  1. using the flags -Xllvm -sil-regionbasedisolation-log=verbose in a compiler built with assertions enabled โ†ฉ๏ธŽ

  2. read: adding copious print statements into the compiler โ†ฉ๏ธŽ

12 Likes

and, as i hit the character limit in the original post by including these, here are excerpts of the RBI logs for the relevant function:

๐Ÿชต expand for pre-dataflow logs ๐Ÿชต
โ•พโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ผ
Performing pre-dataflow scan to gather flow insensitive information $s6output27mutableLocalCaptureDataRaceyyF:
โ•พโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ผ
PartialApplyReachability::add.
Value:   %1 = begin_borrow [var_decl] %0 : ${ var Int }  // users: %27, %12, %2
User:   %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
Propagating Captures Result!
(BitNo, Value):
    0:   %1 = begin_borrow [var_decl] %0 : ${ var Int }  // users: %27, %12, %2
(Block,GenBits):
    bb0.
        Entry: 0[]
        Gen: 1[0]
        Exit: 1[0]
Initializing Function Args:
    None.
โ•พโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ผ
Compiling basic block for function $s6output27mutableLocalCaptureDataRaceyyF: bb0
โ•พโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ผ
sil_scope 1 { loc "<source>":2:6 parent @$s6output27mutableLocalCaptureDataRaceyyF : $@convention(thin) () -> () }
sil_scope 2 { loc "<source>":3:7 parent 1 }
sil_scope 3 { loc "<source>":3:11 parent 1 }
bb0:
  %0 = alloc_box ${ var Int }, var, name "x"      // users: %28, %1
  %1 = begin_borrow [var_decl] %0 : ${ var Int }  // users: %27, %12, %2
  %2 = project_box %1 : ${ var Int }, 0           // users: %24, %7
  %3 = integer_literal $Builtin.IntLiteral, 0     // user: %6
  %4 = metatype $@thin Int.Type                   // user: %6
  // function_ref Int.init(_builtinIntegerLiteral:)
  %5 = function_ref @$sSi22_builtinIntegerLiteralSiBI_tcfC : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int // user: %6
  %6 = apply %5(%3, %4) : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int // user: %7
  store %6 to [trivial] %2 : $*Int                // id: %7
  %8 = metatype $@thin Task<(), Never>.Type       // user: %16
  %9 = alloc_stack $Optional<TaskPriority>        // users: %18, %17, %16, %10
  inject_enum_addr %9 : $*Optional<TaskPriority>, #Optional.none!enumelt // id: %10
  // function_ref closure #1 in mutableLocalCaptureDataRace()
  %11 = function_ref @$s6output27mutableLocalCaptureDataRaceyyFyyYacfU_ : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %14
  %12 = copy_value %1 : ${ var Int }              // user: %14
  %13 = enum $Optional<any Actor>, #Optional.none!enumelt // user: %14
  %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
  // function_ref static Task<>.detached(priority:operation:)
  %15 = function_ref @$sScTss5NeverORs_rlE8detached8priority9operationScTyxABGScPSg_xyYaYAcntFZ : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %16
  %16 = apply %15<(), Never>(%9, %14, %8) : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %19
  destroy_addr %9 : $*Optional<TaskPriority>      // id: %17
  dealloc_stack %9 : $*Optional<TaskPriority>     // id: %18
  destroy_value %16 : $Task<(), Never>            // id: %19
  %20 = integer_literal $Builtin.IntLiteral, 2    // user: %23
  %21 = metatype $@thin Int.Type                  // user: %23
  // function_ref Int.init(_builtinIntegerLiteral:)
  %22 = function_ref @$sSi22_builtinIntegerLiteralSiBI_tcfC : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int // user: %23
  %23 = apply %22(%20, %21) : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int // user: %25
  %24 = begin_access [modify] [dynamic] %2 : $*Int // users: %25, %26
  store %23 to [trivial] %24 : $*Int              // id: %25
  end_access %24 : $*Int                          // id: %26
  end_borrow %1 : ${ var Int }                    // id: %27
  destroy_value %0 : ${ var Int }                 // id: %28
  %29 = tuple ()                                  // user: %30
  return %29 : $()                                // id: %30
โ•พโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ผ
Results:
Visiting:   %0 = alloc_box ${ var Int }, var, name "x"      // users: %28, %1
    Semantics: assign_fresh
 โ”Œโ”€โ”ฌโ”€โ•ผ  %0 = alloc_box ${ var Int }, var, name "x"      // users: %28, %1
 โ”‚ โ””โ”€โ•ผ  line:3:7
 โ”œโ”€โ”€โ”€โ”€โ”€โ•ผ assign_fresh %%0:   %0 = alloc_box ${ var Int }, var, name "x"      // users: %28, %1
 โ””โ”€โ”€โ”€โ”€โ”€โ•ผ Used Values
          โ””โ•ผ State: %%0. TrackableValueState[id: 0][is_no_alias: no][is_sendable: no][region_value_kind: disconnected].
             Rep Value:   %0 = alloc_box ${ var Int }, var, name "x"      // users: %28, %1
             Type: ${ var Int }
Visiting:   %1 = begin_borrow [var_decl] %0 : ${ var Int }  // users: %27, %12, %2
    Semantics: assign
 โ”Œโ”€โ”ฌโ”€โ•ผ  %1 = begin_borrow [var_decl] %0 : ${ var Int }  // users: %27, %12, %2
 โ”‚ โ””โ”€โ•ผ  line:3:7
 โ”œโ”€โ”€โ”€โ”€โ”€โ•ผ require %%0:   %1 = begin_borrow [var_decl] %0 : ${ var Int }  // users: %27, %12, %2
 โ”‚    โ””โ•ผ assign %%1 = %%0:   %1 = begin_borrow [var_decl] %0 : ${ var Int }  // users: %27, %12, %2
 โ””โ”€โ”€โ”€โ”€โ”€โ•ผ Used Values
          โ””โ•ผ State: %%0. TrackableValueState[id: 0][is_no_alias: no][is_sendable: no][region_value_kind: disconnected].
             Rep Value:   %0 = alloc_box ${ var Int }, var, name "x"      // users: %28, %1
             Type: ${ var Int }
          โ””โ•ผ State: %%1. TrackableValueState[id: 1][is_no_alias: no][is_sendable: no][region_value_kind: disconnected].
             Rep Value:   %1 = begin_borrow [var_decl] %0 : ${ var Int }  // users: %27, %12, %2
             Type: ${ var Int }
Visiting:   %2 = project_box %1 : ${ var Int }, 0           // users: %24, %7
    Semantics: look_through
Visiting:   %3 = integer_literal $Builtin.IntLiteral, 0     // user: %6
    Semantics: assign_fresh
Visiting:   %4 = metatype $@thin Int.Type                   // user: %6
    Semantics: ignored
Visiting:   // function_ref Int.init(_builtinIntegerLiteral:)
  %5 = function_ref @$sSi22_builtinIntegerLiteralSiBI_tcfC : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int // user: %6
    Semantics: assign_fresh
Visiting:   %6 = apply %5(%3, %4) : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int // user: %7
    Semantics: apply
Visiting:   store %6 to [trivial] %2 : $*Int                // id: %7
    Semantics: store
Visiting:   %8 = metatype $@thin Task<(), Never>.Type       // user: %16
    Semantics: ignored
Visiting:   %9 = alloc_stack $Optional<TaskPriority>        // users: %18, %17, %16, %10
    Semantics: assign_fresh
Visiting:   inject_enum_addr %9 : $*Optional<TaskPriority>, #Optional.none!enumelt // id: %10
    Semantics: ignored
Visiting:   // function_ref closure #1 in mutableLocalCaptureDataRace()
  %11 = function_ref @$s6output27mutableLocalCaptureDataRaceyyFyyYacfU_ : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %14
    Semantics: assign_fresh
Visiting:   %12 = copy_value %1 : ${ var Int }              // user: %14
    Semantics: look_through
Visiting:   %13 = enum $Optional<any Actor>, #Optional.none!enumelt // user: %14
    Semantics: assign_fresh
Visiting:   %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
    Semantics: special
 โ”Œโ”€โ”ฌโ”€โ•ผ  %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
 โ”‚ โ””โ”€โ•ผ  line:5:17
 โ”œโ”€โ”€โ”€โ”€โ”€โ•ผ require %%1:   %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
 โ”‚    โ””โ•ผ assign %%10 = %%1:   %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
 โ””โ”€โ”€โ”€โ”€โ”€โ•ผ Used Values
          โ””โ•ผ State: %%1. TrackableValueState[id: 1][is_no_alias: no][is_sendable: no][region_value_kind: disconnected].
             Rep Value:   %1 = begin_borrow [var_decl] %0 : ${ var Int }  // users: %27, %12, %2
             Type: ${ var Int }
          โ””โ•ผ State: %%10. TrackableValueState[id: 10][is_no_alias: no][is_sendable: no][region_value_kind: disconnected].
             Rep Value:   %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
             Type: $@isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <()>
Visiting:   // function_ref static Task<>.detached(priority:operation:)
  %15 = function_ref @$sScTss5NeverORs_rlE8detached8priority9operationScTyxABGScPSg_xyYaYAcntFZ : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %16
    Semantics: assign_fresh
Visiting:   %16 = apply %15<(), Never>(%9, %14, %8) : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %19
    Semantics: apply
 โ”Œโ”€โ”ฌโ”€โ•ผ  %16 = apply %15<(), Never>(%9, %14, %8) : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %19
 โ”‚ โ””โ”€โ•ผ  line:5:8
 โ”œโ”€โ”€โ”€โ”€โ”€โ•ผ require %%10:   %16 = apply %15<(), Never>(%9, %14, %8) : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %19
 โ”‚    โ””โ•ผ send %%10:   %16 = apply %15<(), Never>(%9, %14, %8) : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %19
 โ””โ”€โ”€โ”€โ”€โ”€โ•ผ Used Values
          โ””โ•ผ State: %%10. TrackableValueState[id: 10][is_no_alias: no][is_sendable: no][region_value_kind: disconnected].
             Rep Value:   %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
             Type: $@isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <()>
Visiting:   destroy_addr %9 : $*Optional<TaskPriority>      // id: %17
    Semantics: ignored
Visiting:   dealloc_stack %9 : $*Optional<TaskPriority>     // id: %18
    Semantics: ignored
Visiting:   destroy_value %16 : $Task<(), Never>            // id: %19
    Semantics: ignored
Visiting:   %20 = integer_literal $Builtin.IntLiteral, 2    // user: %23
    Semantics: assign_fresh
Visiting:   %21 = metatype $@thin Int.Type                  // user: %23
    Semantics: ignored
Visiting:   // function_ref Int.init(_builtinIntegerLiteral:)
  %22 = function_ref @$sSi22_builtinIntegerLiteralSiBI_tcfC : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int // user: %23
    Semantics: assign_fresh
Visiting:   %23 = apply %22(%20, %21) : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int // user: %25
    Semantics: apply
Visiting:   %24 = begin_access [modify] [dynamic] %2 : $*Int // users: %25, %26
    Semantics: look_through
Visiting:   store %23 to [trivial] %24 : $*Int              // id: %25
    Semantics: store
Visiting:   end_access %24 : $*Int                          // id: %26
    Semantics: ignored
Visiting:   end_borrow %1 : ${ var Int }                    // id: %27
    Semantics: ignored
Visiting:   destroy_value %0 : ${ var Int }                 // id: %28
    Semantics: ignored
Visiting:   %29 = tuple ()                                  // user: %30
    Semantics: assign
Visiting:   return %29 : $()                                // id: %30
    Semantics: require
๐Ÿชต expand for dataflow logs ๐Ÿชต
โ•พโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ผ
Performing Dataflow!
โ•พโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ผ
Values!
%%0: TrackableValue. State: TrackableValueState[id: 0][is_no_alias: no][is_sendable: no][region_value_kind: disconnected].
    Rep Value:   %0 = alloc_box ${ var Int }, var, name "x"      // users: %28, %1
%%1: TrackableValue. State: TrackableValueState[id: 1][is_no_alias: no][is_sendable: no][region_value_kind: disconnected].
    Rep Value:   %1 = begin_borrow [var_decl] %0 : ${ var Int }  // users: %27, %12, %2
%%2: TrackableValue. State: TrackableValueState[id: 2][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   %2 = project_box %1 : ${ var Int }, 0           // users: %24, %7
%%3: TrackableValue. State: TrackableValueState[id: 3][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   %3 = integer_literal $Builtin.IntLiteral, 0     // user: %6
%%4: TrackableValue. State: TrackableValueState[id: 4][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   // function_ref Int.init(_builtinIntegerLiteral:)
  %5 = function_ref @$sSi22_builtinIntegerLiteralSiBI_tcfC : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int // user: %6
%%5: TrackableValue. State: TrackableValueState[id: 5][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   %4 = metatype $@thin Int.Type                   // user: %6
%%6: TrackableValue. State: TrackableValueState[id: 6][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   %6 = apply %5(%3, %4) : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int // user: %7
%%7: TrackableValue. State: TrackableValueState[id: 7][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   %9 = alloc_stack $Optional<TaskPriority>        // users: %18, %17, %16, %10
%%8: TrackableValue. State: TrackableValueState[id: 8][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   // function_ref closure #1 in mutableLocalCaptureDataRace()
  %11 = function_ref @$s6output27mutableLocalCaptureDataRaceyyFyyYacfU_ : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %14
%%9: TrackableValue. State: TrackableValueState[id: 9][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   %13 = enum $Optional<any Actor>, #Optional.none!enumelt // user: %14
%%10: TrackableValue. State: TrackableValueState[id: 10][is_no_alias: no][is_sendable: no][region_value_kind: disconnected].
    Rep Value:   %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
%%11: TrackableValue. State: TrackableValueState[id: 11][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   // function_ref static Task<>.detached(priority:operation:)
  %15 = function_ref @$sScTss5NeverORs_rlE8detached8priority9operationScTyxABGScPSg_xyYaYAcntFZ : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %16
%%12: TrackableValue. State: TrackableValueState[id: 12][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   %8 = metatype $@thin Task<(), Never>.Type       // user: %16
%%13: TrackableValue. State: TrackableValueState[id: 13][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   %16 = apply %15<(), Never>(%9, %14, %8) : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %19
%%14: TrackableValue. State: TrackableValueState[id: 14][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   %20 = integer_literal $Builtin.IntLiteral, 2    // user: %23
%%15: TrackableValue. State: TrackableValueState[id: 15][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   // function_ref Int.init(_builtinIntegerLiteral:)
  %22 = function_ref @$sSi22_builtinIntegerLiteralSiBI_tcfC : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int // user: %23
%%16: TrackableValue. State: TrackableValueState[id: 16][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   %21 = metatype $@thin Int.Type                  // user: %23
%%17: TrackableValue. State: TrackableValueState[id: 17][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   %23 = apply %22(%20, %21) : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int // user: %25
%%18: TrackableValue. State: TrackableValueState[id: 18][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   %24 = begin_access [modify] [dynamic] %2 : $*Int // users: %25, %26
%%19: TrackableValue. State: TrackableValueState[id: 19][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   %29 = tuple ()                                  // user: %30
Block: bb0
    Visiting Preds!
Applying: assign_fresh %%0:   %0 = alloc_box ${ var Int }, var, name "x"      // users: %28, %1
    Before: []
    After:  [(0)]
Applying: require %%0:   %1 = begin_borrow [var_decl] %0 : ${ var Int }  // users: %27, %12, %2
    Before: [(0)]
    After:  [(0)]
Applying: assign %%1 = %%0:   %1 = begin_borrow [var_decl] %0 : ${ var Int }  // users: %27, %12, %2
    Before: [(0)]
    After:  [(0 1)]
Applying: require %%1:   %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
    Before: [(0 1)]
    After:  [(0 1)]
Applying: assign %%10 = %%1:   %14 = partial_apply [callee_guaranteed] [isolated_any] %11(%13, %12) : $@convention(thin) @async @substituted <ฯ„_0_0> (@guaranteed Optional<any Actor>, @guaranteed { var Int }) -> @out ฯ„_0_0 for <()> // user: %16
    Before: [(0 1)]
    After:  [(0 1 10)]
Applying: require %%10:   %16 = apply %15<(), Never>(%9, %14, %8) : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %19
    Before: [(0 1 10)]
    After:  [(0 1 10)]
Applying: send %%10:   %16 = apply %15<(), Never>(%9, %14, %8) : $@convention(method) <ฯ„_0_0, ฯ„_0_1 where ฯ„_0_0 : Sendable, ฯ„_0_1 == Never> (@in_guaranteed Optional<TaskPriority>, @sil_sending @owned @isolated(any) @async @callee_guaranteed @substituted <ฯ„_0_0> () -> @out ฯ„_0_0 for <ฯ„_0_0>, @thin Task<ฯ„_0_0, Never>.Type) -> @owned Task<ฯ„_0_0, Never> // user: %19
    Before: [(0 1 10)]
    After:  [{0 1 10}]
    Working Partition: [{0 1 10}]
    Exit Partition: [{0 1 10}]
    Updated Partition: yes

I was trying to fix exactly the same issue. Idea behind my fix attempt was to treat pointer values as first-class, but I'm not familiar with the sufficiently to judge if that is the right approach. It is more of an experiment. You might be right, maybe we need instead to treat begin_borrow the same way project_box is handled in getUnderlyingTrackedValueHelper().

But regardless of that, change in translateSILMerge() seems to be needed as well.

1 Like

after looking into it more, i've begun to suspect that even the handling of project_box may not be quite right (at least, per my current understanding). in addition to the change you made to translateSILMerge(), i tried adding an additional clause to the sendable value early exit in getUnderlyingTrackedValueHelper() basically like:

auto *di = value->getDefiningInstruction();
if ((di && !isStaticallyLookThroughInst(di)) &&
    !SILIsolationInfo::isNonSendableType(value))
  return UnderlyingTrackedValueInfo(value);

however, that then caused the transfernonsendable.swift tests to fail, specifically the one that checks that it's okay to send a tuple with a mix of non-sendable and sendable fields as long as the non-sendable elements aren't used after send. so just skipping that sendable type check for 'look through' instructions seemingly isn't sufficient, or would require additional changes elsewhere.

so, going back to the original example, i noticed that the debug logs had this line:

%%2: TrackableValue. State: TrackableValueState[id: 2][is_no_alias: no][is_sendable: yes][region_value_kind: disconnected].
    Rep Value:   %2 = project_box %1 : ${ var Int }, 0           // users: %24, %7

upon reflection, i found this confusing because project_box is supposed to be 'look through', so why would it show up as a 'representative value' of some trackable state? shouldn't the representative be the thing being projected (the alloc_box in this case)?

my current theory is that we perhaps need to do a more thorough inspection of the use-def chain for the look through instructions that are dealing with addresses. in this WIP PR i've hoisted the use-def walking logic to occur before the sendability test in such cases, and that does appear to resolve the issue without regressing the existing tests (though still hasn't passed CI for seemingly unrelated reasons). i'm still not sure if such a change really makes sense though...

yeah i wonder if this change should perhaps be made regardless of whether it fixes any currently-known bugs...

So I took a little look at this. There is an issue in translateMerge where we are not requiring the destination operand if all of the src are Sendable. There is also a further issue around how we are currently handling Sendable addresses from non-Sendable boxes. I am right now thinking about the problem. The issue is more complex than the WIP PR. Give me a little bit to detangle it.

3 Likes

@Michael_Gottesman, any updates on this?

I want to submit Proposal to unban weak let bindings by nickolas-pohilets ยท Pull Request #2771 ยท swiftlang/swift-evolution ยท GitHub for review, but this bug would cause a lot of confusion during review process. So I think it is better to fix it first.

This is the fix for it: [rbi] Teach RBI how to handle non-Sendable bases of Sendable values by gottesmm ยท Pull Request #80745 ยท swiftlang/swift ยท GitHub. Like I mentioned above, it required changing a bit of the "machine".

2 Likes

I believe this issue is related to OP's case: A simple concurrency safety breach of a var being captured by reference ยท Issue #76929 ยท swiftlang/swift ยท GitHub. It's a long standing regression.

BTW, thanks for sharing your compilation, it's very informative.

1 Like

thanks! i tested on the main branch since the PR was already merged, and while it does appear the changes address some of the issues (yay!), code that is structurally nearly identical (at least from a source perspective) but with slight variations on the isolation crossings seems to still fail to produce the expected diagnostics:

// this is now diagnosed:

@MainActor
func mutableLocalCaptureDataRace() {
  var x = 0

  Task.detached { x = 1 }
       |- error: sending value of non-Sendable type '() async -> ()' risks causing data races [#SendingRisksDataRace]
       `- note: Passing value of non-Sendable type '() async -> ()' as a 'sending' argument to static method 'detached(priority:operation:)' risks causing races in between local and cal

  x = 2
    `- note: access can happen concurrently
}

// but this is not:

nonisolated func mutableLocalCaptureDataRace2() {
  var x = 0

  Task { @MainActor in x = 1 } // or could use `Task.detached` here

  x = 2
}

// and neither is this:

@globalActor
actor GA { static let shared = GA() }

@MainActor
func mutableLocalCaptureDataRace3() {
  var x = 0

  Task.detached { @GA in x = 1 }

  x = 2
}

technically it seems the use after send is seen during the diagnostic walk (you can see it reported in the logs), but it's hitting the error suppression condition here:

  void handleLocalUseAfterSend(LocalUseAfterSendError error) const {
    const auto &partitionOp = *error.op;
    REGIONBASEDISOLATION_LOG(error.print(llvm::dbgs(), info->getValueMap()));

    // Ignore this if we are erroring on a mutable base of a Sendable value and
    // if when we sent the value's region was not closure captured.
    if (error.op->getOptions().containsOnly(
            PartitionOp::Flag::RequireOfMutableBaseOfSendableValue) &&
        !operandToStateMap.get(error.sendingOp).isClosureCaptured)
      return;

    sendingOpToRequireInstMultiMap.insert(
        error.sendingOp, RequireInst::forUseAfterSend(partitionOp.getSourceInst()));
  }

any idea offhand what may be going on there @Michael_Gottesman? FWIW i think the second example here is essentially a reduction of the issue you referenced @CrystDragon.

The two cases that I've described in False negative in isolation checking ยท Issue #80014 ยท swiftlang/swift ยท GitHub don't reproduce anymore, but in the following scenario data race is still not detected:

// Compiled with -swift-version 6
func foo() {
    let x = X()
    doIt { [weak x] in
        x?.bar()
        x = nil // x is mutable, and should not be possible to capture in @Sendable closure
    }
}

func doIt(_ block: @escaping @Sendable () -> Void) {
}

final class X: Sendable {
    func bar() {}
}
1 Like
// but this is not:

nonisolated func mutableLocalCaptureDataRace2() {
  var x = 0

  Task { @MainActor in x = 1 } // or could use `Task.detached` here

  x = 2
}

// and neither is this:

@globalActor
actor GA { static let shared = GA() }

@MainActor
func mutableLocalCaptureDataRace3() {
  var x = 0

  Task.detached { @GA in x = 1 }

  x = 2
}

I looked at this. This is due to a slight off by one error in a certain reachability analysis... not with the patch I did. I have a small fix.

2 Likes