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.

Sign up for GitHub

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

Jump to bottom

libcxx's <fstream> no longer works as of wasi-sdk 21 #373

Closed
dicej opened this issue Jan 9, 2024 · 4 comments · Fixed by #375
Closed

libcxx's <fstream> no longer works as of wasi-sdk 21 #373

dicej opened this issue Jan 9, 2024 · 4 comments · Fixed by #375

Comments

@dicej
Copy link
Contributor

dicej commented Jan 9, 2024

As of LLVM 17, which includes https://reviews.llvm.org/D152168, libcxx has combined the old _LIBCPP_HAS_NO_FILESYSTEM_LIBRARY and _LIBCPP_HAS_NO_FSTREAM preprocessor symbols into a single _LIBCPP_HAS_NO_FILESYSTEM symbol, which means there's no longer any way to enable <fstream> without enabling <filesystem>. Consequently, wasi-sdk 21 no longer supports e.g. std::ifstream etc.

The solution may involve setting -DLIBCXX_ENABLE_FILESYSTEM:BOOL=ON and then fixing the breakage. I just tried that and hit fatal error: 'sys/statvfs.h' file not found. I'm going to try creating that file and adding stubs to it to see how far I get. Meanwhile, I'm open to alternative approaches.
@sbc100
Copy link
Member

sbc100 commented Jan 9, 2024

Probably best to modify libc++ upstream to not depend on statvfs.h when building for wasi.

@dicej
Copy link
Contributor Author

dicej commented Jan 9, 2024

@sbc100 it's ugly, but it works: dicej/llvm-project@4fe4740

Note that it wasn't just statvfs, but also realpath, chmod, etc.

Does that seem appropriate to submit as an upstream PR?

dicej added a commit to dicej/wasi-sdk that referenced this issue Jan 9, 2024
@dicej
address WebAssembly#373
633420a 
Signed-off-by: Joel Dice <[email protected]>
@sunfishcode
Copy link
Member

I just had an idea: I expect WASI will be adding chmod and realpath in the future; statvfs is trickier, but perhaps WASI could provide at least some of the fields in the future. So, what if for now, we add stubs to wasi-libc for chmod, realpath, and statvfs, which all fail with ENOSYS? That should be enough to fix the libcxx build errors, and then we can fill in the implementations as WASI adds the underlying support.

Does that sound reasonable?

dicej added a commit to dicej/wasi-libc that referenced this issue Jan 10, 2024
@dicej
add stubs for statvfs, chmod, etc.
9290577 
Per WebAssembly/wasi-sdk#373, LLVM's libc++ no longer
allows us to enable `<fstream>` and `<filesystem>` separately -- it's both or
neither.  Consequently, we either need to patch libc++ to not use `statvfs`,
`chmod`, etc. or add stub functions for those features to `wasi-libc`.  Since
we're planning to eventually support those features with WASI Preview 2 and
beyond, it makes sense to do the latter.

Note that since libc++ uses `DT_SOCK`, I've added a definition for it -- even
though WASI Preview 1 does not define it.  No Preview 1 file will ever have that
type, so code that handles that type will never be reached, but defining it
allows us to avoid WASI-specific patches to libc++.

Related to `DT_SOCK`, I had to change the `S_IFIFO` value so it does not
conflict with `S_IFSOCK`, thereby avoiding ambiguity in `__wasilibc_iftodt`.

Signed-off-by: Joel Dice <[email protected]>
@dicej dicej mentioned this issue Jan 10, 2024
Merged
sunfishcode pushed a commit to WebAssembly/wasi-libc that referenced this issue Jan 11, 2024
@dicej
add stubs for statvfs, chmod, etc. (#463)
cc62fa8 
Per WebAssembly/wasi-sdk#373, LLVM's libc++ no longer
allows us to enable `<fstream>` and `<filesystem>` separately -- it's both or
neither.  Consequently, we either need to patch libc++ to not use `statvfs`,
`chmod`, etc. or add stub functions for those features to `wasi-libc`.  Since
we're planning to eventually support those features with WASI Preview 2 and
beyond, it makes sense to do the latter.

Note that since libc++ uses `DT_SOCK`, I've added a definition for it -- even
though WASI Preview 1 does not define it.  No Preview 1 file will ever have that
type, so code that handles that type will never be reached, but defining it
allows us to avoid WASI-specific patches to libc++.

Related to `DT_SOCK`, I had to change the `S_IFIFO` value so it does not
conflict with `S_IFSOCK`, thereby avoiding ambiguity in `__wasilibc_iftodt`.

Signed-off-by: Joel Dice <[email protected]>
@dicej
Copy link
Contributor Author

dicej commented Jan 11, 2024

This is fixed by WebAssembly/wasi-libc#463 -- just need to update the wasi-libc submodule in this repo to pull it in.

dicej added a commit to dicej/wasi-sdk that referenced this issue Jan 11, 2024
@dicej
update wasi-libc and Makefile to fix <fstream>
a6af752 
Fixes WebAssembly#373

Signed-off-by: Joel Dice <[email protected]>
dicej added a commit to dicej/wasi-sdk that referenced this issue Jan 11, 2024
@dicej
update wasi-libc and Makefile to fix <fstream>
964191c 
As of LLVM 17, which includes https://reviews.llvm.org/D152168, libcxx has
combined the old `_LIBCPP_HAS_NO_FILESYSTEM_LIBRARY` and
`_LIBCPP_HAS_NO_FSTREAM` preprocessor symbols into a single
`_LIBCPP_HAS_NO_FILESYSTEM` symbol, which means there's no longer any way to
enable `<fstream>` without enabling `<filesystem>`.

The solution is to set `-DLIBCXX_ENABLE_FILESYSTEM:BOOL=ON` and update
`wasi-libc`, which includes stubs for the functions required by libcxx's
`<filesystem>` implementation.

Fixes WebAssembly#373

Signed-off-by: Joel Dice <[email protected]>
@dicej dicej mentioned this issue Jan 11, 2024
Merged
abrown pushed a commit that referenced this issue Jan 12, 2024
@dicej
update wasi-libc and Makefile to fix <fstream> (#375)
9fbc7b5 
As of LLVM 17, which includes https://reviews.llvm.org/D152168, libcxx has
combined the old `_LIBCPP_HAS_NO_FILESYSTEM_LIBRARY` and
`_LIBCPP_HAS_NO_FSTREAM` preprocessor symbols into a single
`_LIBCPP_HAS_NO_FILESYSTEM` symbol, which means there's no longer any way to
enable `<fstream>` without enabling `<filesystem>`.

The solution is to set `-DLIBCXX_ENABLE_FILESYSTEM:BOOL=ON` and update
`wasi-libc`, which includes stubs for the functions required by libcxx's
`<filesystem>` implementation.

Fixes #373

Signed-off-by: Joel Dice <[email protected]>
@pmp-p pmp-p mentioned this issue Jan 24, 2024
Closed
@dicej dicej mentioned this issue Feb 13, 2024
Merged
@pmp-p pmp-p mentioned this issue Mar 7, 2024
Closed
@dicej dicej mentioned this issue Jul 16, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

update wasi-libc and Makefile to fix <fstream> dicej/wasi-sdk
3 participants
@sbc100 @dicej @sunfishcode