Skip to content

Conversation

nate-chandler
Copy link
Contributor

Now that the SwiftStdlib 5.9 macro has been defined, this test failing with stdlib debug builds is exposed. That is occurring because the back deployment fallback $ss9TaskLocalC13withValueImpl_9operation4file4lineqd__xn_qd__yYaKXESSSutYaKlFTwB is now being called.

In unoptimized stdlib builds, the version of that function that is deserialized into the test case module features an unoptimized copy of the argument %1 into a temporary (%10):

  %10 = alloc_stack $Value                        // users: %14, %13, %11
  copy_addr %1 to [init] %10 : $*Value            // id: %11
  // function_ref swift_task_localValuePush
  %12 = function_ref @swift_task_localValuePush : $@convention(thin) <τ_0_0> (Builtin.RawPointer, @in τ_0_0) -> () // user: %13
  %13 = apply %12<Value>(%9, %10) : $@convention(thin) <τ_0_0> (Builtin.RawPointer, @in τ_0_0) -> ()
  dealloc_stack %10 : $*Value                     // id: %14

This is a problem because swift_task_localValuePush allocates in the async stack (see rdar://107275872) but that fact isn't encoded in SIL (the fix for which is tracked by rdar://108260399), so the alloc_stack/dealloc_stack surrounding that call result in a violation of stack discipline.

push // alloc_stack
push // swift_task_localValuePush
pop // dealloc_stack -- oops

In optimized stdlib builds, the copy has been optimized away by the time the function is deserialized into the test case module:

bb0(%0 : $*R, %1 : $*Value, %2 : @guaranteed $@noescape @async @callee_guaranteed @substituted <τ_0_0> () -> (@out τ_0_0, @error any Error) for <R>, %3 : @guaranteed $String, %4 : $UInt, %5 : @guaranteed $TaskLocal<Value>):
  // function_ref _checkIllegalTaskLocalBindingWithinWithTaskGroup(file:line:)
  %6 = function_ref @$ss039_checkIllegalTaskLocalBindingWithinWithC5Group4file4lineySS_SutF : $@convention(thin) (@guaranteed String, UInt) -> () // user: %7
  %7 = apply %6(%3, %4) : $@convention(thin) (@guaranteed String, UInt) -> ()
  // function_ref TaskLocal.key.getter
  %8 = function_ref @$ss9TaskLocalC3keyBpvg : $@convention(method) <τ_0_0 where τ_0_0 : Sendable> (@guaranteed TaskLocal<τ_0_0>) -> Builtin.RawPointer // user: %9
  %9 = apply %8<Value>(%5) : $@convention(method) <τ_0_0 where τ_0_0 : Sendable> (@guaranteed TaskLocal<τ_0_0>) -> Builtin.RawPointer // user: %11
  // function_ref swift_task_localValuePush
  %10 = function_ref @swift_task_localValuePush : $@convention(thin) <τ_0_0> (Builtin.RawPointer, @in τ_0_0) -> () // user: %11
  %11 = apply %10<Value>(%9, %1) : $@convention(thin) <τ_0_0> (Builtin.RawPointer, @in τ_0_0) -> ()

The argument %1 is forwarded into the apply of swift_task_localValuePush.

rdar://112898559

Now that the SwiftStdlib 5.9 macro has been defined, this test failing
with stdlib debug builds is exposed. That is occurring because the back
deployment fallback
`$ss9TaskLocalC13withValueImpl_9operation4file4lineqd__xn_qd__yYaKXESSSutYaKlFTwB`
is now being called.

In unoptimized stdlib builds, the version of that function that is
deserialized into the test case module features an unoptimized copy of
the argument `%1` into a temporary (`%10`):

```
  %10 = alloc_stack $Value                        // users: %14, %13, %11
  copy_addr %1 to [init] %10 : $*Value            // id: %11
  // function_ref swift_task_localValuePush
  %12 = function_ref @swift_task_localValuePush : $@convention(thin) <τ_0_0> (Builtin.RawPointer, @in τ_0_0) -> () // user: %13
  %13 = apply %12<Value>(%9, %10) : $@convention(thin) <τ_0_0> (Builtin.RawPointer, @in τ_0_0) -> ()
  dealloc_stack %10 : $*Value                     // id: %14
```

This is a problem because `swift_task_localValuePush` allocates in the
async stack (see rdar://107275872) but that fact isn't encoded in SIL
(the fix for which is tracked by rdar://108260399), so the
`alloc_stack`/`dealloc_stack` surrounding that call result in a
violation of stack discipline.

```
push // alloc_stack
push // swift_task_localValuePush
pop // dealloc_stack -- oops
```

In optimized stdlib builds, the copy has been optimized away by the time
the function is deserialized into the test case module:

```
bb0(%0 : $*R, %1 : $*Value, %2 : @guaranteed $@NoEscape @async @callee_guaranteed @Substituted <τ_0_0> () -> (@out τ_0_0, @error any Error) for <R>, %3 : @guaranteed $String, %4 : $UInt, %5 : @guaranteed $TaskLocal<Value>):
  // function_ref _checkIllegalTaskLocalBindingWithinWithTaskGroup(file:line:)
  %6 = function_ref @$ss039_checkIllegalTaskLocalBindingWithinWithC5Group4file4lineySS_SutF : $@convention(thin) (@guaranteed String, UInt) -> () // user: %7
  %7 = apply %6(%3, %4) : $@convention(thin) (@guaranteed String, UInt) -> ()
  // function_ref TaskLocal.key.getter
  %8 = function_ref @$ss9TaskLocalC3keyBpvg : $@convention(method) <τ_0_0 where τ_0_0 : Sendable> (@guaranteed TaskLocal<τ_0_0>) -> Builtin.RawPointer // user: %9
  %9 = apply %8<Value>(%5) : $@convention(method) <τ_0_0 where τ_0_0 : Sendable> (@guaranteed TaskLocal<τ_0_0>) -> Builtin.RawPointer // user: %11
  // function_ref swift_task_localValuePush
  %10 = function_ref @swift_task_localValuePush : $@convention(thin) <τ_0_0> (Builtin.RawPointer, @in τ_0_0) -> () // user: %11
  %11 = apply %10<Value>(%9, %1) : $@convention(thin) <τ_0_0> (Builtin.RawPointer, @in τ_0_0) -> ()
```

The argument `%1` is forwarded into the apply of `swift_task_localValuePush`.

rdar://112898559
@nate-chandler nate-chandler requested a review from a team as a code owner July 27, 2023 03:36
@nate-chandler
Copy link
Contributor Author

@swift-ci please test and merge

@swift-ci swift-ci merged commit d84f386 into swiftlang:release/5.9 Jul 27, 2023
@nate-chandler nate-chandler deleted the cherrypick/release/5.9/rdar112898559 branch July 27, 2023 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants