Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Is it a validation error to use memory.* if there is no memory? #79

Closed
tlively opened this issue Apr 4, 2019 · 4 comments
Closed

Is it a validation error to use memory.* if there is no memory? #79

tlively opened this issue Apr 4, 2019 · 4 comments

Comments

@tlively
Copy link
Member

tlively commented Apr 4, 2019

I didn't see this mentioned in overview.md.

@binji
Copy link
Member

binji commented Apr 4, 2019

@lars-t-hansen
Copy link
Contributor

It's not required for data.drop, though.

On purpose or oversight? Firefox will refuse to validate if data.drop is used without a memory, or elem.drop is used without a table; I can argue it both ways and I'm curious what people think.

@rossberg
Copy link
Member

rossberg commented Apr 4, 2019

That makes sense to me. Conceptually, all memory ops reference memory index 0, but in the future may allow others, making the index explicit; data.drop OTOH does not have any memory index.

(That is, the check is really just an ordinary index check, except that we only support index 0 at the moment.)

@tlively
Copy link
Member Author

tlively commented Apr 5, 2019

Thanks, all!

@tlively tlively closed this as completed Apr 5, 2019
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 28, 2019
…=lhansen.

Our Wasm implementations current reject data.drop and elem.drop if there is no
memory or table in the module.  This isn't consistent with the current
bulk-memory specification, though.
See WebAssembly/bulk-memory-operations#79.

This patch fixes that by slightly loosening the relevant checks, so as to
allow a segment to be dropped even if there is no memory, providing that that
segment is a passive one.

Most of the changes are in the test file passive-segs-nonboundary.js:

* There are four new checks for data.drop without memory and the equivalents
  for table.drop.

* Test construction function `do_test` has been made more flexible, so it is
  now possible to independently specify whether a table/memory is present,
  whether active initialisers are present, and whether passive initialisers
  are present.

In WasmModule.cpp, a couple of places where a segment vector was asserted to
be empty have been changed to assert that it contains only passive segments.
Attempts to do this using std::all_of and a closure were unsuccessful (see bug
1547031) and instead a debug-only helper function `AllSegmentsArePassive` was
added.

Differential Revision: https://phabricator.services.mozilla.com/D28951

--HG--
extra : rebase_source : 2f85bf4fb32a424098ba24670790adee917f882f
mykmelez pushed a commit to mykmelez/gecko that referenced this issue Apr 29, 2019
…=lhansen.

Our Wasm implementations current reject data.drop and elem.drop if there is no
memory or table in the module.  This isn't consistent with the current
bulk-memory specification, though.
See WebAssembly/bulk-memory-operations#79.

This patch fixes that by slightly loosening the relevant checks, so as to
allow a segment to be dropped even if there is no memory, providing that that
segment is a passive one.

Most of the changes are in the test file passive-segs-nonboundary.js:

* There are four new checks for data.drop without memory and the equivalents
  for table.drop.

* Test construction function `do_test` has been made more flexible, so it is
  now possible to independently specify whether a table/memory is present,
  whether active initialisers are present, and whether passive initialisers
  are present.

In WasmModule.cpp, a couple of places where a segment vector was asserted to
be empty have been changed to assert that it contains only passive segments.
Attempts to do this using std::all_of and a closure were unsuccessful (see bug
1547031) and instead a debug-only helper function `AllSegmentsArePassive` was
added.

Differential Revision: https://phabricator.services.mozilla.com/D28951
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 4, 2019
…=lhansen.

Our Wasm implementations current reject data.drop and elem.drop if there is no
memory or table in the module.  This isn't consistent with the current
bulk-memory specification, though.
See WebAssembly/bulk-memory-operations#79.

This patch fixes that by slightly loosening the relevant checks, so as to
allow a segment to be dropped even if there is no memory, providing that that
segment is a passive one.

Most of the changes are in the test file passive-segs-nonboundary.js:

* There are four new checks for data.drop without memory and the equivalents
  for table.drop.

* Test construction function `do_test` has been made more flexible, so it is
  now possible to independently specify whether a table/memory is present,
  whether active initialisers are present, and whether passive initialisers
  are present.

In WasmModule.cpp, a couple of places where a segment vector was asserted to
be empty have been changed to assert that it contains only passive segments.
Attempts to do this using std::all_of and a closure were unsuccessful (see bug
1547031) and instead a debug-only helper function `AllSegmentsArePassive` was
added.

Differential Revision: https://phabricator.services.mozilla.com/D28951

UltraBlame original commit: e81669455530c9d127aafd89c1fdce308b183a68
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 4, 2019
…=lhansen.

Our Wasm implementations current reject data.drop and elem.drop if there is no
memory or table in the module.  This isn't consistent with the current
bulk-memory specification, though.
See WebAssembly/bulk-memory-operations#79.

This patch fixes that by slightly loosening the relevant checks, so as to
allow a segment to be dropped even if there is no memory, providing that that
segment is a passive one.

Most of the changes are in the test file passive-segs-nonboundary.js:

* There are four new checks for data.drop without memory and the equivalents
  for table.drop.

* Test construction function `do_test` has been made more flexible, so it is
  now possible to independently specify whether a table/memory is present,
  whether active initialisers are present, and whether passive initialisers
  are present.

In WasmModule.cpp, a couple of places where a segment vector was asserted to
be empty have been changed to assert that it contains only passive segments.
Attempts to do this using std::all_of and a closure were unsuccessful (see bug
1547031) and instead a debug-only helper function `AllSegmentsArePassive` was
added.

Differential Revision: https://phabricator.services.mozilla.com/D28951

UltraBlame original commit: e81669455530c9d127aafd89c1fdce308b183a68
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
…=lhansen.

Our Wasm implementations current reject data.drop and elem.drop if there is no
memory or table in the module.  This isn't consistent with the current
bulk-memory specification, though.
See WebAssembly/bulk-memory-operations#79.

This patch fixes that by slightly loosening the relevant checks, so as to
allow a segment to be dropped even if there is no memory, providing that that
segment is a passive one.

Most of the changes are in the test file passive-segs-nonboundary.js:

* There are four new checks for data.drop without memory and the equivalents
  for table.drop.

* Test construction function `do_test` has been made more flexible, so it is
  now possible to independently specify whether a table/memory is present,
  whether active initialisers are present, and whether passive initialisers
  are present.

In WasmModule.cpp, a couple of places where a segment vector was asserted to
be empty have been changed to assert that it contains only passive segments.
Attempts to do this using std::all_of and a closure were unsuccessful (see bug
1547031) and instead a debug-only helper function `AllSegmentsArePassive` was
added.

Differential Revision: https://phabricator.services.mozilla.com/D28951

UltraBlame original commit: e81669455530c9d127aafd89c1fdce308b183a68
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants