for (let a in [0]) a = e for (let a in [0]) function () { a } asserts js debug shell on JM changeset 7c6f62fcbd91 with -m at Assertion failure: entries[localIndex(n)].type.inMemory() && entries[localIndex(n)].data.inMemory(), at ../methodjit/FrameState.cpp:808 when passed in as a CLI argument.

need to see what the perf affects are, but I think this is the right idea.

Applying this patch today causes: FAILURES: /home/adrake/src/moo/js/src/trace-test/tests/basic/testDestructuring.js /home/adrake/src/moo/js/src/trace-test/tests/basic/testGroupAssignment.js With: Assertion failure: fe->type.inMemory(), at /home/adrake/src/moo/js/src/methodjit/FrameState-inl.h:698 on both.

Ok, will take this today.

Andrew, please try this rebased one - WFM.

Bug explanation: whenever we store to a slot that could escape (anything in eval code, or an upvar), we have to make sure it's synced. The old code tried to be clever but "let" places new locals in random areas of the stack. Safest & easiest thing is to just sync.

I'm sorry, I should have specified. Those assertions happen with my patch to force the eval bit on. If you take those tests and add 'eval()' to every function (doesn't matter where), those errors can be reproduced without it.

Comment on attachment 462571 [details] [diff] [review] fix > if (!localFe->type.synced()) > syncType(localFe, addressOf(localFe), masm); > if (!localFe->data.synced()) > syncData(localFe, addressOf(localFe), masm); This code pattern occurs in a few places. On x86_64, we can be a bit cleverer and avoid loads in certain situations. What we really mean is "sync the entire FrameEntry" -- changing this to call some syncFE() would allow us to make use of the x86_64 optimization more easily. (Or just add '/* TODO: x64 optimization */', and I can patch that later when I do a sweep.) Looks good.

(In reply to comment #6) > I'm sorry, I should have specified. Those assertions happen with my patch to > force the eval bit on. If you take those tests and add 'eval()' to every > function (doesn't matter where), those errors can be reproduced without it. (Asserts still trip for me, sorry if that wasn't clear)

(In reply to comment #7) Sean, good point - does this function exist already? Andrew, so this assert looks overzealous. I'll just weaken it to only check fixed variables.

(In reply to comment #11) > Sean, good point - does this function exist already? It exists only in an old local patch I abandoned -- so, no.

A testcase for this bug was automatically identified at js/src/jit-test/tests/jaeger/bug580884-3.js.