Add Module.customSections #877

lukewagner · 2016-11-22T21:45:59Z

This PR adds a reflective static Module function for returning the contents of user-defined sections. The intended use case is dynamic linking: after compiling a Module which is to be dynamically linked to other instances via shared Memory/Table, the linker needs metadata to know, e.g., how much space to allocate for global data and Table entries (including zero/empty entries, so simply exposing the length of data and elem sections isn't sufficient). Providing access to metadata through user-defined sections avoids the need for some out-of-band metadata source or the need to do ad hoc binary decoding at runtime.

Two interesting details:

this proposal only returns user-defined sections (known sections should have separate reflective methods like exports/imports). However, by rejecting sectionName arguments that aren't already a string, we leave open the possibility of passing Number values in the future to extract known sections
because sections are not necessarily unique, the function returns a (possibly-empty) array of results when querying a given user-defined section name

rossberg · 2016-11-23T07:22:47Z

JS.md

+Array sections(modueObject, sectionName)
+```
+
+If `moduleObject` is not a `WebAssembly.Module` instance, a [`TypeError`](https://tc39.github.io/ecma262/#sec-native-error-types-used-in-this-standard-typeerror)


Perhaps s/instance/object/, to avoid confusion

Hey, yeah, that is confusing. In other places we just say "if x is not a Y" (no "instance" or "object"), so I'll do that here too.

rossberg · 2016-11-23T07:28:00Z

Looks good, the only thing I wonder about is the method name, which suggests that it can return any kind of section. Would userSections be more adequate? As you say, known sections should rather get their own methods, so I wouldn't worry about extending this one. Even if we ever wanted to return raw known sections, that would probably still be addressed better with a separate method (that e.g. takes a symbolic section name instead of a meaningless number).

lukewagner · 2016-11-23T16:05:14Z

Hah, I was going back and forth on exactly this. That's a good point re: known sections and symbolic names, though. I'll switch.

lukewagner · 2016-11-23T16:12:15Z

(I also put back the ToString on the sectionName argument, not because I want all the crazy coercions it allows, but just for regularity since it seems everything string-taking does a ToString.)

rossberg · 2016-11-23T16:12:28Z

LGTM

jfbastien

I'd rather leave this open until next week, since folks are off for Thanksgiving.

jfbastien · 2016-11-23T16:35:43Z

JS.md

@@ -205,6 +205,25 @@ to the Object `{ module: String(i.module_name), name: String(i.item_name), kind:

 Note: other fields like `signature` may be added in the future.

+### `WebAssembly.Module.userSections`


We call them "unknown" sections in the binary format. I'd rather be consistent (either name is fine).

Ugh, I hadn't noticed; it seems we call them both "user-defined" and "unknown" sections. Seems like we should just go with one. I actually like "unknown" better than "user defined"; it contrasts better with "known". @rossberg-chromium ?

TBH I don't like the term "user". Seems condescending.
But "unknown" is weird.
So yeah, I don't care for either name ;-)

How about "named sections" everywhere?

Yeah, I have the same vague feeling re "user". I like "named".

We moved to calling these "user" sections in wabt, and in my proposed llvm binutils patch.

I think "user" is fine, and has more meaning than either "named" or "unkown" which don't seem to convey much to me. We're all used to "user_data" pointers in callbacks.

jfbastien · 2016-11-23T16:37:05Z

JS.md

+[user-defined section](BinaryEncoding.md#high-level-structure) (i.e., section with
+`id` 0) whose `name` field ([decoded as UTF-8](Web.md#names)) is equal to
+`sectionNameString` to an `ArrayBuffer` containing the section's `payload_data`.
+(Note: `payload_data` does not include `name` or `name_len`.)


This Array is a copy of the original content, correct?

Well, technically ArrayBuffer, but yes. I'll add "copy of" to clarify.

I was wondering about this. Why make a copy? Don't we have the original buffer copied already (to prevent user from modifying it while compiling)? Maybe not a big deal, but it seems wasteful when we could just provide a view over the existing copy.

If we had the ability to return a read-only view, I'd do that, but otherwise we're in trouble if the user tries to write to the view.

Hm, too bad. Though I suppose users who are concerned about this cost for very large modules can parse the module themselves.

jfbastien · 2016-11-23T16:38:29Z

JS.md

+The `userSections` function has the signature:
+
+```
+Array userSections(modueObject, sectionName)


Typo "modueObject".

lukewagner · 2016-11-23T16:44:16Z

@jfbastien Agreed, I was going to wait as well.

rossberg · 2016-11-23T16:57:46Z

"Unknown" is weird, "named" a bit too generic. How about "custom"?

lukewagner · 2016-11-28T20:51:17Z

"Custom sections" sounds fine to me. Anyone else?

lukewagner · 2016-11-28T22:16:33Z

Updated, PTAL.

jfbastien · 2016-11-28T22:18:37Z

lgtm

binji · 2016-11-28T23:49:28Z

Custom is fine. But wouldn't you prefer "Bespoke"? :-)

rossberg · 2016-11-29T08:21:54Z

lgtm

Add Module.sections

bd1f38b

rossberg reviewed Nov 23, 2016

View reviewed changes

Don't say 'instance' to avoid confusion

c10e7b4

Luke Wagner added 2 commits November 23, 2016 10:06

s/sections/userSections/

94f3942

Put ToString() back, for JS API regularity

fea04e4

jfbastien reviewed Nov 23, 2016

View reviewed changes

Luke Wagner added 2 commits November 23, 2016 10:42

Fix spelling of moduleObject

8dde47e

Clarify returning a copy

1ae783e

s/user|unknown/custom/ and in BinaryEncoding.md too

a441a8c

lukewagner merged commit e380336 into master Nov 29, 2016

lukewagner deleted the add-sections-method branch November 29, 2016 15:29

lukewagner changed the title ~~Add Module.sections~~ Add Module.customSections Nov 30, 2016

Cellule mentioned this pull request Apr 7, 2017

WASM - Names Section chakra-core/ChakraCore#2811

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Module.customSections #877

Add Module.customSections #877

lukewagner commented Nov 22, 2016

rossberg Nov 23, 2016

lukewagner Nov 23, 2016

rossberg commented Nov 23, 2016

lukewagner commented Nov 23, 2016

lukewagner commented Nov 23, 2016

rossberg commented Nov 23, 2016

jfbastien left a comment

jfbastien Nov 23, 2016

lukewagner Nov 23, 2016 •

edited

Loading

jfbastien Nov 23, 2016

lukewagner Nov 23, 2016

sbc100 Nov 23, 2016

jfbastien Nov 23, 2016

lukewagner Nov 23, 2016

binji Nov 28, 2016

lukewagner Nov 28, 2016

binji Nov 28, 2016

jfbastien Nov 23, 2016

lukewagner commented Nov 23, 2016

rossberg commented Nov 23, 2016

lukewagner commented Nov 28, 2016

lukewagner commented Nov 28, 2016

jfbastien commented Nov 28, 2016

binji commented Nov 28, 2016

rossberg commented Nov 29, 2016

		@@ -205,6 +205,25 @@ to the Object `{ module: String(i.module_name), name: String(i.item_name), kind:

		Note: other fields like `signature` may be added in the future.

		### `WebAssembly.Module.userSections`

Add Module.customSections #877

Add Module.customSections #877

Conversation

lukewagner commented Nov 22, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rossberg commented Nov 23, 2016

lukewagner commented Nov 23, 2016

lukewagner commented Nov 23, 2016

rossberg commented Nov 23, 2016

jfbastien left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukewagner Nov 23, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukewagner commented Nov 23, 2016

rossberg commented Nov 23, 2016

lukewagner commented Nov 28, 2016

lukewagner commented Nov 28, 2016

jfbastien commented Nov 28, 2016

binji commented Nov 28, 2016

rossberg commented Nov 29, 2016

lukewagner Nov 23, 2016 •

edited

Loading