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

rustc: Add a #[wasm_custom_section] attribute #48883

Merged
merged 2 commits into from
Mar 23, 2018

Conversation

alexcrichton
Copy link
Member

This commit is an implementation of adding custom sections to wasm artifacts in
rustc. The intention here is to expose the ability of the wasm binary format to
contain custom sections with arbitrary user-defined data. Currently neither our
version of LLVM nor LLD supports this so the implementation is currently custom
to rustc itself.

The implementation here is to attach a #[wasm_custom_section = "foo"]
attribute to any const which has a type like [u8; N]. Other types of
constants aren't supported yet but may be added one day! This should hopefully
be enough to get off the ground with some custom section support.

The current semantics are that any constant tagged with #[wasm_custom_section]
section will be appended to the corresponding section in the final output wasm
artifact (and this affects dependencies linked in as well, not just the final
crate). This means that whatever is interpreting the contents must be able to
interpret binary-concatenated sections (or each constant needs to be in its own
custom section).

To test this change the existing run-make test suite was moved to a
run-make-fulldeps folder and a new run-make test suite was added which
applies to all targets by default. This test suite currently only has one test
which only runs for the wasm target (using a node.js script to use WebAssembly
in JS to parse the wasm output).

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 9, 2018
@@ -2359,11 +2359,6 @@ impl<'test> TestCx<'test> {
}

fn run_rmake_test(&self) {
// FIXME(#11094): we should fix these tests
if self.config.host != self.config.target {
Copy link
Member

Choose a reason for hiding this comment

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

hm, won't this make us run more tests now? Or do we handle this elsewhere for fulldeps suites somehow?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this was added long long ago when we still ran more tests in more situations (like tests on dist builders), but nowadays I don't think this is ever triggered. If I'm wrong though this'll just fail in CI


if attr.value_str().is_none() {
self.tcx.sess.span_err(attr.span, "must be of the form \
#[wasm_bindgen = \"foo\"]");
Copy link
Contributor

Choose a reason for hiding this comment

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

wasm_custom_section?

pub fn add_custom_sections(path: &Path, sections: &BTreeMap<String, Vec<u8>>) {
let mut wasm = fs::read(path).expect("failed to read wasm output");

// see https://github.com/WebAssembly/design/blob/master/BinaryEncoding.md#module-structure
Copy link
Contributor

Choose a reason for hiding this comment

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

@bors
Copy link
Contributor

bors commented Mar 11, 2018

☔ The latest upstream changes (presumably #48799) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Mar 14, 2018

☔ The latest upstream changes (presumably #48811) made this pull request unmergeable. Please resolve the merge conflicts.

@nikomatsakis
Copy link
Contributor

Travis is sad:

you need to replace ty::ParamEnv::empty(traits::Reveal::All) with ty::ParamEnv::reveal_all()

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

r=me once travis is happy

@@ -455,6 +455,9 @@ declare_features! (

// Parentheses in patterns
(active, pattern_parentheses, "1.26.0", None, None),

// The #[wasm_custom_section] attribute
(active, wasm_custom_section, "1.26.0", None, None),
Copy link
Contributor

Choose a reason for hiding this comment

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

(don't we need a feature-gate test for this ?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Heh probably!

@nikomatsakis nikomatsakis added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 14, 2018
@alexcrichton
Copy link
Member Author

@bors: r=nikomatsakis

@bors
Copy link
Contributor

bors commented Mar 14, 2018

📌 Commit 464f338 has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 14, 2018
@alexcrichton
Copy link
Member Author

@bors: r=nikomatsakis

@bors
Copy link
Contributor

bors commented Mar 14, 2018

📌 Commit 5d760d3 has been approved by nikomatsakis

@alexcrichton
Copy link
Member Author

@bors: r=nikomatsakis

@bors
Copy link
Contributor

bors commented Mar 14, 2018

📌 Commit 3c9dcad has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Mar 22, 2018

☔ The latest upstream changes (presumably #49264) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 22, 2018
This commit is an implementation of adding custom sections to wasm artifacts in
rustc. The intention here is to expose the ability of the wasm binary format to
contain custom sections with arbitrary user-defined data. Currently neither our
version of LLVM nor LLD supports this so the implementation is currently custom
to rustc itself.

The implementation here is to attach a `#[wasm_custom_section = "foo"]`
attribute to any `const` which has a type like `[u8; N]`. Other types of
constants aren't supported yet but may be added one day! This should hopefully
be enough to get off the ground with *some* custom section support.

The current semantics are that any constant tagged with `#[wasm_custom_section]`
section will be *appended* to the corresponding section in the final output wasm
artifact (and this affects dependencies linked in as well, not just the final
crate). This means that whatever is interpreting the contents must be able to
interpret binary-concatenated sections (or each constant needs to be in its own
custom section).

To test this change the existing `run-make` test suite was moved to a
`run-make-fulldeps` folder and a new `run-make` test suite was added which
applies to all targets by default. This test suite currently only has one test
which only runs for the wasm target (using a node.js script to use `WebAssembly`
in JS to parse the wasm output).
This commit adds a new attribute to the Rust compiler specific to the wasm
target (and no other targets). The `#[wasm_import_module]` attribute is used to
specify the module that a name is imported from, and is used like so:

    #[wasm_import_module = "./foo.js"]
    extern {
        fn some_js_function();
    }

Here the import of the symbol `some_js_function` is tagged with the `./foo.js`
module in the wasm output file. Wasm-the-format includes two fields on all
imports, a module and a field. The field is the symbol name (`some_js_function`
above) and the module has historically unconditionally been `"env"`. I'm not
sure if this `"env"` convention has asm.js or LLVM roots, but regardless we'd
like the ability to configure it!

The proposed ES module integration with wasm (aka a wasm module is "just another
ES module") requires that the import module of wasm imports is interpreted as an
ES module import, meaning that you'll need to encode paths, NPM packages, etc.
As a result, we'll need this to be something other than `"env"`!

Unfortunately neither our version of LLVM nor LLD supports custom import modules
(aka anything not `"env"`). My hope is that by the time LLVM 7 is released both
will have support, but in the meantime this commit adds some primitive
encoding/decoding of wasm files to the compiler. This way rustc postprocesses
the wasm module that LLVM emits to ensure it's got all the imports we'd like to
have in it.

Eventually I'd ideally like to unconditionally require this attribute to be
placed on all `extern { ... }` blocks. For now though it seemed prudent to add
it as an unstable attribute, so for now it's not required (as that'd force usage
of a feature gate). Hopefully it doesn't take too long to "stabilize" this!

cc rust-lang-nursery/rust-wasm#29
@alexcrichton
Copy link
Member Author

@bors: r=nikomatsakis

@bors
Copy link
Contributor

bors commented Mar 22, 2018

📌 Commit d889957 has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 22, 2018
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Mar 23, 2018
…r=nikomatsakis

rustc: Add a `#[wasm_custom_section]` attribute

This commit is an implementation of adding custom sections to wasm artifacts in
rustc. The intention here is to expose the ability of the wasm binary format to
contain custom sections with arbitrary user-defined data. Currently neither our
version of LLVM nor LLD supports this so the implementation is currently custom
to rustc itself.

The implementation here is to attach a `#[wasm_custom_section = "foo"]`
attribute to any `const` which has a type like `[u8; N]`. Other types of
constants aren't supported yet but may be added one day! This should hopefully
be enough to get off the ground with *some* custom section support.

The current semantics are that any constant tagged with `#[wasm_custom_section]`
section will be *appended* to the corresponding section in the final output wasm
artifact (and this affects dependencies linked in as well, not just the final
crate). This means that whatever is interpreting the contents must be able to
interpret binary-concatenated sections (or each constant needs to be in its own
custom section).

To test this change the existing `run-make` test suite was moved to a
`run-make-fulldeps` folder and a new `run-make` test suite was added which
applies to all targets by default. This test suite currently only has one test
which only runs for the wasm target (using a node.js script to use `WebAssembly`
in JS to parse the wasm output).
bors added a commit that referenced this pull request Mar 23, 2018
@alexcrichton alexcrichton merged commit d889957 into rust-lang:master Mar 23, 2018
@eddyb
Copy link
Member

eddyb commented Mar 28, 2018

Do we want to use #[section_name] for this long-term by any chance?

@alexcrichton alexcrichton deleted the wasm-custom-sections branch March 28, 2018 23:02
@alexcrichton
Copy link
Member Author

@eddyb perhaps, although it has a pretty radically different meaning on wasm so far which doens't map well (aka we bypass llvm completely)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants