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

Update wasi-libc to d03829489904d38c624f6de9983190f1e5e7c9c5 #19614

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

jedisct1
Copy link
Contributor

Most notable improvements include realpath and stubs for statvfs, chmod, etc.

Other changes are for WASI 0.2 which is going to be painful to support since 3rd party tools written in Rust may become necessary in addition to LLVM (WebAssembly/wasi-sdk#402)

@jedisct1
Copy link
Contributor Author

This also removes junk we were shipping but irrelevant to webassembly.

jedisct1 and others added 3 commits April 11, 2024 23:59
* master:
  translate-c: allow str literals in bool expressions
  std.debug.panic: pass the args
  fetch: combine unpack error validation in a single function
  std.Build: revert the breaking changes only
  remove deprecated LazyPath.path union tag
@lin72h
Copy link

lin72h commented Apr 12, 2024

10KLoc reduce, feel so good

* master: (29 commits)
  cmake: support setting the dynamic linker
  std.Build: revert --host-target, --host-cpu, --host-dynamic-linker
  Sema: cap depth of value printing in type names
  Sema: correctly make inferred allocs constant
  std.zig.system.linux: Use arm.zig detection for AArch64 CPU features
  Value: convert undefined values to 0xAA for bitwise operations
  Value: fix out-of-bounds slice access writing zero-bit undef value
  compiler: un-implement ziglang#19634
  test: disable newly failing test
  compiler: rework comptime pointer representation and access
  x86_64: fix miscompilation regression in package fetching code
  std: improve std.once tests
  lib/std/Build/Step/CheckObject: dump section as string
  zig init: Replace deprecated LazyPath.path with Build.path()
  std.Build.Step.ConfigHeader: better error message
  windows_argv standalone test: Only test against MSVC if it's available
  ArgIteratorWindows: Match post-2008 C runtime rather than CommandLineToArgvW
  fix namespacing of std.fs.Dir.Walker.Entry
  std.fs.Dir.Walker: maintain a null byte in path names
  wasi: change default os version to `0.1.0`
  ...
@jedisct1
Copy link
Contributor Author

Added the wasi-libc libdl shim, so that tools such as py2wasm can use zig cc.

@andrewrk
Copy link
Member

andrewrk commented Jun 8, 2024

man. wasi-libc kinda sucks. how about we provide libc via zig code instead?

@jedisct1
Copy link
Contributor Author

jedisct1 commented Jun 8, 2024

wasi-libc is horrible. And it's getting worse with the wasi preview 2 bits.

But rewriting it in Zig would be a lot of work.

@@ -2748,7 +2748,7 @@ pub fn is_libc_lib_name(target: std.Target, name: []const u8) bool {
return true;
if (eqlIgnoreCase(ignore_case, name, "resolv"))
return true;
if (eqlIgnoreCase(ignore_case, name, "dl"))
if (eqlIgnoreCase(ignore_case, name, "dl") and !target.isWasm())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the libdl shim, isn't this wrong?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would pick the wrong libdl.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, we have a special case for WASI here:

zig/src/main.zig

Lines 3766 to 3821 in c96f9a0

for (create_module.system_libs.keys(), create_module.system_libs.values()) |lib_name, info| {
if (target.is_libc_lib_name(lib_name)) {
create_module.opts.link_libc = true;
continue;
}
if (target.is_libcpp_lib_name(lib_name)) {
create_module.opts.link_libcpp = true;
continue;
}
switch (target_util.classifyCompilerRtLibName(target, lib_name)) {
.none => {},
.only_libunwind, .both => {
create_module.opts.link_libunwind = true;
continue;
},
.only_compiler_rt => {
warn("ignoring superfluous library '{s}': this dependency is fulfilled instead by compiler-rt which zig unconditionally provides", .{lib_name});
continue;
},
}
if (target.isMinGW()) {
const exists = mingw.libExists(arena, target, zig_lib_directory, lib_name) catch |err| {
fatal("failed to check zig installation for DLL import libs: {s}", .{
@errorName(err),
});
};
if (exists) {
try create_module.resolved_system_libs.append(arena, .{
.name = lib_name,
.lib = .{
.needed = true,
.weak = false,
.path = null,
},
});
continue;
}
}
if (fs.path.isAbsolute(lib_name)) {
fatal("cannot use absolute path as a system library: {s}", .{lib_name});
}
if (target.os.tag == .wasi) {
if (wasi_libc.getEmulatedLibCRTFile(lib_name)) |crt_file| {
try create_module.wasi_emulated_libs.append(arena, crt_file);
continue;
}
}
try external_system_libs.append(arena, .{
.name = lib_name,
.info = info,
});
}

Still seems not good that is_libc_lib_name() returns false for these since all other parts of the codebase (like std.Build) will get the wrong answer.

Can't we just make is_libc_lib_name() do the right thing, and move the special case in main.zig so it happens before the is_libc_lib_name() check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this is worth too much changes. The WASI dl library is just an awful hack to hopefully make some apps compile out of the box. But it only contains stub functions. None of them work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well the above should apply to the other emulated libraries as well - e.g. wasi-emulated-clocks, which seems a fair bit more useful than the silly dl shim. is_libc_lib_name() will currently say that wasi-emulated-clocks isn't a libc library name, misleading std.Build. This is why I'm suggesting we just make is_libc_lib_name() do what it's supposed to do and then move the code block in main.zig up a bit. Then, going forward, adding new library names will work just like it does for the other libcs we provide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants