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.

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

Verify SwiftPM dependencies that contain C and C++ #595

Closed
MaxDesiatov opened this issue Apr 7, 2020 · 12 comments
Closed

Verify SwiftPM dependencies that contain C and C++ #595

MaxDesiatov opened this issue Apr 7, 2020 · 12 comments
Labels
bug Something isn't working

Comments

@MaxDesiatov
Copy link

MaxDesiatov commented Apr 7, 2020

Such packages don't seem to be successfully building as of DEVELOPMENT-SNAPSHOT-2020-04-06-a

@MaxDesiatov MaxDesiatov added the bug Something isn't working label Apr 7, 2020
@MaxDesiatov MaxDesiatov changed the title Fix building packages that contain C and C++ code Fix building SwiftPM dependencies that contain C and C++ code Apr 7, 2020
@MaxDesiatov MaxDesiatov changed the title Fix building SwiftPM dependencies that contain C and C++ code Fix building SwiftPM dependencies that contain C and C++ Apr 7, 2020
@MaxDesiatov
Copy link
Author

One example when buiding OpenCombine:

While building module 'std' imported from .swiftenv/versions/wasm-DEVELOPMENT-SNAPSHOT-2020-04-05-a/usr/share/wasi-sysroot/include/c++/v1/stdint.h:123:
In file included from <module-includes>:20:
fatal error: 'setjmp.h' file not found
#include_next <setjmp.h>
              ^~~~~~~~~~
In file included from .build/checkouts/OpenCombine/Sources/COpenCombineHelpers/COpenCombineHelpers.cpp:8:
In file included from .build/checkouts/OpenCombine/Sources/COpenCombineHelpers/include/COpenCombineHelpers.h:11:
: could not build module 'std'
#include_next <stdint.h>
 ~~~~~~~~~~~~~^
2 errors generated.

@kateinoigakukun
Copy link
Member

FYI: setjmp.h is omitted by wasi-libc because of WebAssembly limitation. https://github.com/swiftwasm/wasi-libc/blob/6f0e5680720bcb3ead62b71ea1bf9e6d50173dd1/Makefile#L294

@MaxDesiatov
Copy link
Author

I understand that, was a bit more confused by this part:

could not build module 'std'
#include_next <stdint.h>

I'm currently trying to fix OpenCombine by enabling conditional target dependencies from swiftlang/swift-package-manager#2428 so that it avoids building COpenCombineHelpers for WASI and uses custom stubs instead, but I'm currently blocked by #645, will try again tomorrow after the next snapshot is built with the latest SwiftPM code.

@MaxDesiatov MaxDesiatov changed the title Fix building SwiftPM dependencies that contain C and C++ Verify SwiftPM dependencies that contain C and C++ Apr 12, 2020
@MaxDesiatov
Copy link
Author

MaxDesiatov commented May 1, 2020

I can confirm that building C++ code does not work with our toolchain, probably due to the inclusion of pthread headers in our builds:

% cat test.cpp
#include <iostream>

int main() {
    std::cout << "Hello World!";
    return 0;
}

% ~/.swiftenv/versions/wasm-DEVELOPMENT-SNAPSHOT-2020-04-28-a/usr/bin/clang++ \
--sysroot=/Users/maxd/.swiftenv/versions/wasm-DEVELOPMENT-SNAPSHOT-2020-04-28-a/usr/share/wasi-sysroot \
test.cpp -o test
wasm-ld: error: /Users/maxd/.swiftenv/versions/wasm-DEVELOPMENT-SNAPSHOT-2020-04-28-a/usr/share/wasi-sysroot/lib/wasm32-wasi/libc++.a(memory.cpp.obj): 
undefined symbol: pthread_mutex_lock
wasm-ld: error: /Users/maxd/.swiftenv/versions/wasm-DEVELOPMENT-SNAPSHOT-2020-04-28-a/usr/share/wasi-sysroot/lib/wasm32-wasi/libc++.a(memory.cpp.obj): 
undefined symbol: pthread_mutex_unlock
wasm-ld: error: /Users/maxd/.swiftenv/versions/wasm-DEVELOPMENT-SNAPSHOT-2020-04-28-a/usr/share/wasi-sysroot/lib/wasm32-wasi/libc++.a(condition_variable.cpp.obj): 
undefined symbol: pthread_cond_wait
wasm-ld: error: /Users/maxd/.swiftenv/versions/wasm-DEVELOPMENT-SNAPSHOT-2020-04-28-a/usr/share/wasi-sysroot/lib/wasm32-wasi/libc++.a(condition_variable.cpp.obj): 
undefined symbol: pthread_cond_broadcast
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

@ddunbar
Copy link

ddunbar commented May 6, 2020

What I don't understand here is why WASI includes headers which reference setjmp.h, if they exclude it? It may be that these are the compiler's headers, and they are actually what need to be changed?

@ddunbar
Copy link

ddunbar commented May 6, 2020

For the setjmp issue specifically, it looks like perhaps setjmp.h, csetjmp and related entries in the module.modulemap for the compiler internal headers should just be removed? I also had to work around a bogus reference to ::signal from csignal

@MaxDesiatov
Copy link
Author

MaxDesiatov commented May 6, 2020

Interesting, apparently wasi-sdk omits some headers (including setjmp.h), but doesn't do that for C++ headers? I'll double check.

Also, as far as I know, the only substantial difference between our wasi-sdk fork and the upstream repo is the inclusion of pthread headers. I'm not sure whether we should actually include them, omitting them would require updating a lot more code probably. Maybe it's worth sticking to it and waiting until wasm32/WASI get proper threading support? We do provide pthread stubs, but those are only linked when you build with SwiftPM, which is not applicable for standalone C++ executables like the invocation of clang++ above. Perhaps that's fine, the downside being that our users need to switch their toolchain to something pthread-less like the upstream wasi-sdk just to compile some standalone C++ targeting WASI.

@ddunbar
Copy link

ddunbar commented May 6, 2020

Upstream probably just forgot about the C++ setjmp headers... modules is what is causing it to break and that is rare to use

@ddunbar
Copy link

ddunbar commented May 6, 2020

My vote would be to drop pthread and match what WASI is doing, but I don’t really know what I am talking about

@MaxDesiatov
Copy link
Author

Upstream probably just forgot about the C++ setjmp headers... modules is what is causing it to break and that is rare to use

There's an issue upstream that tracks this inconsistency, doesn't seem like there were any updates recently WebAssembly/wasi-sdk#93

My vote would be to drop pthread and match what WASI is doing, but I don’t really know what I am talking about

That's my vote too, I'd actually prefer to use upstream wasi-sdk directly, but I didn't touch the pthread parts in the first place to speak confidently about it, @kateinoigakukun quite probably knows more about it.

@MaxDesiatov
Copy link
Author

Just to confirm, C packages build ok, I've got a fork of zlib (the only change being a Package.swift addition) building now, so it's just C++ package that are problematic due to wasi-sdk not cleaning up their C++ headers.

@kateinoigakukun
Copy link
Member

@MaxDesiatov Sorry for waiting a long time for this issue.

In short, you can link pthread things with libswiftWasiPthread.a

$ clang++ main.cpp \
   --sysroot=$(dirname $(swiftenv which swiftc))/../share/wasi-sysroot \
   -L$(dirname $(swiftenv which swiftc))/../lib/swift_static/wasi \
   -lswiftWasiPthread 

But in the long run, it's essential to support pthread in wasi-libc.

MaxDesiatov added a commit that referenced this issue Jul 14, 2020
Update WASI SDK to fix C++ setjmp/signal headers

Fixes the issue #595 with C++ packages for the development snapshots made from the `swiftwasm` branch. Previously C++ dependencies couldn't be built as default Clang headers weren't tailored to support WASI, WebAssembly/wasi-sdk#93 being one of the examples.

This should also unblock TokamakUI/Tokamak#170.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants