Closed Bug 1149811 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(2 files)

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.
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?
https://hg.mozilla.org/mozilla-central/rev/e1f860c2331b
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: