cc,wasi: include emulated libs in WASI libc and refactor WASI libc handling by kubkon · Pull Request #8992 · ziglang/zig

kubkon · 2021-06-04T23:14:26Z

This commit includes emulated libc sublibs that were not included in the compilation and caching of WASI libc that ships with Zig. The libs include (emulated): process clocks, getpid, mman, and signal.

With this change, it is now possible to successfully cross-compile wasm3 engine to WASI with zig cc.

For the future though, it might be worth considering splitting WASI libc into libc-proper and modularised emulated libs as it is done in upstream, and then have them included only if the user specifically requests emulation/parts of it.

EDIT:

This PR now replicates the expected behavior when using clang with upstream wasi-libc sysroot: linking emulated subcomponents such as process clocks or signals requires an explicit link flag in the compiler invocation, for example:

zig cc -target wasm32-wasi -lwasi-emulated-process-clocks main.c -o main.wasm

In build.zig, accessing emulated subcomponents is equivalent to linking a system lib, e.g.,:

pub fn build(b: *std.build.Builder) void {
    const exe = b.addExecutable("main", null);
    exe.linkSystemLibrary("wasi-emulated-process-clocks");
}

EDIT2:

I've also streamlined the compilation logic a little in that we only compile those emulated components that were requested and cache them:

Move parsing of system libs into main.zig next to where we decide
if we should link libC, and, if targeting WASI, if the specified
libname equals one of the emulated components, save it on the side
and remove it from the system libs. Then, build only those parts
of WASI libc that were preserved in the previous step.

This also fixes building of different crt1 bits needed to support
reactors and commands.

EDIT3:

Refactoring of linker logic fixes #8886.

cc @MasterQ32

kubkon · 2021-06-05T07:38:23Z

I'm making the PR into a draft PR since I'm not happy with the fact that we bundle emulations in wasi-libc proper. As far as I can work out, that should not be the case, and instead, it should be up to the user to explicitly request emulation libs via -l flags. For example:

/home/kubkon/dev/zig/build/lib/zig/libc/include/wasm-wasi-musl/signal.h:2:2: error: "wasm lacks signal support; to enable minimal signal emulation, compile with -D_WASI_EMULATED_SIGNAL and link with -lwasi-emulated-signal"
#error "wasm lacks signal support; to enable minimal signal emulation, \

I think I'd like us to match the hinted behaviour in the error message within zig itself.

Summoning @jedisct1 to offer his insight and opinion.

jedisct1 · 2021-06-05T10:39:18Z

These emulation libraries are very useful to port software to WASI.

And using -l flags would totally make sense to enable them.

andrewrk · 2021-06-08T22:42:08Z

+    /// * getpid
+    /// * mman
+    /// * signal
+    wasi_emulated_libs: []const []const u8 = &[0][]const u8{},


These should be enum items and not strings since the strings are checked by the CLI, right?

The strings correspond to the names of required emulated WASI libc subcomponents imported as libraries on the linker line, e.g., -lwasi-emulated-process-clocks would result in us saving wasi-emulated-process-clocks in wasi_emulated_libs slice. We could store a slice of enums too, but we still need to get the name of the matching pre-compiled lib using comp.get_libc_crt_file(arena, "libwasi-emulated-process-clocks.a"). I'm fine either way TBH, and if you feel that matching strings to enum variants in main.zig makes more sense, I'm happy to make the necessary tweaks.

Done in 2ee1f78.

andrewrk · 2021-06-08T22:42:57Z

-        if (comp.wantBuildWasiLibcSysrootFromSource()) {
-            try comp.work_queue.write(&[_]Job{.{ .wasi_libc_sysroot = {} }});
+        if (comp.wantBuildWasiLibcFromSource()) {
+            try comp.work_queue.ensureUnusedCapacity(6); // worst-case we need all components


Suggested change

try comp.work_queue.ensureUnusedCapacity(6); // worst-case we need all components

try comp.work_queue.ensureUnusedCapacity(wasi_emulated_libs.len + 2);

Done in 2ee1f78.

andrewrk · 2021-06-08T22:44:15Z

+    if (comp.getTarget().os.tag == .wasi and mem.eql(u8, "wasi_snapshot_preview1", lib_name)) {
+        // Any referenced symbol from this lib, will be undefined until
+        // runtime as this lib is provided directly by the runtime.
+        return;
+    }


I don't quite understand why this is here. The comment doesn't make sense to me because that is true about all dynamically linked libraries. What makes wasi_snapshot_preview1 special?

wasi_snapshot_preview1 is actually not a dynamic library per se; it's a set of syscalls implicitly provided by the sandbox, therefore we cannot really link against it. There are two options here: we either remove any mention of wasi_snapshot_preview1 from the linker line (which will be there since we import syscalls directly into our libstd), or disable linking dynamically by wasm-ld (which was the case until now). I'm happy either way, and if you think we should preserve the status quo, I'll delete this bit, and remove any mention of dynamic linking from wasm-ld wrapper in Wasm.zig.

I think this logic would make more sense to me if it was in link/Wasm.zig which sounds like one of your suggestions 👍

Oh, actually. I'll move this logic into link/Wasm.zig but I cannot get rid of the logic which unpacks system_libs since we are allowed to pass static libs via -l flags.

Done in 2ee1f78.

This commit includes emulated libc sublibs that were not included in the compilation and caching of WASI libc that ships with Zig. The libs include (emulated): process clocks, getpid, mman, and signal. With this change, it is now possible to successfully cross-compile `wasm3` engine to WASI with `zig cc`. For the future though, it might be worth considering splitting WASI libc into libc-proper and modularised emulated libs as it is done in upstream, and then have them included only if the user specifically requests emulation/parts of it.

This replicates the expected behavior when using `clang` with upstream `wasi-libc` sysroot: linking emulated subcomponents such as process clocks or signals requires an explicit link flag in the compiler invocation, for example: ``` zig cc -target wasm32-wasi -lwasi-emulated-process-clocks main.c -o main.wasm ```

Move parsing of system libs into `main.zig` next to where we decide if we should link libC, and, if targeting WASI, if the specified libname equals one of the emulated components, save it on the side and remove it from the system libs. Then, build *only* those parts of WASI libc that were preserved in the previous step. This also fixes building of different crt1 bits needed to support reactors and commands.

Do not try to link WASI libc or emulated subcomponents when not targeting WASI; e.g., when targeting `wasm32-freestanding`.

* then, in `link/Wasm.zig` map `CRTFile` to full emulated libs name * move logic for removing any mention of WASI snapshot `wasi_snapshot_preview1` from `Compilation.zig` into `link/Wasm.zig`

kubkon added arch-wasm 32-bit and 64-bit WebAssembly os-wasi WebAssembly System Interface zig cc Zig as a drop-in C compiler feature labels Jun 4, 2021

kubkon added this to the 0.8.1 milestone Jun 4, 2021

kubkon added bug Observed behavior contradicts documented or intended behavior enhancement Solving this issue will likely involve adding new logic or components to the codebase. and removed bug Observed behavior contradicts documented or intended behavior labels Jun 4, 2021

kubkon modified the milestones: 0.8.1, 0.9.0 Jun 4, 2021

kubkon marked this pull request as draft June 5, 2021 07:38

kubkon changed the title ~~zig,cc,wasi: include emulated libs in WASI libc~~ cc,wasi: include emulated libs in WASI libc Jun 5, 2021

kubkon force-pushed the cc-wasm32-wasi-emulations branch from 2830b77 to a533e02 Compare June 7, 2021 06:40

kubkon marked this pull request as ready for review June 7, 2021 06:45

kubkon requested review from andrewrk and jedisct1 June 7, 2021 06:46

This was referenced Jun 7, 2021

Add build.zig to qjs-wasi project saghul/wasi-lab#8

Open

cc,wasi: support WASI reactors via -mexec-model flag #8978

Closed

kubkon force-pushed the cc-wasm32-wasi-emulations branch from ff254d4 to 910b6f4 Compare June 7, 2021 15:05

kubkon changed the title ~~cc,wasi: include emulated libs in WASI libc~~ cc,wasi: include emulated libs in WASI libc and refactor WASI libc handling Jun 7, 2021

kubkon mentioned this pull request Jun 8, 2021

Add minimal build.zig build script wasm3/wasm3#247

Merged

andrewrk requested changes Jun 8, 2021

View reviewed changes

kubkon added 6 commits June 9, 2021 01:25

wasi: skip adding wasi_snapshot_preview1 as lib dep in stage1

95745f7

wasi: clean up linking logic

dd84ccd

Do not try to link WASI libc or emulated subcomponents when not targeting WASI; e.g., when targeting `wasm32-freestanding`.

cc,wasi: store CRTFile enum in wasi_emulated_libs

2ee1f78

* then, in `link/Wasm.zig` map `CRTFile` to full emulated libs name * move logic for removing any mention of WASI snapshot `wasi_snapshot_preview1` from `Compilation.zig` into `link/Wasm.zig`

kubkon force-pushed the cc-wasm32-wasi-emulations branch from 910b6f4 to 2ee1f78 Compare June 8, 2021 23:26

kubkon requested a review from andrewrk June 8, 2021 23:27

andrewrk approved these changes Jun 8, 2021

View reviewed changes

kubkon merged commit a6ae5a7 into master Jun 9, 2021

kubkon deleted the cc-wasm32-wasi-emulations branch June 9, 2021 07:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

cc,wasi: include emulated libs in WASI libc and refactor WASI libc handling#8992

cc,wasi: include emulated libs in WASI libc and refactor WASI libc handling#8992
kubkon merged 6 commits intomasterfrom
cc-wasm32-wasi-emulations

kubkon commented Jun 4, 2021 •

edited

Loading

Uh oh!

kubkon commented Jun 5, 2021

Uh oh!

jedisct1 commented Jun 5, 2021

Uh oh!

andrewrk Jun 8, 2021

Uh oh!

kubkon Jun 8, 2021

Uh oh!

kubkon Jun 8, 2021

Uh oh!

andrewrk Jun 8, 2021

Uh oh!

kubkon Jun 8, 2021

Uh oh!

andrewrk Jun 8, 2021

Uh oh!

kubkon Jun 8, 2021

Uh oh!

andrewrk Jun 8, 2021

Uh oh!

kubkon Jun 8, 2021

Uh oh!

kubkon Jun 8, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	try comp.work_queue.ensureUnusedCapacity(6); // worst-case we need all components
	try comp.work_queue.ensureUnusedCapacity(wasi_emulated_libs.len + 2);

Uh oh!

Conversation

kubkon commented Jun 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kubkon commented Jun 5, 2021

Uh oh!

jedisct1 commented Jun 5, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kubkon commented Jun 4, 2021 •

edited

Loading