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

Tracking issue for the #[wasm_custom_section] attribute #51088

Closed
kennytm opened this Issue May 26, 2018 · 16 comments

Comments

Projects
None yet
5 participants
@kennytm
Member

kennytm commented May 26, 2018

This issue tracks the wasm_custom_section attribute which is used to define the contents of a custom section in the wasm executable.

cc rustwasm/team#82.

The attribute is used like so:

#[wasm_custom_section = "foo"]
const CUSTOM_SECTION_CONTENT: [u8; 5] = [1, 2, 3, 4, 5];

The attribute in its current form can only be applied to const values and must be of the form [u8; N]. The attribute also must be of the form #[wasm_custom_section = "name"].

Custom sections are a feature of the WebAssembly binary encoding. They are intended to allow wasm files to encode arbitrary information useful for tools like debuggers, profilers, or transformations like wasm-bindgen. Each custom section must have a UTF-8 name and a payload of bytes. There is no restricton on the name or payload otherwise.

The linker, LLD, for the wasm target will concatenate custom sections of the same name in the order the objects appear on the comand line. For example if both crate a and crate b define a custom section with the name foo the final output will contain only one custom section with the name of foo where the contents of a and b are concatenated (byte-wise).

Helpful links:

bors added a commit that referenced this issue May 26, 2018

Auto merge of #51090 - kennytm:tidy-check-missing-tracking-issue, r=a…
…lexcrichton

Ensure every unstable language feature has a tracking issue.

Filled in the missing numbers:

* `abi_ptx` → #38788
* `generators` → #43122
* `global_allocator` → #27389

Reused existing tracking issues because they were decomposed from a larger feature

* `*_target_feature` → #44839 (reusing the old `target_feature` number)
* `proc_macros_*` → #38356 (reusing the to-be-stabilized `proc_macros` number)

Filed new issues

* `exhaustive_patterns` → #51085
* `pattern_parentheses` → #51087
* `wasm_custom_section` and `wasm_import_module` → #51088

bors added a commit that referenced this issue May 27, 2018

Auto merge of #51090 - kennytm:tidy-check-missing-tracking-issue, r=a…
…lexcrichton

Ensure every unstable language feature has a tracking issue.

Filled in the missing numbers:

* `abi_ptx` → #38788
* `generators` → #43122
* `global_allocator` → #27389

Reused existing tracking issues because they were decomposed from a larger feature

* `*_target_feature` → #44839 (reusing the old `target_feature` number)
* `proc_macros_*` → #38356 (reusing the to-be-stabilized `proc_macros` number)

Filed new issues

* `exhaustive_patterns` → #51085
* `pattern_parentheses` → #51087
* `wasm_custom_section` and `wasm_import_module` → #51088

@alexcrichton alexcrichton changed the title from Tracking issue for #[wasm_custom_section] and #[wasm_import_module] attributes to Tracking issue for the #[wasm_custom_section] attribute Jul 6, 2018

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Jul 6, 2018

I've commandeered this issue for just #[wasm_custom_section] and moved #[wasm_import_module] to a different issue as I suspect this will be more controversial than the import modules.

@kennytm

This comment was marked as resolved.

Member

kennytm commented Jul 6, 2018

Oh, feature_gate.rs will need to be updated to the new tracking issue (#52090).

// The #![wasm_import_module] attribute
(active, wasm_import_module, "1.26.0", Some(51088), None),

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Jul 10, 2018

Similar to #52090 I think this feature is ready enough for stabilization now, could I perhaps persuade a member of @rust-lang/compiler to start the FCP for this issue? There's a description of how this issue works in the OP.

Additionally, cc @rust-lang/wg-wasm, y'all are probably interested too!

For me the motivation for this issue is to ensure that wasm-bindgen works on stable for the 2018 edition.

@kennytm

This comment has been minimized.

Member

kennytm commented Jul 10, 2018

Questions:

  1. Why a const instead of a static
  2. Why a new attribute instead of reusing #[link_section]
@oli-obk

This comment has been minimized.

Contributor

oli-obk commented Jul 11, 2018

The linker, LLD, for the wasm target will concatenate custom sections of the same name in the order the objects appear on the comand line. For example if both crate a and crate b define a custom section with the name foo the final output will contain only one custom section with the name of foo where the contents of a and b are concatenated (byte-wise).

If the same section name is used multiple times within a crate, rustc currently accepts this (http://play.rust-lang.org/?gist=d586d1b74b220b29579472bca8f4167d&version=nightly&mode=debug&edition=2015) but I'm fairly certain that this causes an undefined ordering. Especially with incremental involved. Even if it doesn't, reordering modules with sections inside will cause these sections to be reordered.

I'm assuming we could just forbid using the same section name twice within a single crate?

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Jul 11, 2018

@kennytm

Why a const instead of a static

The general idea I had here was that static represents an addressable value in memory, but here custom sections aren't at all accessible by normal code, so I somewhat arbitrarily chose const.

Why a new attribute instead of reusing #[link_section]

A good question! If we went with #[link_section] we wouldn't even have to FCP anything :). I originally did a custom attribute due to the difference with #[link_section] but it may be best to just go back and use it (I'd be up for either). The #[link_section] attribute is more applicable to other platforms like ELF and COFF whereas for wasm you can't shove arbitrary data and constants into custom sections. Additionally you can't have things like relocations to those sections.

So originally I chose a different attribute because it could (a) be unstable vs #[link_section], (b) didn't want to conflate it with being attached to anything, and (c) had strict requirements (currently) on the form of only allowing [u8; N]. Now whether these are good ideas to stick with remains an unknown!


@oli-obk

Correct it is accepted multiple times within one crate! Also correct that there's a somewhat undefined ordering coming out the other end. I don't think the problem is exclusively within one crate though in that you can't really say much about orderings between crates either. In general I believe this just means that you either have to design custom sections to be unique or binary-concatenatable-yet-still-understandable.

Another motivation for supporting multiple uses within a crate is that each #[wasm_bindgen] macro generates a custom section entry, so not allowing it there would be a problem!

@oli-obk

This comment has been minimized.

Contributor

oli-obk commented Jul 11, 2018

@rfcbot fcp merge

@rfcbot

This comment has been minimized.

rfcbot commented Jul 11, 2018

Team member @oli-obk has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@eddyb

This comment has been minimized.

Member

eddyb commented Jul 12, 2018

I prefer #[link_section] as well. Note that the format is already platform-dependent, i.e. for macOS' Mach-O you need something like __DATA,__data instead of .data.

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Jul 12, 2018

Note that if #[link_section] is used it's not just the name that has different formats but it's also the value itself. For example you can put #[link_section] on functions on Linux but you can't do that on wasm, on wasm the #[link_section] attribute would only be applicable to statics/constants which have byte values.

@eddyb

This comment has been minimized.

Member

eddyb commented Jul 12, 2018

@alexcrichton wait, why the byte restriction? You can get the byte contents of any miri allocation, the only restriction you have to impose is no relocations (and other executable formats also have relocation restrictions).

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Jul 12, 2018

@eddyb Ok cool! That may be possible yeah. In that sense this could also be an issue in that case to change to:

  • Remove #[wasm_custom_section] in favor of #[link_section]
  • On the wasm target, #[link_section] can only be applied to static values, erroring out on all other locations
  • On the wasm target, #[link_section] requires no relocations

Denying relocations means denying this, right?

#[link_section = "foo"]
static A: &u8 = &1;
@eddyb

This comment has been minimized.

Member

eddyb commented Jul 12, 2018

@alexcrichton Yeah. miri literally uses a relocation model, so it should be easy to implement.

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Jul 12, 2018

@eddyb hm but it still leaves a weird question I think in terms of:

#[link_section = "foo"]
static A: u8 = 8;

fn main() {
    println!("{}", A);
}

This means that A is both in the main memory as well as a custom section? On all other platforms main code can reference custom sections?

@eddyb

This comment has been minimized.

Member

eddyb commented Jul 13, 2018

@alexcrichton IIRC at least in ELF sections can be marked as not being loaded to memory at all, I wonder what happens then! My guess is either the relocation in A fails to link or the code segfaults.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 13, 2018

rustc: Use link_section, not wasm_custom_section
This commit transitions definitions of custom sections on the wasm target from
the unstable `#[wasm_custom_section]` attribute to the
already-stable-for-other-targets `#[link_section]` attribute. Mostly the same
restrictions apply as before, except that this now applies only to statics.

Closes rust-lang#51088
@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Jul 13, 2018

Ok I've submitted #52353 with @eddyb's suggestion, and if that's merged it'll just close this issue

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 16, 2018

rustc: Use link_section, not wasm_custom_section
This commit transitions definitions of custom sections on the wasm target from
the unstable `#[wasm_custom_section]` attribute to the
already-stable-for-other-targets `#[link_section]` attribute. Mostly the same
restrictions apply as before, except that this now applies only to statics.

Closes rust-lang#51088

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 16, 2018

rustc: Use link_section, not wasm_custom_section
This commit transitions definitions of custom sections on the wasm target from
the unstable `#[wasm_custom_section]` attribute to the
already-stable-for-other-targets `#[link_section]` attribute. Mostly the same
restrictions apply as before, except that this now applies only to statics.

Closes rust-lang#51088

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 16, 2018

rustc: Use link_section, not wasm_custom_section
This commit transitions definitions of custom sections on the wasm target from
the unstable `#[wasm_custom_section]` attribute to the
already-stable-for-other-targets `#[link_section]` attribute. Mostly the same
restrictions apply as before, except that this now applies only to statics.

Closes rust-lang#51088

bors added a commit that referenced this issue Jul 18, 2018

Auto merge of #52353 - alexcrichton:wasm-custom-section, r=eddyb
rustc: Use link_section, not wasm_custom_section

This commit transitions definitions of custom sections on the wasm target from
the unstable `#[wasm_custom_section]` attribute to the
already-stable-for-other-targets `#[link_section]` attribute. Mostly the same
restrictions apply as before, except that this now applies only to statics.

Closes #51088

@bors bors closed this in #52353 Jul 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment