Closed Bug 1496333 Opened 1 year ago Closed 1 year ago
Assertion failure: positional
Params .get().length() == params Count, at js/src/frontend/Bin Source .cpp:483
tested on m-i 21b67d2084a6 Configure flags: --enable-warnings-as-errors --disable-optimize --enable-debug Runtime flag: -B rest.binjs Result: Assertion failure: positionalParams.get().length() == paramsCount, at js/src/frontend/BinSource.cpp:483
encoded from js/src/jit-test/tests/basic/spread-call-rest.js I'll add all testcase in bug 1495611, so no need to add it here
I'll take a look from this
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Getting the parameter list length sounds like it could go bad quickly, guessing sec-high.
so, turns out that the length of ListOfAssertedMaybePositionalParameterName has nothing to do with non-rest parameter's length, for the following reasons: * ListOfAssertedMaybePositionalParameterName can contain the name of destructuring parameters, so the length of ListOfAssertedMaybePositionalParameterName can be greater than the length of parameters (even more than 65535) * there can be empty destructuring parameters, so the length of parameters can be shorter than ListOfAssertedMaybePositionalParameterName length it means we need to resize positionalParams on demand, up to 65535 (which is just implementation detail tho).
Comment on attachment 9015487 [details] [diff] [review] Do not use AssertedParameterScope.paramNames.length for allocation. apparently the patch is broken. I'll post again
Comment on attachment 9015770 [details] [diff] [review] Do not use AssertedParameterScope.paramNames.length for allocation. Review of attachment 9015770 [details] [diff] [review]: ----------------------------------------------------------------- Makes sense to me. Nice of you to wire up destructuring ahead of time, even though we can't parse them right now.
Attachment #9015770 - Flags: review?(efaustbmo) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/3293a3393b4a68e942b7f8fd5f2dd77b337b8676 Bug 1496333 - Do not use AssertedParameterScope.paramNames.length for allocation. r=efaust
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Hello, In order to verify this issue, could you please provide some STR? It would be greatly appreciated, thank you!
Hello, I tried to reproduce this issue using the steps from comment 11, but to no avail. Can you please confirm the fix repairs the issue? Thank you!
Flags: qe-verify+ → needinfo?(arai.unmht)
confirmed the fix with debug build of jsshell on macOS, retrieved from treeherder before: m-c c1ee6c65f85b : reproducible after: m-c 4c11ab0cd989 : not reproducible
I am closing this issue based on comment 13. Thank you!
You need to log in before you can comment on or make changes to this bug.