wasmtime / Issue #1931 Write ELF image and instantiate co... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / Issue #1931 Write ELF image and instantiate co...

Wasmtime GitHub notifications bot (Jun 26 2020 at 21:20):

yurydelendik commented on Issue #1931:

Items to verify:

[ ] AArch64

[ ] Windows unwind info

Wasmtime GitHub notifications bot (Jun 26 2020 at 21:53):

github-actions[bot] commented on Issue #1931:

Subscribe to Label Action

cc @bnjbvr, @peterhuene

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:wasm", "wasmtime:api"

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

bnjbvr: cranelift

peterhuene: wasmtime:api

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

Learn more.
</details>

Wasmtime GitHub notifications bot (Jun 27 2020 at 11:05):

tschneidereit commented on Issue #1931:

CC @pchickey, would be good to get your input on the general approach here.

There is somewhat common/duplicated code in cranelift-object and creates/obj, this PR is not refactoring these crates yet. It is a work for follow up PRs.

Can you give some details on why this should be done as a follow-up? I think it might be good to do this refactoring as part of the initial landing if possible.

Wasmtime GitHub notifications bot (Jun 29 2020 at 16:48):

yurydelendik commented on Issue #1931:

There is somewhat common/duplicated code in cranelift-object and creates/obj, this PR is not refactoring these crates yet. It is a work for follow up PRs.

Can you give some details on why this should be done as a follow-up? I think it might be good to do this refactoring as part of the initial landing if possible.

cranelift-object is more generic backend for creating (ELF) object files, and does not fully reflect what is needed for wasm atm. crates/obj is close to what we need though it emits information such as tables or data segments into object file (as well as supporting other formats than ELF).

Once we define how crates/obj and "wasm2obj" are used, we can merge/refactor its and this PR code to remove duplicated logic. I wonder who are the users of these crate and utility. @pchickey, are you using it these in some way?

Wasmtime GitHub notifications bot (Jun 29 2020 at 17:03):

yurydelendik commented on Issue #1931:

One of my main thoughts when reading this is about the various copies happening. To make sure I understand thing, can you double check this?

When generating code for the first time, we first invoke cranelift which emits function code into a buffer for each function.

Next we copy (?) each buffer into an object data structure to create an ELF image

Next we copy that ELF image into the final executable page residing in CodeMemory

That is about to be correct. Notice that we generated the ELF image for GDB JIT code, so for debug mode nothing is changed is term of copies.

I'm looking into making object to emit image directly into CodeMemory, where we can directly protect regions for execution. Alternatively, the last copy operation can be removed when the code becomes PIC, so we don't have to patch the image, so there is a potential to use mmap+fs, and compete with current caching logic.

Wasmtime GitHub notifications bot (Jun 29 2020 at 18:46):

yurydelendik edited a comment on Issue #1931:

There is somewhat common/duplicated code in cranelift-object and creates/obj, this PR is not refactoring these crates yet. It is a work for follow up PRs.

Can you give some details on why this should be done as a follow-up? I think it might be good to do this refactoring as part of the initial landing if possible.

cranelift-object is more generic backend for creating (ELF) object files, and does not fully reflect what is needed for wasm atm. crates/obj is close to what we need though it emits information such as tables or data segments into object file (as well as supporting other formats than ELF).

Once we define how crates/obj and "wasm2obj" are used, we can merge/refactor its and this PR code to remove duplicated logic. I wonder who are the users of these crate and utility. @pchickey, are you using it these in some unique way?

Wasmtime GitHub notifications bot (Jun 29 2020 at 22:28):

yurydelendik commented on Issue #1931:

[ ] check if https://github.com/gimli-rs/object/issues/240 will help with copy reduction

Wasmtime GitHub notifications bot (Jun 30 2020 at 14:48):

alexcrichton commented on Issue #1931:

Ok, it seems reasonable to go ahead with this in the meantime if more refactoring is required internally to get to the goal of "mmap and go". I'd want to make sure we don't paint ourselves into a corner where that's not possible, but it sounds like that's not the case.

Can you ad some high-level documentation somewhere for this, too? It'd be good I think to have a doc block describing how a wasm module is transformed into an ELF image, so whenever we read/write that we can refer back to those docs about what should go where.

Wasmtime GitHub notifications bot (Jul 01 2020 at 14:29):

yurydelendik commented on Issue #1931:

Items to verify: * AArch64

@cfallin could you verify if I refactored Arm64Call properly ? Not sure how to test it.

Windows unwind info

@peterhuene I think the Windows Unwind stuff where properly refactored and tests are passing. If you have time, can you glance the changes?

Wasmtime GitHub notifications bot (Jul 01 2020 at 15:03):

yurydelendik edited a comment on Issue #1931:

Items to verify: * AArch64

@cfallin could you verify if I refactored Arm64Call properly ? Not sure how to test it.

Windows unwind info

@peterhuene I think the Windows Unwind stuff were properly refactored and tests are passing. If you have time, can you glance the changes?

