XDR decoding of JSScript+LazyScript doesn't initScript on the LazyScript

RESOLVED FIXED in Firefox 40

Status

()

Core
JavaScript Engine
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

Trunk
mozilla40
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(2 attachments)

This is making the patches in bug 679939 fail try.  If we xdr a non-lazy but lazifiable function, we create a JSScript and LazyScript, but never actually point the LazyScript to the JSScript.  Then if CreateLazyScriptsForCompartment it asserts that if the LazyScript has no JSScript then the function is lazy... which it's not.
Created attachment 8586426 [details] [diff] [review]
When XDR-decoding a non-lazy function that can be lazified, we need to set up a backpointer from its LazyScript to its JSScript
Attachment #8586426 - Flags: review?(luke)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Comment on attachment 8586426 [details] [diff] [review]
When XDR-decoding a non-lazy function that can be lazified, we need to set up a backpointer from its LazyScript to its JSScript

Review of attachment 8586426 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsscript.cpp
@@ +520,5 @@
>              MOZ_ASSERT(!lazy->sourceObject());
>              lazy->setParent(enclosingScope, &script->scriptSourceUnwrap());
> +
> +            MOZ_ASSERT(!lazy->maybeScript());
> +            lazy->initScript(script);

Do you see any opportunities to hoist these initializations into LazyScript::Create() so that we have less of this forgettable trailing init code?
Attachment #8586426 - Flags: review?(luke) → review+
Hmm.  We have two callers of LazyScript::Create: this code and XDRLazyScript.  The latter is only used when there is no JSScript, so the initScript part would not matter there.  But it _does_ call setParent()... after doing free variables and inner functions.

We just discussed the inner functions thing, so I'll add an assert there.

I'm going to assume that the ordering of setParent/initScript and free variable/inner function stuff does not matter.

There is also a LazyScript::CreateRaw, which is called from Create and from Parser<SyntaxParseHandler>::finishFunctionDefinition.  In the latter case, there is no JSScript to be had and it looks like the setParent() call happens later in BytecodeEmitter.

OK.  So I guess I can move this stuff to Create, at least, and just pass null for the script from XDRLazyScript.  Sound good?
Created attachment 8586807 [details] [diff] [review]
Patch addressing the review comments
Backed out for various test failures: https://hg.mozilla.org/integration/mozilla-inbound/rev/635c61503146
Flags: needinfo?(bzbarsky)
https://hg.mozilla.org/mozilla-central/rev/e1f860c2331b
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox40: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.