[spec] Implementation restrictions #483

rossberg · 2017-05-19T11:11:42Z

This PR adds an appendix that lists the allowed (mostly numeric) limits that implementations may impose on WebAssembly programs. I also included lazy validation here, since it seems to fit into this context and there is no other obvious place to put it.

jfbastien · 2017-05-19T16:17:03Z

document/appendix/implementation.rst

+* the length of an :ref:`element segment <syntax-elem>`
+* the length of a :ref:`data segment <syntax-data>`
+* the length of a :ref:`name <syntax-name>`
+* the range of :ref:`code points <syntax-codepoint>` in a :ref:`name <syntax-name>`


I'm not sure restricting codepoints makes sense. Can you drop from here and discuss in a separate issue?

This was the outcome of the discussion on #1016. It allows environments that don't understand Unicode to limit import/export names to ASCII in particular. I added a note explaining the intent.

There wasn't really a discussion or consensus though. I'm not sure restricting versus just ignoring is the right approach, or mandating some form of implementation specification for this, etc. I'd like to discuss this separately, to keep a record of how we came to the decision, and to be really explicit to folks involved. That way we won't revisit the question unless new information comes to light.

So I'd like to tackle this on a follow-up to this PR, which otherwise is pretty straightforward.

Can you elaborate on the alternatives you're thinking of? Note that you cannot ignore imports. Requiring an implementation to document its restrictions seems reasonable, but is independent of the specific item.

Anyway, let's have the discussion here.

I'd like to have the discussion in a separate issue, not here. It is desirable because sub-comments are hidden once a patch is updated, and harder to search for. A separate issue for just this calls out the discussion. Once discussed and consensus is clearly reached we won't revisit the discussion unless new information arises. Discussing here leaves open the possibility of re-discussing.

This does seem to have already been called out quite explicitly so I think any further discussion would be revisiting what was already consensus on that issue.

It was a comment on an issue, with no effect on the PR's content.

I think you misunderstand what I'm trying to get at: within the standards process, we can revisit this issue as it stands. By separating it, and polling the group on it, I first expect the outcome to stay as Andreas proposes and I expect we will not revisit the issue for lack of new information (this is a requirement!).

Without separating out the issue we're sneaking it in here among otherwise unsurprising things. That's not the way I want to conduct this effort. I want to call out what could be contentious so that people who care clearly see it and can respond. Doing so further forces us to document our thought process.

So, I maintain that I'd like this to be separate.

It was a comment on the same logical issue, though, with eyes on it from everyone involved in the utf8 discussion, so it seemed sufficiently visible.

Split out into separate issue: #488

jfbastien · 2017-05-19T16:18:02Z

document/appendix/implementation.rst

+* the size of an individual :ref:`token <text-token>`
+* the nesting depth of :ref:`folded instructions <text-foldedinstr>`
+* the length of symbolic :ref:`identifiers <text-id>`
+* the range of literal :ref:`characters <text-char>` (code points) allowed in the :ref:`source text <source>`


jfbastien · 2017-05-19T16:19:07Z

document/appendix/implementation.rst

+
+.. note::
+   This is to allow implementations to use interpretation or just-in-time compilation for functions.
+   The function must still be fully validated before execution of its body begins.


@MikeHolman can you confirm this is what you want?

yup, this is what we want.

jfbastien · 2017-05-19T16:20:14Z

document/appendix/implementation.rst

+* the number of :ref:`values <syntax-val>` on the :ref:`stack <syntax-stack>`
+
+If the runtime limits of an implementation are exceeded during execution of a computation,
+then it may terminate that computation by causing a trap or reporting an embedder-specific error to the invoking code.


Why trap here? Isn't error sufficient?

Well, technically, a trap is the only runtime error Wasm currently has.

But more to the point, this doesn't say that an impl has to manifest resource exhaustion as a trap, but based on previous discussions it should at least be a legal option, if not the preferred one. Remember that we even required that initially for stack overflow, until we found out that that specific case is kind of weird in a JS embedding. Yet it might still be the best choice for other errors or other embeddings.

