Skip to content

DiscardingTaskGroup has ABI signature mismatch issue #63060

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kateinoigakukun opened this issue Jan 17, 2023 · 7 comments
Closed

DiscardingTaskGroup has ABI signature mismatch issue #63060

kateinoigakukun opened this issue Jan 17, 2023 · 7 comments
Assignees
Labels
affects ABI Flag: Affects ABI bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. Concurrencу Area → standard library: The `Concurrency` module under the standard library umbrella crash Bug: A crash, i.e., an abnormal termination of software run-time crash Bug → crash: Swift code crashed during execution standard library Area: Standard library umbrella

Comments

@kateinoigakukun
Copy link
Member

Description

There is an ABI signature mismatch in DiscardingTaskGroup introduced by #62914.

Steps to reproduce

It looks like the following example crashes in WebAssembly because of ABI signature mismatch.

@main struct Main {
  static func main() async {
    await withDiscardingTaskGroup() { group in
      group.addTask {
      }
    }
  }
}
$ wasminspect a.out
Function exec failure indirect call type mismatch for '$sIeghH_s5Error_pIegHzo_TRTA':
 >> call_indirect instruction expected FuncType { params: [I32, I32, I32, I32], returns: [] }
 >> but actual implementation has      FuncType { params: [I32, I32, I32], returns: [] }
(wasminspect) bt
0: future_adapter(swift::AsyncContext*)
1: swift::runJobInEstablishedExecutorContext(swift::Job*)
2: swift_job_run
3: swift_task_donateThreadToGlobalExecutorUntil
4: swift_task_asyncMainDrainQueueImpl()
5: swift_task_asyncMainDrainQueue
6: main
7: main
8: __main_void
9: __original_main
10: _start
11: _start.command_export

It means $sIeghH_s5Error_pIegHzo_TRTA called by future_adapter is expected to be void (OpaqueValue *indirectResult, SWIFT_ASYNC_CONTEXT AsyncContext *, SWIFT_CONTEXT void *); but it actually has void (SWIFT_ASYNC_CONTEXT AsyncContext *, SWIFT_CONTEXT void *) due to void result type.

This mismatch happens only when swift_task_create is emitted through CreateAsyncTaskInGroup SIL builtin function because it does not create re-abstraction thunk for its continuation function even though CreateAsyncTask SIL builtin function create a thunk for it.

https://github.com/apple/swift/blob/a5bcf13ea905770713e647764bbdc687ca3f2dae/lib/SILGen/SILGenBuiltin.cpp#L1567-L1571

https://github.com/apple/swift/blob/a5bcf13ea905770713e647764bbdc687ca3f2dae/lib/SILGen/SILGenBuiltin.cpp#L1608-L1609

Fortunately(?), this mismatch does not cause real problems on platforms where Swift contexts and error are allocated in reserved registers, but it causes something wrong in other platforms.

I investigated this a little bit closer, and I found swift_task_create supports null futureResultType with non_future_adapter, which calls the continuation function without indirect result pointer. I thought it would be better to use this feature for DiscardingTaskGroup and where the result type is known to be void.
However, it looks like no one is using this now. Do we still use this path now?
https://github.com/apple/swift/blob/4551d21375f6696ba25055ddb1a085f6bf82e6e1/stdlib/public/Concurrency/Task.cpp#L870-L879

It's not clear to me whether way (1. reabstraction thunk, 2. null futureResultType) I should take to fix this, so I opened this ticket to ask you @ktoso :)

Expected behavior

No ABI signature mismatch.

Environment

  • Swift compiler version info
  • Xcode version info
  • Deployment target:
@kateinoigakukun kateinoigakukun added bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. concurrency Feature: umbrella label for concurrency language features labels Jan 17, 2023
@kateinoigakukun kateinoigakukun changed the title DiscardingTaskGroup has ABI signature mismatch issue DiscardingTaskGroup has ABI signature mismatch issue Jan 17, 2023
@ktoso
Copy link
Contributor

ktoso commented Jan 17, 2023

Thank you for the report, I'll look into it and ask around what we should do here. null futureResultType seems easier perhaps, but I don't know what the "right" solution is -- I'll ask within the compiler team for advice. cc @DougGregor @kavon

@rjmccall
Copy link
Contributor

Passing a null result type means the task won't have a future segment at all. That should be fine (and preferred) for DiscardingTaskGroup, since its subtasks aren't exposed as Tasks, but it does mean that you can't just use it whenever the task returns Void. It looks like you'll need to add a new SIL builtin to create a futureless task in a task group.

@ktoso
Copy link
Contributor

ktoso commented Jan 25, 2023

Do you think you'll be able to submit a PR here @kateinoigakukun ? Just want to confirm if you're intending to.

@kateinoigakukun
Copy link
Member Author

@ktoso I don't think I'll be able to do it for a while, sorry 🙇

@ktoso
Copy link
Contributor

ktoso commented Jan 25, 2023

No worries, just needed clarity if your PoC commit was signal that you're about to fix this or not :)

I'll try to make the time soon.

@ktoso
Copy link
Contributor

ktoso commented Feb 21, 2023

rdar://104628947

@kateinoigakukun
Copy link
Member Author

Fixed by #66219

@AnthonyLatsis AnthonyLatsis added standard library Area: Standard library umbrella affects ABI Flag: Affects ABI Concurrencу Area → standard library: The `Concurrency` module under the standard library umbrella crash Bug: A crash, i.e., an abnormal termination of software run-time crash Bug → crash: Swift code crashed during execution and removed concurrency Feature: umbrella label for concurrency language features labels Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects ABI Flag: Affects ABI bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. Concurrencу Area → standard library: The `Concurrency` module under the standard library umbrella crash Bug: A crash, i.e., an abnormal termination of software run-time crash Bug → crash: Swift code crashed during execution standard library Area: Standard library umbrella
Projects
None yet
Development

No branches or pull requests

4 participants