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

Memory.prototype.grow(1) TypeArray(memory.buffer, memory.buffer.byteLength/2, memory.buffer.byteLength) RangeError #2719

Closed
guest271314 opened this issue Jul 29, 2020 · 18 comments

Comments

@guest271314
Copy link

try {
  const m = new WebAssembly.Memory({initial:1, maximum: 2, shared: true});
  const encoder = new TextEncoder();
  const t_0 = encoder.encode('a'.repeat(m.buffer.byteLength));
  const u8_0 = new Uint8Array(m.buffer, 0, m.byteLength);
  u8_0.set(u8_0, 0);
  m.grow(1);
  // does not throw RangeError
  const u8_m_byteLength = new Uint8Array(m.buffer.byteLength);
  const t_1 = encoder.encode('b'.repeat(m.buffer.byteLength/2));
  // throws RangeError
  const u8_1 = new Uint8Array(m.buffer, m.buffer.byteLength/2, m.buffer.byteLength);
  // we do not get here
  // u8_1.set(t_1, m.buffer.byteLength/2);
} catch (e) {
  console.error(e);
  console.trace();
} 

Chromium 86

RangeError: Invalid typed array length: 131072
    at new Uint8Array (<anonymous>)
    at <anonymous>:13:16
(anonymous) @ VM2317:17

Nightly 80

RangeError: attempting to construct out-of-bounds TypedArray on ArrayBuffer
    <anonymous> debugger eval code:12
debugger eval code:16:11

Adjusting the code

try {
  const m = new WebAssembly.Memory({initial:1, maximum: 2, shared: true});
  const encoder = new TextEncoder();
  const t_0 = encoder.encode('a'.repeat(m.buffer.byteLength));
  const u8_0 = new Uint8Array(m.buffer, 0, m.byteLength);
  u8_0.set(u8_0, 0);
  m.grow(1);
  // does not throw RangeError
  const u8_m_byteLength = new Uint8Array(m.buffer.byteLength);
  const t_1 = encoder.encode('b'.repeat(m.buffer.byteLength/2));
  // does not throw RangeError
  const u8_1 = new Uint8Array(m.buffer, m.buffer.byteLength/2);
  // throws RangeError
  u8_1.set(t_1, m.buffer.byteLength/2);
} catch (e) {
  console.error(e);
  console.trace();
} 

Chromium 86

RangeError: offset is out of bounds
    at Uint8Array.set (<anonymous>)
    at <anonymous>:15:8
(anonymous) @ VM2376:17
console.trace
(anonymous) @ VM2376:18

Nightly 80

RangeError: source array is too long
    <anonymous> debugger eval code:14
console.trace() debugger eval code:17:11
    <anonymous> debugger eval code:17

Which means the original code that am attempting to test

const u8_2 = new Uint8Array(m.buffer, m.buffer.byteLength/2 -1, 2);

is not possible, at least using TypedArray constructor and set() in the above patterns?

Is this specified and expected behaviour?

Or, is this an implementer of SharedArrayBuffer and TypedArray decision?

Related WebAssembly/design#1358.

Am asking the specific question if this is intended result, for testing, to at least have some certainty in a base case for workaround.

@leobalter
Copy link
Member

I'm extremely confused. Please help me out of my ignorance here, can you limit the question to what relates to ECMAScript only?

This project does not observe WebAssembly specs and I'd be thankful if you can save me a time shortening the question to what might be wrong, not covered, or idk on TypedArrays or ArrayBuffer in this project.

Thanks

@guest271314
Copy link
Author

Tried to make the question as simple as possible by using code. Already filed an issue at WebAssembly/design, where was referred to JavaScript and implementers. From a front-end perspective the respective stakeholders either should have considered this already, and tested, or have not gotten around to testing SharedArrayBuffer, which appears to be the case when reading the Issues in this repository re the state of SharedArrayBuffer testing.

Running the code at console at Chromium or Nightly outputs the results.

Essentially the behaviour of RangeError being thrown when the underlying SharedArrayBuffer is grown using Memory.prototype.grow(1) is called, for constructor and set(), AFAICT, is undocumented.