Maybe I'm mistaken, but it seems to me that all traps which can occur are defined as part of individual operation semantics. Separately, we document where embedder-specific exceptions can occur, and these are totally separate from traps though the embedder can manifest them as the same-ish (with some way to differentiate).

Runtime limits aren't caused by specific operations. On the above logic I there don't think they should trap.

Some of the above are tied to specific instructions (e.g. calls), some aren't. Thus "trap or reporting an error". Note that a trap currently is the only way in the core semantics to abort execution. Whether and how different traps are distinguished or reported is up to the API spec or the embedder.

We could introduce the notion of "host trap" or something along these lines to the core spec to suggest a distinction, but it would still be indistinguishable from an ordinary trap as far as the spec itself is concerned. Maybe that purpose is better served by a note?

Forking discussion here: WebAssembly/design#1070

jfbastien · 2017-05-19T16:21:43Z

document/appendix/implementation.rst

+Some of the above limits may already be verified during instantiation, in which case an implementation may report exceedance in the same manner as for :ref:`syntactic limits <impl-syntax>`.
+
+.. note::
+   Concrete limits are usually not fixed but may be dependent on specifics, interdependent, vary over time, or depend on other implementation- or embedder-specific variables.


Add: error at runtime if too much memory is dirtied, or code pages exhausted, or random kill (oom kill, too much cpu, etc.).

Generalised to "implementation- or embedder-specific situations or events". Is that broad enough? I intentionally avoided being more specific or even enumerating examples, since it seems futile trying to compile an open-ended list.

I don't think we want to be too broad. For example, over in service-worker land folks are discussing what's an acceptable policy for killing a worker, and how to kill related workers. That's an open issue because the original design was too broad.

Ack, but this one is an informal note explaining possible motivations for possible restrictions. It doesn't really make sense to prescribe a Why, that's not checkable or enforceable. Especially when the What itself already is intentionally vague. If we wanted to be less broad then we could do so by stating minimum bounds, but so far we decided against that.

lukewagner · 2017-05-26T22:05:19Z

document/appendix/implementation.rst

+* the range of :ref:`code points <syntax-codepoint>` in a :ref:`name <syntax-name>`
+
+If the limits of an implementation are exceeded for a given module,
+then the implementation may reject the :ref:`instantiation <exec-instantiate>` of that module with an embedder-specific error.


Could you also add compilation (otherwise it might suggest that these limit errors can only be reported from instantiate not compile).

Done. Also added validation.

jfbastien

lgtm

[spec] Implementation limits

a10360b

rossberg changed the title ~~[spec] Implementation limits~~ [spec] Implementation restrictions May 19, 2017

Define what may happen when limits are exceeded

de5d66c

jfbastien reviewed May 19, 2017

View reviewed changes

rossberg added 2 commits May 19, 2017 19:25

Comments

5cc81c1

Comments

9d5bdb5

jfbastien mentioned this pull request May 22, 2017

Trap versus embedder-specific error WebAssembly/design#1070

Closed

lukewagner approved these changes May 26, 2017

View reviewed changes

Mention compilation

269e9f2

rossberg force-pushed the spec.limits branch from 93c1b37 to 269e9f2 Compare May 29, 2017 12:21

Defer Unicode limits

4c09551

jfbastien approved these changes May 29, 2017

View reviewed changes

rossberg added 3 commits June 6, 2017 11:13

Avoid mentioning traps

9974248

[spec] Allow impls to limit code point range (#488)

56ac21e

Merge branch 'master' into spec.limits

87feae7

rossberg merged commit 6d6648b into master Jun 6, 2017

rossberg deleted the spec.limits branch June 6, 2017 11:08

[spec] Implementation restrictions #483

[spec] Implementation restrictions #483

Uh oh!

Conversation

rossberg commented May 19, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jfbastien May 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rossberg May 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jfbastien left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jfbastien May 19, 2017 •

edited

Loading

rossberg May 29, 2017 •

edited

Loading