1475669 - Assertion failure: this->is<T>() with Debugger "promiseDependentPromises"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[André Bargull [:anba]]

Probably not S-S, because the Debugger is involved, but Jason offered to take a look and decide on it. :-)


Test case:
---
var g = newGlobal();
var dbg = new Debugger();
var gw = dbg.addDebuggee(g);

g.P = Promise;

g.eval(`
    function newPromiseCapability() {
        var resolve, reject, promise = new Promise(function(r1, r2) {
            resolve = r1;
            reject = r2;
        });
        return {promise, resolve, reject};
    }

    var {promise} = newPromiseCapability();

    P.prototype.then.call(promise);
`);

var promise = gw.makeDebuggeeValue(g.promise);

print(promise.promiseDependentPromises);
---


Asserts with:
---
Assertion failure: this->is<T>(), at /home/andre/git/mozilla-central/js/src/vm/JSObject.h:531

Thread 1 "mozjs-debug" received signal SIGSEGV, Segmentation fault.
0x00000000004acbca in JSObject::as<js::NativeObject> (this=(JSObject *) 0x7ffff7e00bf0 [object Proxy]) at /home/andre/git/mozilla-central/js/src/vm/JSObject.h:531
531             MOZ_ASSERT(this->is<T>());
---

Comment 1

[Daniel Veditz [:dveditz]]

calling it sec-other while you look at it. If it's not a security bug please unhide it or ping us on #security to do so. If it is please take a stab at a severity rating and replace the sec-other.

Comment 2

[Jason Orendorff [:jorendorff]]

Sigh. Predictable. This is sec-low; it's a small part of an exploit, and even if you could fill in the blanks the exploit would have to involve convincing the user to open the debugger.

Easy to fix, just takes time...

Comment 3

[André Bargull [:anba]]

Attachment: bug1475669.patch

Unwrap wrappers per |AddPromiseReaction()| [1], including its MOZ_RELEASE_ASSERTs for good measure.

[1] https://searchfox.org/mozilla-central/rev/88199de427d3c5762b7f3c2a4860c10734abd867/js/src/builtin/Promise.cpp#3731

Comment 4

[Nicolas B. Pierron [:nbp]]

Jason, when would you be able to review André's patch?

Comment 5

[Jason Orendorff [:jorendorff]]

Comment on attachment 8993483 [details] [diff] [review]
bug1475669.patch

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

This returns an array of pointers to things in any number of different compartments. That's so strange that the name of the method should point it out.

(I don't know if the Debugger should return dependent promises from non-debuggee compartments or not; I'll check. Separate bug if not.)

Comment 6

[Jason Orendorff [:jorendorff]]

Also, this needs a test. Taking.

Comment 7

[Jason Orendorff [:jorendorff]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e43a5fede8ca9b95d41c065bcfba4ca4c9b4ed6a

Comment 8

[Jason Orendorff [:jorendorff]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cb2f928fff562ff38b66f474ab8acd628535d985
Bug 1475669 - Return proper result for wrapped reactions. r=jorendorff

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/cb2f928fff56