Stream: git-wasmtime

Topic: wasmtime / issue #4453 cranelift: Use `cranelift-jit` in ...


view this post on Zulip Wasmtime GitHub notifications bot (Jul 15 2022 at 14:37):

github-actions[bot] commented on issue #4453:

Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "fuzzing"

Thus the following users have been cc'd because of the following labels:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Aug 08 2022 at 20:24):

afonso360 commented on issue #4453:

With the CRT issue resolved I think this is ready to merge.

The only changes since the last review were a rebase and enabling the FMA tests that we couldn't enable in #4460.

Thanks for reviewing!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 09 2022 at 20:38):

afonso360 commented on issue #4453:

@jameysharp I agree with your comments, however this is sort of an intermediate PR with the follow up cleaning pretty much all of it.

See #4667, we do away with SingleFunctionCompiler and link the entire test file at once, so the repeated compilation issue goes away.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 09 2022 at 20:39):

afonso360 edited a comment on issue #4453:

@jameysharp I agree with your comments, however this is sort of an intermediate PR with the follow up cleaning pretty much all of it.

See #4667, we do away with SingleFunctionCompiler and link the entire test file at once, so the repeated compilation issue goes away.

I'd prefer not to have to clean this up, since it's going away as soon as that is done. But let me know if this is a blocker.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 09 2022 at 21:55):

jameysharp commented on issue #4453:

I hadn't looked at 4667 yet, and that resolves my concerns. Thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2022 at 15:59):

uweigand commented on issue #4453:

As of this commit, I'm now seeing test failures on s390x (Ubuntu 20.04), specifically when running cargo test --doc in the filetests directory. The symptom is:

Test executable failed (exit status: 101).
stderr:
thread 'main' panicked at 'unable to make memory readable+executable: SystemCall(Os { code: 13, kind: PermissionDenied, message: "Permission denied" })', cranelift/jit/src/memory.rs:178:30

when running the sample code in src/function_runner.rs. A backtrace shows that the failing call is:

#0  0x000003fffdc8cce0 in mprotect () from /lib64/libc.so.6
#1  0x000002aa0008ee96 in region::os::unix::set_protection (base=0x2aa00847000, size=4096, protection=...) at src/os/unix.rs:30
#2  0x000002aa0008e880 in region::protect::protect (address=0x2aa00847000, size=4096, protection=...) at src/protect.rs:48
#3  0x000002aa00072f6e in cranelift_jit::memory::{impl#2}::set_readable_and_executable::{closure#0} (ptr=0x2aa00847000, len=4096) at cranelift/jit/src/memory.rs:211
#4  0x000002aa00086824 in cranelift_jit::memory::Memory::set_readable_and_executable (self=0x3ffffffd2d0) at cranelift/jit/src/memory.rs:221
#5  0x000002aa00068332 in cranelift_jit::backend::JITModule::finalize_definitions (self=0x3ffffffd260) at cranelift/jit/src/backend.rs:461
#6  0x000002aa00045ae0 in cranelift_filetests::function_runner::TestFileCompiler::compile (self=<error reading variable: value has been optimized out>)
    at cranelift/filetests/src/function_runner.rs:247

Note that this attempts to use mprotect to grant execute permission to memory allocated on the heap. This is not supported everywhere, and will in particular be prevented by various security hardening settings on Linux.

Instead, you should use separate mmap allocations if you need to grant execute permissions.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2022 at 16:59):

afonso360 commented on issue #4453:

:wave: Hey,

Do the regular filetests pass for you on main? We still use mprotect, so I'm surprised as to how we haven't detected this sooner.

cranelift-jit has the selinux-fix feature that switches to mmap based allocations, if you try to use that, does that make it pass?

If so, that might be related to: https://github.com/bytecodealliance/wasmtime/issues/4986

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2022 at 17:00):

afonso360 edited a comment on issue #4453:

:wave: Hey,

Do the regular filetests pass for you on main? We still use mprotect, so I'm surprised as to how we haven't detected this sooner.

cranelift-jit has the selinux-fix feature that switches to mmap based allocations, if you try to use that, does that make it pass?

If so, this might be related to: https://github.com/bytecodealliance/wasmtime/issues/4986

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2022 at 17:06):

afonso360 edited a comment on issue #4453:

:wave: Hey,

~Do the regular filetests pass for you on main? We still use mprotect, so I'm surprised as to how we haven't detected this sooner.~

Edit: Ignore that I misread your comment and thought that only the doc tests were failing.

cranelift-jit has the selinux-fix feature that switches to mmap based allocations, if you try to use that, does that make it pass?

If so, this might be related to: https://github.com/bytecodealliance/wasmtime/issues/4986

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2022 at 17:07):

afonso360 edited a comment on issue #4453:

:wave: Hey,

~Do the regular filetests pass for you on main? We still use mprotect, so I'm surprised as to how we haven't detected this sooner.~

Edit: Ignore that, I misread your comment and thought that only the doc tests were failing.

cranelift-jit has the selinux-fix feature that switches to mmap based allocations, if you try to use that, does that make it pass?

If so, this might be related to: https://github.com/bytecodealliance/wasmtime/issues/4986

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2022 at 17:27):

uweigand commented on issue #4453:

No, only the doc tests are indeed failing. I've been debugging this right now, and the difference is that doc test sample program is single-threaded and uses the main heap, where mprotect PROT_EXEC fails. However, when running the main filetests, the clif-util program is multi-threaded and glibc uses per-thread heaps, which are separately created via mmap, and therefore the mprotect happens to work ...

How would I use the selinux-fix feature with a doc test?

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2022 at 17:38):

afonso360 commented on issue #4453:

No, only the doc tests are indeed failing. I've been debugging this right now, and the difference is that doc test sample program is single-threaded and uses the main heap, where mprotect PROT_EXEC fails. However, when running the main filetests, the clif-util program is multi-threaded and glibc uses per-thread heaps, which are separately created via mmap, and therefore the mprotect happens to work ...

That's interesting, and weird!

How would I use the selinux-fix feature with a doc test?

This should do it.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2022 at 17:39):

afonso360 edited a comment on issue #4453:

No, only the doc tests are indeed failing. I've been debugging this right now, and the difference is that doc test sample program is single-threaded and uses the main heap, where mprotect PROT_EXEC fails. However, when running the main filetests, the clif-util program is multi-threaded and glibc uses per-thread heaps, which are separately created via mmap, and therefore the mprotect happens to work ...

That's interesting, and weird!

How would I use the selinux-fix feature with a doc test?

This should do it.

However I think that we probably should switch to a mmap allocator since that apparently can solve a bunch of other issues with cranelift-jit.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2022 at 17:59):

uweigand commented on issue #4453:

How would I use the selinux-fix feature with a doc test?

This should do it.

This fixes the problem for me. Submitted as https://github.com/bytecodealliance/wasmtime/pull/5204

However I think that we probably should switch to a mmap allocator since that apparently can solve a bunch of other issues with cranelift-jit.

Agreed!


Last updated: Oct 23 2024 at 20:03 UTC