Skip to content
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.

Sign up for GitHub

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

Jump to bottom

6.0: Fix WASI support #825

Merged
merged 13 commits into from
Aug 29, 2024
Merged

6.0: Fix WASI support #825

merged 13 commits into from
Aug 29, 2024

Conversation

MaxDesiatov
Copy link
Contributor

@MaxDesiatov MaxDesiatov commented Aug 5, 2024 •
edited
Loading

Explanation: these patches restore support for WASI that was available in swift-corelibs-foundation before it was recored.
Scope: Isolated to WASI codepaths.
Risk: Low due to isolated scope.
Testing: Tested manually for WASI, on CI with the existing test suite for other platforms.
Issue: rdar://133231577
Reviewer: @MaxDesiatov and @jmschonfeld

@kateinoigakukun @MaxDesiatov
Add explicit include of wasi/libc-environ.h (#786)
cfb5a02 
This is necessary to get the `__wasilibc_get_environ` function
declaration.

(cherry picked from commit 243066f)
@kateinoigakukun @MaxDesiatov
Add explicit void type parameter to C functions without parameters (#775
189cfd8 
)

C functions with `()` as parameter list can take any number of
parameters. But WebAssembly requires static signature information for
every function call, so we need to explicitly specify `(void)` to
indicate that the function takes no parameters.

(cherry picked from commit 8f34f38)
@kateinoigakukun @MaxDesiatov
Exclude EREMOTE definition for WASI platform (#778)
9f3ab60 
WASI does not define the EREMOTE error code.

(cherry picked from commit 6bb5ff7)
@kateinoigakukun @MaxDesiatov
Throw .featureUnsupported when attempting to create temp files on W…
9c6d53c 
…ASI (#779)

WASI does not have temp directory concept, and does not provide mktemp
family of functions, so attempting to create a temporary file should be
considered a feature unsupported.

(cherry picked from commit fb11420)
@kateinoigakukun @MaxDesiatov
Fix operatingSystemVersion on WASI (#782)
1b8c399 
The `operatingSystemVersion` property type is a tuple but the it was
returning an `OperatingSystemVersion` instance on unknown platforms.

(cherry picked from commit a8f1225)
@kateinoigakukun @MaxDesiatov
Guard out extended or fs attributes related code on WASI (#784)
0f1c377 
This commit guards out the extended attributes and file system
attributes related code on WASI as WASI does not support these
features. Just return nothing or ignore the set request.

(cherry picked from commit fab7195)
@kateinoigakukun @MaxDesiatov
Guard out user/group related code on WASI (#783)
0e3174f 
* Guard out user/group related code on WASI

This change guards out the user/group related code on WASI, as WASI does
not have the concept of users or groups.

* Throw explicit unsupported error if trying to set user or group on WASI

Instead of implicitly ignoring user-given values, we should throw
exception to make it clear that those values cannot be set on WASI.

(cherry picked from commit 0b3974d)
@kateinoigakukun @MaxDesiatov
Skip sticky-bit check in isDeletableFile on WASI (#785)
05fc5eb 
WASI does not surface the sticky bit and getuid, so we cannot check
whether the file is actually deletable before attempting to delete it.

(cherry picked from commit e90b6c3)
@kateinoigakukun @MaxDesiatov
Implement _copyRegularFile for WASI without sendfile (#787)
779e11a 
WASI doesn't have `sendfile`, so we need to implement the copy in user
space with `read` and `write`. It's not as efficient as `sendfile`, but
it's the best we can do.

(cherry picked from commit 2a6afeb)
@kateinoigakukun @MaxDesiatov
Port LockedState and _ThreadLocal to WASI without any locking (#780)
03c0884 
(cherry picked from commit aa68eeb)
@kateinoigakukun @MaxDesiatov
Add WASI platform conditions for libc imports and word size (#776)
2ca6906 
* Add `import WASILibc` statements to libc import chains

* Declare wasm32 arch as 32-bit environment

* Switch to _pointerBitWidth for architecture checks

This change switches the architecture checks in Data.swift to use the
_pointerBitWidth instead of the arch() checks for consistency with newer
platforms.

(cherry picked from commit c82d167)
@kateinoigakukun @MaxDesiatov
Enable wasi-libc emulation features (#777)
fafdf90 
* Enable wasi-libc emulation features

Those features require explicit macro definitions to be enabled, so add
them to the package definition. Only affects WASI builds.

* Prefer `TARGET_OS_WASI` over `__wasi__`

And explain why we need definition checks for `signal.h` and `sys/mman.h`

(cherry picked from commit c86692f)
@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@kateinoigakukun kateinoigakukun mentioned this pull request Aug 9, 2024
Closed
33 tasks
@MaxDesiatov
Copy link
Contributor Author

@kateinoigakukun is there anything else you'd like to cherry-pick here if possible, keeping changes at low risk?

@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@MaxDesiatov
Copy link
Contributor Author

MaxDesiatov commented Aug 26, 2024 •
edited
Loading

@kateinoigakukun have you seen this failure on Linux before, or is this branch missing some Wasm-related commits from main that would fix this? IIUC release/6.0 was building fine as of #868, so we're probably introducing some regression here.

FoundationEssentialsTests/JSONEncoderTests.swift:2402: Fatal error: Unexpectedly found nil while unwrapping an Optional value

*** Signal 4: Backtracing from 0x7fd04cf28314... done ***

*** Program crashed: Illegal instruction at 0x00007fd04cf28314 ***

Thread 0 "FoundationPrevi" crashed:

0              0x00007fd04cf28314 closure #1 in closure #1 in closure #1 in _assertionFailure(_:_:file:line:flags:) + 308 in libswiftCore.so
1 [ra]         0x00007fd04cf280c2 closure #1 in closure #1 in _assertionFailure(_:_:file:line:flags:) + 257 in libswiftCore.so
2 [ra]         0x00007fd04cf27f7d closure #1 in _assertionFailure(_:_:file:line:flags:) + 380 in libswiftCore.so
3 [ra]         0x00007fd04d0a4576 _assertionFailure(_:_:file:line:flags:) + 229 in libswiftCore.so
4 [ra]         0x00005603dcd72531 JSONEncoderTests._run_failTest<A>(name:type:) + 448 in FoundationPreviewPackageTests.xctest at /build/swift-foundation/Tests/FoundationEssentialsTests/JSONEncoderTests.swift:2402:101
5 [ra]         0x00005603dcd72910 JSONEncoderTests.test_JSONFailTests() + 95 in FoundationPreviewPackageTests.xctest at /build/swift-foundation/Tests/FoundationEssentialsTests/JSONEncoderTests.swift:2415:9
6 [ra]         0x00005603dd3644eb implicit closure #152 in implicit closure #151 in variable initialization expression of staerror: Exited with unexpected signal code 4
tic JSONEncoderTests.__allTests__JSONEncoderTests + 26 in FoundationPreviewPackageTests.xctest at /build/swift-foundation/.build/x86_64-unknown-linux-gnu/debug/FoundationPreviewPackageDiscoveredTests.derived/FoundationEssentialsTests.swift:507:32
7 [ra] [thunk] 0x00005603dd34daef thunk for @escaping @callee_guaranteed () -> (@error @owned Error) + 14 in FoundationPreviewPackageTests.xctest at //<compiler-generated>
8 [ra] [thunk] 0x00005603dd37ad64 thunk for @escaping @callee_guaranteed () -> (@error @owned Error)partial apply + 19 in FoundationPreviewPackageTests.xctest at //<compiler-generated>

@kateinoigakukun
Copy link
Member

kateinoigakukun commented Aug 26, 2024 •
edited
Loading

Hmm, I haven't seen it. At least we haven't touched JSONEncoder stuff in WASI support changes.

@jmschonfeld
Copy link
Contributor

That failure was a regression caused by a change in SwiftPM, which was fixed on main by @xedin with #855 but AFAICT that change was not cherry-picked back to release/6.0. @xedin - was the SwiftPM change cherry-picked to release/6.0? If so we'll need to cherry pick your test fix as well to unblock swift-foundation testing

@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@xedin
Copy link
Contributor

xedin commented Aug 27, 2024

@jmschonfeld No, the changes I made to SwiftPM are main branch only, there weren't cherry-picked to release/6.0

@bnbarham
Copy link

@jmschonfeld No, the changes I made to SwiftPM are main branch only, there weren't cherry-picked to release/6.0

Worth noting though that it is currently using the incorrect path anyway, it's quite possible that the build plan changed here somehow. We should definitely cherry-pick the test fix, it's very safe.

@xedin
Copy link
Contributor

xedin commented Aug 28, 2024

@swift-ci please test Linux platform

itingliu
itingliu approved these changes Aug 28, 2024
View reviewed changes
Copy link
Contributor

@itingliu itingliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have extra commits here that need to be merged back into main, right?

@MaxDesiatov
Copy link
Contributor Author

MaxDesiatov commented Aug 28, 2024 •
edited
Loading

No, this is only change I made and it's already on main:

#elseif canImport(Musl)
import Musl

@MaxDesiatov MaxDesiatov merged commit b0abbb4 into release/6.0 Aug 29, 2024
3 checks passed
@MaxDesiatov MaxDesiatov deleted the maxd/6.0-wasi-support branch August 29, 2024 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@itingliu itingliu itingliu approved these changes

@bnbarham bnbarham Awaiting requested review from bnbarham

@jmschonfeld jmschonfeld Awaiting requested review from jmschonfeld

@iCharlesHu iCharlesHu Awaiting requested review from iCharlesHu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@MaxDesiatov @kateinoigakukun @jmschonfeld @xedin @bnbarham @itingliu