Wasmtime GitHub notifications bot (Jul 01 2020 at 19:48):

peterhuene commented on Issue #1931:

The unwind changes seem reasonable to me.

Wasmtime GitHub notifications bot (Jul 01 2020 at 19:50):

yurydelendik edited a comment on Issue #1931:

Items to verify:

[ ] AArch64

[x] Windows unwind info

Wasmtime GitHub notifications bot (Jul 06 2020 at 18:54):

yurydelendik edited a comment on Issue #1931:

Items to verify:

[x] AArch64

[x] Windows unwind info

Wasmtime GitHub notifications bot (Jul 06 2020 at 19:49):

yurydelendik commented on Issue #1931:

Information related no absolute relocations code discussion:

@sunfishcode @alexcrichton to observe relocations, I'm adding dump of the generated ELF at

diff --git a/crates/jit/src/compiler.rs b/crates/jit/src/compiler.rs
index 46280c120..96aed7a51 100644
--- a/crates/jit/src/compiler.rs
+++ b/crates/jit/src/compiler.rs
@@ -189,4 +189,7 @@ impl Compiler {
         })?;

+        let mut file = ::std::fs::File::create(::std::path::Path::new("test.o")).expect("file");
+        ::std::io::Write::write_all(&mut file, &obj).expect("write");
+
         Ok(Compilation {
             obj,

I after cargo run -- tests/all/debug/testsuite/reverse-str.wasm && gobjdump -xD test.o the following can be observed:

...
    16b:       48 b8 00 00 00 00 00    movabs $0x0,%rax
     172:       00 00 00
                        16d: R_X86_64_64        _wasm_function_0
     175:       49 89 fe                mov    %rdi,%r14
     178:       4c 89 ef                mov    %r13,%rdi
     17b:       4c 89 f6                mov    %r14,%rsi
     17e:       44 89 fa                mov    %r15d,%edx
     181:       40 ff d0                rex callq *%rax
     184:       44 8b bc 24 0c 00 00    mov    0xc(%rsp),%r15d
...

Wasmtime GitHub notifications bot (Jul 06 2020 at 20:10):

yurydelendik edited a comment on Issue #1931:

Information related to no absolute relocations code discussion:

@sunfishcode @alexcrichton to observe relocations, I'm adding dump of the generated ELF at

diff --git a/crates/jit/src/compiler.rs b/crates/jit/src/compiler.rs
index 46280c120..96aed7a51 100644
--- a/crates/jit/src/compiler.rs
+++ b/crates/jit/src/compiler.rs
@@ -189,4 +189,7 @@ impl Compiler {
         })?;

+        let mut file = ::std::fs::File::create(::std::path::Path::new("test.o")).expect("file");
+        ::std::io::Write::write_all(&mut file, &obj).expect("write");
+
         Ok(Compilation {
             obj,

I after cargo run -- tests/all/debug/testsuite/reverse-str.wasm && gobjdump -xD test.o the following can be observed:

...
    16b:       48 b8 00 00 00 00 00    movabs $0x0,%rax
     172:       00 00 00
                        16d: R_X86_64_64        _wasm_function_0
     175:       49 89 fe                mov    %rdi,%r14
     178:       4c 89 ef                mov    %r13,%rdi
     17b:       4c 89 f6                mov    %r14,%rsi
     17e:       44 89 fa                mov    %r15d,%edx
     181:       40 ff d0                rex callq *%rax
     184:       44 8b bc 24 0c 00 00    mov    0xc(%rsp),%r15d
...

Wasmtime GitHub notifications bot (Jul 07 2020 at 15:43):

yurydelendik edited a comment on Issue #1931:

Information related to no absolute relocations code discussion:

@sunfishcode @alexcrichton to observe relocations, I'm adding dump of the generated ELF at

diff --git a/crates/jit/src/compiler.rs b/crates/jit/src/compiler.rs
index 46280c120..96aed7a51 100644
--- a/crates/jit/src/compiler.rs
+++ b/crates/jit/src/compiler.rs
@@ -189,4 +189,7 @@ impl Compiler {
         })?;

+        let mut file = ::std::fs::File::create(::std::path::Path::new("test.o")).expect("file");
+        ::std::io::Write::write_all(&mut file, &obj).expect("write");
+
         Ok(Compilation {
             obj,

And after cargo run -- tests/all/debug/testsuite/reverse-str.wasm && gobjdump -xD test.o the following can be observed:

...
    16b:       48 b8 00 00 00 00 00    movabs $0x0,%rax
     172:       00 00 00
                        16d: R_X86_64_64        _wasm_function_0
     175:       49 89 fe                mov    %rdi,%r14
     178:       4c 89 ef                mov    %r13,%rdi
     17b:       4c 89 f6                mov    %r14,%rsi
     17e:       44 89 fa                mov    %r15d,%edx
     181:       40 ff d0                rex callq *%rax
     184:       44 8b bc 24 0c 00 00    mov    0xc(%rsp),%r15d
...

Last updated: Nov 22 2024 at 17:03 UTC