The SharedArrayBuffer description in the specification could be considered arcane, yet read it anyway attempting to locate where the case of SharedArrayBuffer growing for subsequent TypedArray throws RangeError. Could not find such language.

The question is very basic, is that the intended behaviour?

If not, kindly document the intended behaviour from JavaScript perspective, as the actual result breaks description of TypedArray(buffer, offset, byteLength), and TypedArray.set().

@guest271314
Copy link
Author

The buck has been passed from WebAssembly WebAssembly/design#1358 (comment) to implementers

You'll have much more luck opening such reports against the browser in question, not here. https://bugs.chromium.org/p/chromium/issues/list

to JavaScript WebAssembly/design#1358 (comment)

That's javascript, not webassembly, though.

back to WebAssembly

This project does not observe WebAssembly specs

WebAssembly.Memory.grow() is implemented in JavaScript at the browser, and uses JavaScript language, TypedArray, SharedArrayBuffer, Atomics.

If implementers are consistent in throwing RangeError that is either by design, omission, or limitation of the architecture. Specification authors write and test their language. When the language is not a simple matter to find for a case at the front-end the rational path to proceed is to ask the parties involved in writing the language and implementing the API if they considered a given case, and if so, where does the language in the relevant specifications reflect such consideration?

Will file implementer issues.

@ljharb
Copy link
Member

ljharb commented Jul 29, 2020

In JS, TypedArrays and SharedArrayBuffers can't "grow"; their size is static.

https://github.com/tc39-transfer/proposal-resizablearraybuffer is a proposal to allow that.

@leobalter
Copy link
Member

The question is very basic, is that the intended behaviour?

I believe this is not best the repo for you will to find the answer for your questions.

WebAssembly.Memory.grow() is implemented in JavaScript at the browser, and uses JavaScript language, TypedArray, SharedArrayBuffer, Atomics.

Test262 is limited to the ECMAScript suite and I don't even know what to say about the WebAssembly API.

We can discuss here about test coverage (not implementation) of TypedArray, SharedArrayBuffer, and Atomics. If there is a part of ECMAScript we are not observing through tests, please help me to address this in this project. It would be very helpful if we could use examples that are not relying on any other specification or just illustrates actual and expected behaviors.

For anything else, I'm pretty lost.

@guest271314
Copy link
Author

Well, SharedArrayBuffer can grow per WebAssembly.Memory.prototype.grow(N). The problem appears to be that TypedArray was not specified for that application and nobody has bothered to document that although grow() is shipped, JavaScript has not specified a TypedArray that can "grow". That needs to be documented, in all of the relevant parties' specifications and repositories, if only as a non-normative Note, as, again, if a developer were to just read MDN description of TypedArray and TypedArray.prototype.set() right now, and the controlling specifications, they will not find clear language describing the limitations on the state of the art, and would not have a reason to cite for why using grow() breaks the specification and developer descriptions.

@guest271314
Copy link
Author

We can discuss here about test coverage (not implementation) of TypedArray, SharedArrayBuffer, and Atomics

Well, yes, that is why filed this issue here. The cases are not tested right now.

Just happened to encounter this issue while experimenting with workarounds for the restrictions of different API's. Decided to let the relevant stakeholders know that their documentation - and testing - is lacking in this topic.

@ljharb
Copy link
Member

ljharb commented Jul 29, 2020

@guest271314 I'm pretty sure SharedArrayBuffers can't grow per the JS spec; I don't know anything about WebAssembly.Memory. If wasm is specified in a way that conflicts with JS, then that seems like something to take up with wasm?

@leobalter
Copy link
Member

Well, SharedArrayBuffer can grow per WebAssembly.Memory.prototype.grow(N).

That's a violation of the ECMAScript specs, therefore I cannot see how the problem relates to Test262.

TypedArray was not specified for that application

You would be surprised that TypedArray is specified for a broader usage, indeed.

JavaScript has not specified a TypedArray that can "grow". That needs to be documented, in all of the relevant parties' specifications and repositories

This is not the repo to address this, and for this I'll close this issue to value time of people working in this project. Please use the appropriate channels. I won't be able to help you here.

