This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] WASI support for libcxx
ClosedPublic

Authored by sunfish on Apr 30 2019, 1:10 PM.

Details

Summary

This adds explicit support for the WASI platform to libcxx. Notes:

  • WASI libc uses some components from musl, however it's not fully compatible with musl, and this patch will allow us to stop using _LIBCPP_HAS_MUSL_LIBC and customize for WASI libc specifically.
  • in src/random.cpp, getentropy is declared in <unistd.h> on all platforms I've checked (such as OpenBSD which had it first) except Darwin.

Diff Detail

Repository
rCXX libc++

Event Timeline

sunfish created this revision.Apr 30 2019, 1:10 PM
sbc100 accepted this revision.Apr 30 2019, 1:53 PM
sbc100 added inline comments.
src/thread.cpp
28

Does WASI not define __CloudABI__?

This revision is now accepted and ready to land.Apr 30 2019, 1:53 PM
ldionne added inline comments.Apr 30 2019, 2:27 PM
src/random.cpp
28 ↗(On Diff #197423)

If Fuchsia also needs this change, how come is the code working right now? I'm trying to understand the reason for this change.

sunfish marked an inline comment as done.Apr 30 2019, 3:36 PM
sunfish added inline comments.
src/random.cpp
28 ↗(On Diff #197423)

Some systems declare getentropy in both sys/random.h and unistd.h (eg. GLIBC), some just in unistd.h (eg. musl), and some just in sys/random.h (eg. Darwin). Perhaps Fuchsia is in the last category -- just in sys/random.h.

It looks like a better solution here is to just make WASI libc declare getentropy in its sys/random.h, so that we don't need to change the code here. I'll update the patch here accordingly.

sunfish updated this revision to Diff 197470.Apr 30 2019, 3:44 PM

Drop the getentropy header file change from this patch.

sunfish marked an inline comment as done.Apr 30 2019, 3:58 PM
sunfish added inline comments.
src/thread.cpp
28

No, WASI doesn't define CloudABI. It's derived from CloudABI, but contains numerous differences, so we felt it was best to not claim to be CloudABI to avoid confusion.

ldionne accepted this revision.May 1 2019, 9:31 AM

I don't have any objection about this patch (but I also can't test that it works).

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 1 2019, 9:45 AM