Test case: --- iter = getSelfHostedValue("CreateListIterator")([]); iter.next(); iter.next(); --- Asserts with: Assertion failure: vp->isInt32(), at /home/andre/git/mozilla-central/js/src/vm/SelfHosting.cpp:389 Stack trace: --- #0 0x0000000000c86442 in intrinsic_UnsafeGetInt32FromReservedSlot (cx=0x7ffff691b000, argc=2, vp=0x7ffff3c62148) at /home/andre/git/mozilla-central/js/src/vm/SelfHosting.cpp:389 #1 0x0000000000bdeba8 in js::CallJSNative (cx=0x7ffff691b000, native=0xc863d3 <intrinsic_UnsafeGetInt32FromReservedSlot(JSContext*, unsigned int, JS::Value*)>, args=...) at /home/andre/git/mozilla-central/js/src/jscntxtinlines.h:235 #2 0x0000000000bba7a2 in js::Invoke (cx=0x7ffff691b000, args=..., construct=js::NO_CONSTRUCT) at /home/andre/git/mozilla-central/js/src/vm/Interpreter.cpp:489 #3 0x0000000000bc9c14 in Interpret (cx=0x7ffff691b000, state=...) at /home/andre/git/mozilla-central/js/src/vm/Interpreter.cpp:2798 #4 0x0000000000bba38e in js::RunScript (cx=0x7ffff691b000, state=...) at /home/andre/git/mozilla-central/js/src/vm/Interpreter.cpp:430 #5 0x0000000000bba862 in js::Invoke (cx=0x7ffff691b000, args=..., construct=js::NO_CONSTRUCT) at /home/andre/git/mozilla-central/js/src/vm/Interpreter.cpp:507 #6 0x0000000000bc9c14 in Interpret (cx=0x7ffff691b000, state=...) at /home/andre/git/mozilla-central/js/src/vm/Interpreter.cpp:2798 #7 0x0000000000bba38e in js::RunScript (cx=0x7ffff691b000, state=...) at /home/andre/git/mozilla-central/js/src/vm/Interpreter.cpp:430 #8 0x0000000000bbb979 in js::ExecuteKernel (cx=0x7ffff691b000, script=..., scopeChainArg=..., thisv=..., newTargetValue=..., type=js::EXECUTE_GLOBAL, evalInFrame=..., result=0x7fffffffd500) at /home/andre/git/mozilla-central/js/src/vm/Interpreter.cpp:703 #9 0x0000000000bbbd53 in js::Execute (cx=0x7ffff691b000, script=..., scopeChainArg=..., rval=0x7fffffffd500) at /home/andre/git/mozilla-central/js/src/vm/Interpreter.cpp:742 #10 0x00000000009b648b in ExecuteScript (cx=0x7ffff691b000, scope=..., script=..., rval=0x7fffffffd500) at /home/andre/git/mozilla-central/js/src/jsapi.cpp:4388 #11 0x00000000009b67e3 in JS_ExecuteScript (cx=0x7ffff691b000, scriptArg=..., rval=...) at /home/andre/git/mozilla-central/js/src/jsapi.cpp:4414 #12 0x0000000000431edc in EvalAndPrint (cx=0x7ffff691b000, bytes=0x7ffff6914f80 "iter = getSelfHostedValue(\"CreateListIterator\")([]); iter.next(); iter.next()\n", '\344' <repeats 50 times>, "\300\063\346\367\377\177\374\377", length=78, lineno=1, compileOnly=false, out=0x7ffff6ef7740 <_IO_2_1_stdout_>) at /home/andre/git/mozilla-central/js/src/shell/js.cpp:536 #13 0x00000000004323a2 in ReadEvalPrintLoop (cx=0x7ffff691b000, in=0x7ffff6ef6980 <_IO_2_1_stdin_>, out=0x7ffff6ef7740 <_IO_2_1_stdout_>, compileOnly=false) at /home/andre/git/mozilla-central/js/src/shell/js.cpp:600 #14 0x0000000000432562 in Process (cx=0x7ffff691b000, filename=0x0, forceTTY=true) at /home/andre/git/mozilla-central/js/src/shell/js.cpp:632 #15 0x0000000000446ad7 in ProcessArgs (cx=0x7ffff691b000, op=0x7fffffffda20) at /home/andre/git/mozilla-central/js/src/shell/js.cpp:5986 #16 0x0000000000447eca in Shell (cx=0x7ffff691b000, op=0x7fffffffda20, envp=0x7fffffffdc28) at /home/andre/git/mozilla-central/js/src/shell/js.cpp:6320 #17 0x0000000000449314 in main (argc=1, argv=0x7fffffffdc18, envp=0x7fffffffdc28) at /home/andre/git/mozilla-central/js/src/shell/js.cpp:6677 ---

Yes, we can't assume we have an int32 there.

Comment on attachment 8682420 [details] [diff] [review] bug1220766-list-iterator Review of attachment 8682420 [details] [diff] [review]: ----------------------------------------------------------------- Eh. I feel like... just because the fuzzers can generate this doesn't mean we shouldn't be able to assume it's always an Int32. Can it ever be non-Int32 except through API misuse and fuzzing?

Comment on attachment 8682420 [details] [diff] [review] bug1220766-list-iterator Review of attachment 8682420 [details] [diff] [review]: ----------------------------------------------------------------- I thought this was a fuzz bug but it isn't. Self-hosted functions are allowed to have stricter contracts for use and it's fine to crash when misusing them.

The list iterator will get exposed to user script when Reflect.enumerate is implemented, so it seems useful to fix this bug now.

(In reply to André Bargull from comment #4) > The list iterator will get exposed to user script when Reflect.enumerate is > implemented, so it seems useful to fix this bug now. And Reflect.enumerate can return an iterator whose internal next slot isn't an int32?

From ES2015: 26.1.5 Reflect.enumerate calls the internal [[Enumerate]] method, [[Enumerate]] for module namespace exotic objects (9.4.6.11) returns the result of CreateListIterator (7.4.8). The current implementation of the list iterator's next method in js/src/builtins/Iterator.js sets the ITERATOR_SLOT_NEXT_INDEX reserved slot to Infinity when the iterator is drained. So if next() is called after the iterator has finished, the line `let index = UnsafeGetInt32FromReservedSlot(this, ITERATOR_SLOT_NEXT_INDEX)` will trigger an assertion, because ITERATOR_SLOT_NEXT_INDEX is Infinity which is not an int32. With proper support for Reflect.enumerate, I'd use the following STR: --- 1. Create javascript module file "testcase.js": ``` import* as self from "testcase.js"; var iter = Reflect.enumerate(self); iter.next(); iter.next(); ``` 2. Execute module "testcase.js". --- Unfortunately Reflect.enumerate is not yet supported, so the only way to demonstrate the bug right now is to use the internal getSelfHostedValue() function.

CreateListIterator is also used for iteration of module namespaces and it's possible to trigger this assertion with a testcase based on modules so I think we need to fix this. Re-flagging for review.

Oh, I totally forgot about the built-in module iteration. New test case without getSelfHostedValue(): --- var moduleRepo = Object.create(null); setModuleResolveHook(function(module, specifier) { if (specifier in moduleRepo) return moduleRepo[specifier]; throw "Module " + specifier + " not found"; }); var m = parseModule("import* as self from 'a'; var iter = self[Symbol.iterator](); iter.next(); iter.next();"); moduleRepo["a"] = m; m.declarationInstantiation(); m.evaluation(); ---

Comment on attachment 8682420 [details] [diff] [review] bug1220766-list-iterator Review of attachment 8682420 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for posting other STR.