Skip to content

Build Foundation and enable its test suite #592

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
MaxDesiatov opened this issue Apr 7, 2020 · 12 comments
Closed

Build Foundation and enable its test suite #592

MaxDesiatov opened this issue Apr 7, 2020 · 12 comments
Labels
continuous integration Improvements to continuous integration

Comments

@MaxDesiatov
Copy link

MaxDesiatov commented Apr 7, 2020

We haven't verified how much of the Foundation module works on WASI. At a glance there don't seem to be any obvious blockers that prevent something like FoundationXML from working, but it's clear that FoundationNetworking would have to look quite differently. This needs to be verified by enabling the test suite and checking which of the tests fail.

@MaxDesiatov
Copy link
Author

MaxDesiatov commented Apr 16, 2020

Looks like without #658 and #647 a very significant chunk of CoreFoundation and Foundation needs to be disabled. This is exacerbated by a lack of proper filesystem APIs. After spending a lot of time on it, I don't think I'm very close to fixing this, even simple APIs like CFString, NSString, CFDictionary etc use blocks which require BlocksRuntime, but these simple types are used almost everywhere in the codebase of Foundation. I think important types like Date and Data can probably be reimplemented separately as stubs, while I'm not sure we'll see enough demand for NSString, NSDictionary, NSDate, NSData etc as those are legacy types and should be avoided anyway.

This probably can be revisited after WASI supports threading and dynamic linking properly.

@MaxDesiatov MaxDesiatov changed the title Enable the Foundation test suite Build Foundation and enable its test suite Apr 16, 2020
@MaxDesiatov
Copy link
Author

MaxDesiatov commented May 13, 2020

I've got simple things like print("\(Date().timeIntervalSince1970)") working, but something more complex, e.g. date formatters and time zones crash with out of bounds error.

The stacktrace I got with wasminspect looks like this with code as simple as print("\(Date())"):

0: _swift_release_dealloc
1: bool swift::RefCounts<swift::RefCountBitsT<(swift::RefCountInlinedness)1> >::doDecrementSlow<(swift::PerformDeinit)1>(swift::RefCountBitsT<(swift::RefCountInlinedness)1>, unsigned int)
2: bool swift::RefCounts<swift::RefCountBitsT<(swift::RefCountInlinedness)1> >::doDecrement<(swift::PerformDeinit)1>(unsigned int)
3: swift_release
4: _CFRelease
5: CFRelease
6: __CFTimeZoneCreateSystem
7: CFTimeZoneCopySystem
8: CFTimeZoneCopyDefault
9: __CreateCFDateFormatter
10: CFDateFormatterCreate
11: Foundation.NSDate.description.getter : Swift.String
12: Foundation.Date.description.getter : Swift.String
13: protocol witness for Swift.CustomStringConvertible.description.getter : Swift.String in conformance Foundation.Date : Swift.CustomStringConvertible in Foundation
14: Swift.DefaultStringInterpolation.appendInterpolation<A where A: Swift.CustomStringConvertible>(A) -> ()
15: main
16: __original_main
17: _start

@kateinoigakukun
Copy link
Member

@MaxDesiatov Awesome! I think the out of bounds error is due to thin ICU data.

Could you try to build ICU disabling ICU_DATA_FILTER_FILE for temporary?

https://github.com/swiftwasm/icu4c-wasi/pull/1/files

@MaxDesiatov
Copy link
Author

Do I need to rebuild the whole SDK with this ICU? Or do you think that swapping ICU .a libraries in the existing toolchain/SDK installation and relinking with those would work?

@kateinoigakukun
Copy link
Member

swapping ICU .a libraries in the existing toolchain/SDK installation and relinking with those would work?

Yes, it would work because libicudata only has data section.

@MaxDesiatov
Copy link
Author

MaxDesiatov commented May 13, 2020

That didn't work unfortunately, I've copied .a files from an artifact in this PR swiftwasm/icu4c-wasi#2

It doesn't even seem to be linking with libicudata.a:

/Users/maxd/.swiftenv/versions/wasm-DEVELOPMENT-SNAPSHOT-2020-05-11-a/usr/bin/swiftc 
-sdk /Users/maxd/.swiftenv/versions/wasm-DEVELOPMENT-SNAPSHOT-2020-05-11-a/usr/share/wasi-sysroot 
-I /Users/maxd/.swiftenv/versions/wasm-DEVELOPMENT-SNAPSHOT-2020-05-11-a/usr/lib/swift/wasi/wasm32 
-Xlinker -lCoreFoundation -Xlinker -lBlocksRuntime -Xlinker -licui18n -Xlinker -luuid 
-L /Users/maxd/Documents/life-game-with-swiftwasm/LifeGame/.build/wasm32-unknown-wasi/debug 
-o /Users/maxd/Documents/life-game-with-swiftwasm/LifeGame/.build/wasm32-unknown-wasi/debug/LifeGameWeb
 -module-name LifeGameWeb 
