Closed Bug 1317085 Opened 3 years ago Closed 3 years ago

Assertion failure: unwrapped->isAsync(), at js/src/vm/AsyncFunction.cpp:225

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: arai, Assigned: arai)

Details

(Whiteboard: [adv-main52+])

Attachments

(3 files)

blocks bug 1314055, but leaving the "Blocks" empty for now.

code:
  var code = cacheEntry("async function f(a, b) { await 10; }; print(f.toString());");
  var g = newGlobal({ cloneSingletons: true });
  evaluate(code, { global: g, saveBytecode: true });
  evaluate(code, { global: g, loadBytecode: true });

JSScript's async flags are not handled in XDR.
we should encode the flag, and maybe regenerate slots on decode.
clone also doesn't work, and it needs regenerating slots

will fix them at once.
apparently, we can just deny cloning async function, instead of deep-cloning unwrapped function.
patches are almost ready.
now checking if there's some related bugs.
Assignee: nobody → arai.unmht
Added IsAsync bit to js::XDRScript.

The testcase checks if both slots are valid, and flag is also valid.
  * toString and calling function checks WRAPPED_ASYNC_UNWRAPPED_SLOT
  * arguments.callee checks UNWRAPPED_ASYNC_WRAPPED_SLOT
    and also fun->isAsync()
Attachment #8810128 - Flags: review?(till)
cloning won't be necessary for async function, as it looks like it's used only in XBL method, and I cannot think of the case that it needs async.
https://dxr.mozilla.org/mozilla-central/search?q=CloneFunctionObject(&redirect=false

will double check if cloning doesn't happen elsewhere.
Attachment #8810129 - Flags: review?(till)
maybe related to bug 1316166.
We'll need a testcase that tests relazification.
it just passed without any change.
Attachment #8810130 - Flags: review?(till)
Comment on attachment 8810128 [details] [diff] [review]
Part 1: Handle async function flag in XDR.

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

Good catch!
Attachment #8810128 - Flags: review?(till) → review+
Comment on attachment 8810129 [details] [diff] [review]
Part 2: Throw error when cloning async function.

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

r=me
Attachment #8810129 - Flags: review?(till) → review+
Comment on attachment 8810130 [details] [diff] [review]
Part 3: Add testcase for relazification.

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

r=me
Attachment #8810130 - Flags: review?(till) → review+
thank you for reviewing :)

function cloning happens in the following cases:
  * XBL methods
    that won't be async, at least with current syntax, and if it happens, error will be thrown with patched build
  * self-hosted function
    there's no async function now.  we could reconsider when it becomes necessary
  * Interpterer (JSOP_FUNWITHPROTO, JSOP_LAMBDA, JSOP_LAMBDA_ARROW)
    only unwrapped async function appear there, and it's not yet wrapped, so it won't be any issue (and it's already tested)
  * module top level functions
    it also contains unwrapped async function, that is not yet wrapped.

so, for now it should be safe.
maybe we should file another bug to track it in module.


then, this bug is a regression from 1314055, that is nightly-only.
will land patches shortly.
Group: core-security → core-security-release
Whiteboard: [adv-main52+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.