libc icon indicating copy to clipboard operation
libc copied to clipboard

Support for Wasm32 Linux Target

Open arjunr2 opened this issue 1 year ago • 2 comments

  • [ ] Edit corresponding file(s) under libc-test/semver when you add/remove item(s), e.g. edit linux.txt if you add an item to src/unix/linux_like/linux/mod.rs
  • [ ] Your PR doesn't contain any private or unstable values like *LAST or *MAX (see #3131)
  • [ ] If your PR has a breaking change, please clarify it
  • [ ] If your PR increments version number, it must NOT contain any other changes (otherwise a release could be delayed)
  • [ ] Make sure ci/style.sh passes
  • [ ] cd libc-test && cargo test
    • (this might fail on your env due to environment difference between your env and CI. Ignore failures if you are not sure)

arjunr2 avatar Jul 19 '24 21:07 arjunr2

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @JohnTitor (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

rustbot avatar Jul 19 '24 21:07 rustbot

Notes about the new target wasm32-linux-musl, added in this rustc fork to implement a Wasm Linux Interface

  • Patching of cc, ctest, and libtest are still under progress for this target but should be ready soon. It doesn't impact any other existing linux or wasm target
  • Without libtest, the harness=true tests don't run, but semver passes

arjunr2 avatar Jul 19 '24 21:07 arjunr2

Is this still a work in progress? There are still failing tests.

This should target main rather than libc-0.2. It seems like that may have been the intent anyway, considering the diff is large.

@rustbot author

tgross35 avatar Oct 15 '24 02:10 tgross35

As date of initiating this PR, main was unable to build the rust compiler and I was pointed to libc-0.2, which seemed to work for the compiler. It does make sense for this to target main long term though.

Failing tests seem to emerge from using features like c_variadic that aren't stable yet. What is the suggested approach to deal with this?

arjunr2 avatar Oct 15 '24 04:10 arjunr2

All in all I'm a little confused about the goal of this PR. Is wasm32-linux-musl intended for upstreaming in rustc? I don't think we can expect to support anything in libc without rustc support, usually that is the first step for a new target.

If we do have something to add here, it should target main and definitely needs to be shrunk down to the bare minimum for the first PR (e.g. just read and write) and then grown later.

Cc @alexcrichton per usual for anything wasm related

tgross35 avatar Oct 15 '24 04:10 tgross35

It is intended for upstreaming. This fork of rustc supports the target.

I'm also generally confused about the process since rustc itself depends on a patched libc crate to build. How does one pass CI tests and get changes upstreamed in the presence of cyclic dependencies? If you could let me know the order of operations, I'd follow that.

Thanks

arjunr2 avatar Oct 15 '24 12:10 arjunr2

Process-wise what I might recommend (broadly for this target as opposed to this specific PR) would be:

  • File an MCP about adding this new target to the Rust compiler, probably as Tier 3 first.
  • Land this target's definition in the Rust compiler itself. The target won't be very usable, but it should probably be able to build libcore/liballoc at this point (neither use libc)
  • Locally build the target using a [patch] to point to a fork of libc. Use this PR to get tests and such working.
  • Merge this PR here to libc itself without tests/CI (since the target doesn't built in rustc yet, and tier 3 targets generally aren't tested)
  • Get the libc update back into rustc, now you should in theory be able to build libstd.
  • Start working, probably with -Zbuild-std on getting more tests/support working in various crates.

Put another way to answer this:

How does one pass CI tests and get changes upstreamed in the presence of cyclic dependencies?

The answer I think is generally "you don't" in the sense that new and/or tier 3 targets generally don't have tests. It takes a bit to bootstrap the target into a workable state.

I'd also recommend going with what @tgross35 was mentioning by keeping this PR minimal to start out. This is definitely one where it'd be useful to have the CI checks all enabled for this target but that'll involve toolchain and CI wrangling. That'll ideally be in place before adding all of the target-specific bits and will serve as a good double-check that all the reused definitions here are correct too.

alexcrichton avatar Oct 16 '24 02:10 alexcrichton

Alex covered most of this, but basically a MCP followed by target support merged into rustc checks the "the Rust ecosystem would like to support this" box, after that happens it can start propagating out. rustc can build for the target without anything in std.

After that happens, you can start to add support in libc chunk by chunk as well as in std, leaving unimplemented!() anywhere that libc doesn't support something. It will take multiple PRs in both repos to ratchet up to something that works well, but that is preferable over a single huge PR that is much harder to review.

tgross35 avatar Oct 16 '24 06:10 tgross35

Awesome, thanks @tgross35, @alexcrichton. Will split this PR into multiple chunks and get rid of testing as a tier-3 target

arjunr2 avatar Oct 16 '24 16:10 arjunr2

:umbrella: The latest upstream changes (presumably #4110) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Nov 19 '24 05:11 bors

This makes sense. Whether the additions should go into either b32 or b64 can be discussed. I currently did b64 since the conventions for most records for the wali ABI follows the x64 convention.

As for the syscall shim layer, the issue is that I'm unsure how many packages invoke the raw syscall method from musl-libc. There were several cases in std, which could be easily patched, but if a number of crates tap directly into this symbol from musl, having libc stub them out transparently seems like the most smooth and adds ease for targetability.

arjunr2 avatar Nov 20 '24 01:11 arjunr2

On a side note, I think it would actually be good to standardize the syscall shim across all targets as well. Implicit mismatching of types with expected values from C variadics is a nasty loophole that is not well defined. It would do good (especially for any future targets with virtual syscall layers) to typecast accordingly.

arjunr2 avatar Nov 20 '24 01:11 arjunr2

New PR retargetting this to main

arjunr2 avatar Jan 16 '25 19:01 arjunr2