-emit-executable @/Users/maxd/Documents/life-game-with-swiftwasm/LifeGame/.build/wasm32-unknown-wasi/debug/LifeGameWeb.product/Objects.LinkFileList 
-target wasm32-unknown-wasi 
-L /Users/maxd/.swiftenv/versions/wasm-DEVELOPMENT-SNAPSHOT-2020-05-11-a/usr/lib

% wasmer .build/debug/LifeGameWeb                                    
Error: error: Caught exception of type "MemoryOutOfBounds".

% ../../wasminspect/target/debug/wasminspect .build/debug/LifeGameWeb
(wasminspect) run
Function exec failure Memory(AccessOutOfBounds(Some(18446744073709551612), 29622272))
(wasminspect) thread backtrace
0: _swift_release_dealloc
1: bool swift::RefCounts<swift::RefCountBitsT<(swift::RefCountInlinedness)1> >::doDecrementSlow<(swift::PerformDeinit)1>(swift::RefCountBitsT<(swift::RefCountInlinedness)1>, unsigned int)
2: bool swift::RefCounts<swift::RefCountBitsT<(swift::RefCountInlinedness)1> >::doDecrement<(swift::PerformDeinit)1>(unsigned int)
3: swift_release
4: _CFRelease
5: CFRelease
6: __CFTimeZoneCreateSystem
7: CFTimeZoneCopySystem
8: CFTimeZoneCopyDefault
9: __CreateCFDateFormatter
10: CFDateFormatterCreate
11: Foundation.NSDate.description.getter : Swift.String
12: Foundation.Date.description.getter : Swift.String
13: protocol witness for Swift.CustomStringConvertible.description.getter : Swift.String in conformance Foundation.Date : Swift.CustomStringConvertible in Foundation
14: Swift.DefaultStringInterpolation.appendInterpolation<A where A: Swift.CustomStringConvertible>(A) -> ()
15: main
16: __original_main
17: _start

The only thing I have in Sources/LifeGameWeb/main.swift is this:

import Foundation

print("\(Date())")

I also build with swift build -v --destination destination.json where destination.json looks like this:

{
  "version": 1,
  "target": "wasm32-unknown-wasi",
  "sdk": "/Users/maxd/.swiftenv/versions/wasm-DEVELOPMENT-SNAPSHOT-2020-05-11-a/usr/share/wasi-sysroot",
  "toolchain-bin-dir": "/Users/maxd/.swiftenv/versions/wasm-DEVELOPMENT-SNAPSHOT-2020-05-11-a/usr/bin/",
  "extra-cc-flags": [],
  "extra-swiftc-flags": [
    "-I", "/Users/maxd/.swiftenv/versions/wasm-DEVELOPMENT-SNAPSHOT-2020-05-11-a/usr/lib/swift/wasi/wasm32",
    "-Xlinker", "-lCoreFoundation",
    "-Xlinker", "-lBlocksRuntime",
    "-Xlinker", "-licui18n",
    "-Xlinker", "-luuid"
  ],
  "extra-cpp-flags": []
}

I've manually copied Foundation and CoreFoundation libraries to the toolchain directories like ~/.swiftenv/versions/wasm-DEVELOPMENT-SNAPSHOT-2020-05-11-a/usr/lib/swift/ and ~/.swiftenv/versions/wasm-DEVELOPMENT-SNAPSHOT-2020-05-11-a/usr/lib/swift_static/.

But you probably shouldn't spend time on reproducing it in this state, there are too many manual steps here. I'll try to submit a PR soon that copies Foundation into our installable distribution so that this issue could be reproduced more easily.

@MaxDesiatov
Copy link
Author

Seems like that issue was only reproducible after attempting to call CFRelease on a dynamically allocated CFString. The runtime wasn't able to set string length flags properly in __CFRuntimeSetValue, because for some reason atomic_compare_exchange_weak made the loop to be called twice. I've edited it this way in swiftwasm/swift-corelibs-foundation@1b13a94:

