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

Remove legacy fields from stat struct. NFC #19569

Merged
merged 1 commit into from Jun 8, 2023
Merged

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Jun 8, 2023

Musl needs these in order to support the linux kernel but we don't need to use the same ABI in emscripten.

Fixes: #19567

Musl needs these in order to support the linux kernel but we don't need
to use the same ABI in emscripten.

Fixes: #19567
@sbc100 sbc100 enabled auto-merge (squash) June 8, 2023 22:23
@sbc100 sbc100 disabled auto-merge June 8, 2023 23:19
@sbc100 sbc100 merged commit b182182 into main Jun 8, 2023
21 of 22 checks passed
@sbc100 sbc100 deleted the remove_struct_fields branch June 8, 2023 23:19
@kleisauke
Copy link
Collaborator

This PR broke compatibility with Rust.
https://github.com/rust-lang/libc/blob/8530e6fb1996f3ab41e587df2a2b29c5eaa32ec7/src/unix/linux_like/emscripten/mod.rs#L263-L264
https://github.com/rust-lang/libc/blob/8530e6fb1996f3ab41e587df2a2b29c5eaa32ec7/src/unix/linux_like/emscripten/mod.rs#L270

For example, performing a simple read would result in an OOM due to the changes in the stat struct (ABI mismatch).
https://github.com/rust-lang/rust/blob/e0ba2d038db3afa968e13075b1d6eabd24339708/library/std/src/fs.rs#L253-L254

Standalone reproducer:
src/main.rs:

use std::io;
use std::fs::File;

fn main() -> io::Result<()> {
    fs::read("hello.txt")?;
    Ok(())
}
$ curl https://sh.rustup.rs -sSf | sh -s -- -y --profile minimal --target wasm32-unknown-emscripten --default-toolchain nightly-2023-06-24 --component rust-src
$ source "$HOME/.cargo/env"
$ cargo new foo
$ cd foo
$ cat <<EOT > src/main.rs
use std::io;
use std::fs;

fn main() -> io::Result<()> {
    fs::read("hello.txt")?;
    Ok(())
}
EOT
$ RUSTFLAGS="-Clink-arg=-sNODERAWFS" cargo build --release -Zbuild-std=panic_abort,std --target wasm32-unknown-emscripten
   Compiling core v0.0.0 (...)
   Compiling libc v0.2.146
   ...
   Compiling foo v0.1.0 (...)
    Finished release [optimized] target(s) in 8.96s
$ echo "Hello, world!" > hello.txt
$ node target/wasm32-unknown-emscripten/release/foo.js
memory allocation of 1687607155 bytes failed
Aborted()
...
Node.js v20.3.0
$ echo $?
7

This was originally discovered on wasm-vips' test suite.

Details
memory allocation of 1687599246 bytes failed
Aborted(native code called abort())
  1) foreign
       svgload:
     RuntimeError: Aborted(native code called abort())
      at abort (file:///home/kleisauke/wasm-vips/lib/vips-node.mjs:567:10)
      at _abort (file:///home/kleisauke/wasm-vips/lib/vips-node.mjs:8180:2)
      at std::sys::unix::abort_internal::he27d3d15008e2a87 (wasm://wasm/0080a6de:wasm-function[2650]:0x16fa3e)
      at std::process::abort::h7669195b6a4c4363 (wasm://wasm/0080a6de:wasm-function[2649]:0x16fa34)
      at std::alloc::rust_oom::h027b9d2aafb59d5a (wasm://wasm/0080a6de:wasm-function[2740]:0x173dae)
      at __rg_oom (wasm://wasm/0080a6de:wasm-function[2741]:0x173dbc)
      at __rust_alloc_error_handler (wasm://wasm/0080a6de:wasm-function[447]:0x21703)
      at alloc::alloc::handle_alloc_error::rt_error::h7270c8e33f70b9e6 (wasm://wasm/0080a6de:wasm-function[2854]:0x17ab38)
      at alloc::alloc::handle_alloc_error::ha3344d8667c57d1d (wasm://wasm/0080a6de:wasm-function[2853]:0x17ab2a)
      at invoke_vii (file:///home/kleisauke/wasm-vips/lib/vips-node.mjs:16599:27)
      at std::fs::read::inner::hd4ced50fb263ad9f (wasm://wasm/0080a6de:wasm-function[2706]:0x172747)
      at resvg_parse_tree_from_file (wasm://wasm/0080a6de:wasm-function[365]:0x1935a)
      at vips_foreign_load_svg_file_header (wasm://wasm/0080a6de:wasm-function[224]:0x10b2a)
      at vips_foreign_load_build (wasm://wasm/019056d6:wasm-function[1217]:0xb9d68)
      at vips_object_build (wasm://wasm/019056d6:wasm-function[2579]:0x1ba0f9)
      at vips_cache_operation_buildp (wasm://wasm/019056d6:wasm-function[2744]:0x1c53f4)
      at vips::Image::call(char const*, char const*, vips::Option*, emscripten::val, vips::Image const*) (wasm://wasm/019056d6:wasm-function[177]:0x363bb)
      at invoke_viiiii (file:///home/kleisauke/wasm-vips/lib/vips-node.mjs:16621:27)
      at vips::Image::svgload(std::__2::basic_string<char, std::__2::char_traits<char>, std::__2::allocator<char>> const&, emscripten::val) (wasm://wasm/019056d6:wasm-function[291]:0x4b662)
      at invoke_viii (file:///home/kleisauke/wasm-vips/lib/vips-node.mjs:16511:27)
      at embind_init_my_module()::$_152::__invoke(std::__2::basic_string<char, std::__2::char_traits<char>, std::__2::allocator<char>> const&) (wasm://wasm/019056d6:wasm-function[862]:0xa090a)
      at invoke_vii (file:///home/kleisauke/wasm-vips/lib/vips-node.mjs:16599:27)
      at emscripten::internal::Invoker<vips::Interpolate, std::__2::basic_string<char, std::__2::char_traits<char>, std::__2::allocator<char>> const&>::invoke(vips::Interpolate (*)(std::__2::basic_string<char, std::__2::char_traits<char>, std::__2::allocator<char>> const&), emscripten::internal::BindingType<std::__2::basic_string<char, std::__2::char_traits<char>, std::__2::allocator<char>>, void>::StringStruct*) (wasm://wasm/019056d6:wasm-function[602]:0x8e47c)
      at Function.Image$svgload (eval at newFunc (file:///home/kleisauke/wasm-vips/lib/vips-node.mjs:6825:22), <anonymous>:8:10)
      at proto.<computed> [as svgload] (file:///home/kleisauke/wasm-vips/lib/vips-node.mjs:6495:61)
      at Object.svgload (file:///home/kleisauke/wasm-vips/test/unit/test_foreign.js:20:33)
      at fileLoader (file:///home/kleisauke/wasm-vips/test/unit/test_foreign.js:79:33)
      at Context.<anonymous> (file:///home/kleisauke/wasm-vips/test/unit/test_foreign.js:909:5)
      at process.processImmediate (node:internal/timers:478:21)

@sbc100
Copy link
Collaborator Author

sbc100 commented Jun 24, 2023

That sounds like something that should be fixed downstream in rust? Or am I misunderstanding?

@kleisauke
Copy link
Collaborator

Indeed, I just opened PR rust-lang/libc#3282 for this.

bors added a commit to rust-lang/libc that referenced this pull request Jun 25, 2023
bors added a commit to rust-lang/libc that referenced this pull request Jun 27, 2023
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.

None yet

4 participants