@guest271314
Copy link
Author

Do any of you actually read the OP, the links, and the entire thread? Filed an Issue at WebAssembly/design first, where was referred to file a Chromium bug and that the scope was JavaScript.

Have the parties even actually discussed this convergence, as WebAssembly does use JavaScript and JavaScript does describe "abstractions" re SharedArrayBuffer, while appearing to stop short of actually defining limitations.

@anba
Copy link
Contributor

anba commented Jul 29, 2020

new Uint8Array(m.buffer, m.buffer.byteLength/2, m.buffer.byteLength); where m.buffer is a (Shared)ArrayBuffer will always throw, because if there's N element long buffer, you start at N/2 and then request to use N elements, there's simple not enough space in the buffer.

There are ten apples (N), you can keep half of it (N/2), but ten apples you have to give away. That simply doesn't work, right? 😄

@guest271314
Copy link
Author

That's a violation of the ECMAScript specs, therefore I cannot see how the problem relates to Test262.

Where? Cite your references.

You would be surprised that TypedArray is specified for a broader usage, indeed.

Just cite the primary sources that you are talking about. Do not "believe" in anything. SHow the evidence for your claims.

This is not the repo to address this

Yes, it is. This repository should have already had tests for those cases, but have failed to do so thus far, and is still passing the buck, to whom, is unknown.

@guest271314
Copy link
Author

@anba That does not explain why set() variation throws.

@leobalter
Copy link
Member

That's a violation of the ECMAScript specs, therefore I cannot see how the problem relates to Test262.

Where? Cite your references.

https://tc39.es/ecma262/#sec-sharedarraybuffer-objects

You would be surprised that TypedArray is specified for a broader usage, indeed.

Just cite the primary sources that you are talking about. Do not "believe" in anything. SHow the evidence for your claims.

https://docs.tizen.org/application/web/guides/w3c/supplement/typedarray/

This is not the repo to address this

Yes, it is. This repository should have already had tests for those cases, but have failed to do so thus far, and is still passing the buck, to whom, is unknown.

Have you seen what this project is? This is a project with tests. The only tests here are to cover ECMAScript specs and TC39 proposals in Stage 3.

I cannot offer you tests for cases you're just saying that needs to be documented. Please go through the adequate TC39 process.

@anba
Copy link
Contributor

anba commented Jul 29, 2020

For the set() case:

  • const t_1 = encoder.encode('b'.repeat(m.buffer.byteLength/2)); creates an Uint8Array of length m.buffer.byteLength/2.
  • const u8_1 = new Uint8Array(m.buffer, m.buffer.byteLength/2); creates another Uint8Array of length m.buffer.byteLength/2.
  • u8_1.set(t_1, m.buffer.byteLength/2); assigns all elements of t_1 into u8_1, starting at offset m.buffer.byteLength/2.

Hmm, all elements, so m.buffer.byteLength/2 elements, starting at offset m.buffer.byteLength/2, means the end offset is m.buffer.byteLength, but u8_1 has only m.buffer.byteLength/2 elements space. That also doesn't work.

@guest271314
Copy link
Author

@leobalter What exactly is that "process". Some organizations use the term "process" or claim to have a "process" yet have no clue what real "proceess", or procedure is.

Am not interested in joining any organization just to point out the obvious. The obvious has already been pointed out. The closure of this issue demonstrates that the organization prefers to pass the buck rather than be the authority in some regards.

Why tests would not be perfomed re how TypedArray's are being used with the state of the art technology does not make sense, here. All variations need to be tested, if only to determine where we are, in reality, not in the fiction of language that has obviously not been adapted to the present, implemented code that uses products of JavaScript (Ecmascript). Essentially putting on blinders, which is the opposite of testing.

@guest271314
Copy link
Author

@anba AFAICT there is no configuration or pattern that will "work".

@leobalter
Copy link
Member

Am not interested in joining any organization just to point out the obvious. The obvious has already been pointed out.

ok.

@tc39 tc39 locked as spam and limited conversation to collaborators Jul 29, 2020
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