static inline void __CFRuntimeSetValue(CFTypeRef cf, uint8_t n1, uint8_t n2, uint8_t x) {
    printf("start of the call to __CFRuntimeSetValue\n");
    __CFInfoType info = atomic_load(&(((CFRuntimeBase *)cf)->_cfinfoa));
    __CFInfoType newInfo;
    __CFInfoType mask = __CFInfoMask(n1, n2);
    printf("previous value between bits %d and %d on %p was %d\n", n1, n2, cf, info);
    #if !TARGET_OS_WASI
    do {
    #endif
        // maybe don't need to do the negation part because the right side promises that we are not going to touch the rest of the word
        newInfo = (info & ~mask) | ((x << n2) & mask);
        printf("going to set value between bits %d and %d to %d on %p\n", n1, n2, newInfo, cf);
    #if !TARGET_OS_WASI
    } while (!atomic_compare_exchange_weak(&(((CFRuntimeBase *)cf)->_cfinfoa), &info, newInfo));
    #else
    ((CFRuntimeBase *)cf)->_cfinfoa = newInfo;
    #endif
}

When #if !TARGET_OS_WASI is not there, you get this output:

start of the call to __CFRuntimeSetValue
previous value between bits 6 and 5 on 0x310230 was 1920
going to set value between bits 6 and 5 to 1920 on 0x310230
going to set value between bits 0 and 0 to 1920 on 0x310230
setting flag 4 on 0x310230 to 0
start of the call to __CFRuntimeSetValue
previous value between bits 4 and 4 on 0x310230 was 1920
going to set value between bits 4 and 4 to 1920 on 0x310230
going to set value between bits 0 and 0 to 1920 on 0x310230
getting flag 4 on 0x310230, value is 0
setting flag 3 on 0x310230 to 1
start of the call to __CFRuntimeSetValue
previous value between bits 3 and 3 on 0x310230 was 1920
going to set value between bits 3 and 3 to 1928 on 0x310230
going to set value between bits 0 and 0 to 1920 on 0x310230
getting flag 3 on 0x310230, value is 0
setting useLengthByte to 1
setting flag 2 on 0x310230 to 1
start of the call to __CFRuntimeSetValue
previous value between bits 2 and 2 on 0x310230 was 1920
going to set value between bits 2 and 2 to 1924 on 0x310230
going to set value between bits 0 and 0 to 1920 on 0x310230

This is mind-boggling, n1 and n2 values aren't modified in the loop, but they get reset to 0 on the second iteration somehow 🤯 Could be a bug with WebAssembly codegen for atomic_compare_exchange_weak in LLVM itself?

@MaxDesiatov
Copy link
Author

The issues is fixed now here, just a one-liner fix, but took ages to diagnose... 😰

|| TARGET_OS_WASI was missing in this condition, thus __CFInitialize was never called:

#if TARGET_OS_LINUX || TARGET_OS_BSD || TARGET_OS_WASI
static void __CFInitialize(void) __attribute__ ((constructor));
static
#endif

We still need to wait for upstream snapshots to be updated as current nightly swift.org snapshots are much older than upstream master and cause build issues in #1141.

Or we need to update the CI builds to become multi-stage, which is going to take some time too. We actually have to do that at some point anyway before everything is upstreamed.

@MaxDesiatov MaxDesiatov pinned this issue Jun 3, 2020
@MaxDesiatov MaxDesiatov added the continuous integration Improvements to continuous integration label Jun 10, 2020
@MaxDesiatov
Copy link
Author

Starting with wasm-DEVELOPMENT-SNAPSHOT-2020-06-07-a and later, both Foundation and XCTest should be included in the distribution. I'll keep the issue open until we enable the test suite on our CI.

@bensyverson
Copy link

What's the next step with Foundation? Importing it with wasm-DEVELOPMENT-SNAPSHOT-2020-06-07-a fails with: error: missing required module 'CoreFoundation'

Curious to know if there's a small project related to Foundation that I could tackle.

@MaxDesiatov
Copy link
Author

There are still some special flags that you need to pass to link with Foundation, but the main branch of carton already supports automatic linking. I hope to tag a new version with that feature later today or maybe tomorrow if all goes well.

There are still many areas of Foundation that aren't supported currently, probably Bundle being the main one sorely missing to fully enable SwiftPM package resources support. You can check the full list of files removed from SwiftWasm builds in this CMakeLists.txt file. The main entry point for building it is the build-foundation.sh script indirectly invoked from our main ci.sh script. We also don't run Foundation tests yet on CI, adding support for that would be greatly appreciated 🙏

@MaxDesiatov
Copy link
Author

Closing this as superseded by #1714.

@MaxDesiatov MaxDesiatov unpinned this issue Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
continuous integration Improvements to continuous integration
Projects
None yet
Development

No branches or pull requests

3 participants