1317416 - GetPrototypeFromConstructor needs to retrieve the fallback prototype from the constructor's realm

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[André Bargull [:anba]]

Test case from test262:
---
var other = newGlobal();

var C = new other.Function();
C.prototype = null;

class B extends function() {} {
  constructor() {
    super();
  }
}

var b = Reflect.construct(B, [], C);

print(Object.getPrototypeOf(b) === other.Object.prototype);
---

Expected: Prints "true"
Actual: Prints "false"


ES2017 spec, 9.1.14 GetPrototypeFromConstructor, step 4.a:
https://tc39.github.io/ecma262/#sec-getprototypefromconstructor

Comment 2

[Boris Zbarsky [:bzbarsky]]

To be clear, "The constructor's realm" is NOT the same thing as ctor->global() in spidermonkey, because the spec has no concept of realms for anything except function objects.  Since bound functions and Proxy objects are not function objects, what's supposed to happen is that the realm of the underlying callable gets used.  And for other non-function callables the spec just gives up and uses current realm.  See https://tc39.github.io/ecma262/#sec-getfunctionrealm

Comment 3

[Jason Orendorff [:jorendorff]]

I have a patch that mostly fixes this. I'm going to move the remainder to another bug.

Comment 4

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1317416 - GetPrototypeFromConstructor needs to retrieve the fallback prototype from the constructor's realm. r?jandem

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1ce15a2a79e0
GetPrototypeFromConstructor needs to retrieve the fallback prototype from the constructor's realm. r=jandem

Comment 6

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/1ce15a2a79e0

Comment 7

[Boris Zbarsky [:bzbarsky]]

I'm a little confused. It sure looks like we're basically using ctor->global() (after unwrapping cross-compartment wrappers), which per comment 2 is not what the spec says to do here...

Is that the part that's going to another bug? What is that other bug?

(Also, we still need jsapi for this stuff to fix bug 1317658... I was under the impression that it would get added here, but maybe we need a separate bug for that too?)

Comment 8

[Jason Orendorff [:jorendorff]]

Yes, we're basically using ctor->global().

This is correct because GetPrototypeFromConstructor is called exclusively from the builtin constructors themselves. If it's possible to get to any of these points while args.callee() is a proxy or bound function, we have a bug. But I don't think that's possible. For bound functions, here's what I observe:

let g = newGlobal({newCompartment:true});
let C = g.Function.prototype.bind.call(Object);
let obj = new C;
assertEq(obj.__proto__, Object.prototype);  // not g.Object.prototype

C is a CCW to a bound function to a CCW to Object. Everything seems to go through as expected.

The fact that args.callee() was originally C is not carried forward into the nested invocations. Thus when the Object constructor runs, args.callee() and args.newTarget() are both Object, not the bound function object or a cross-compartment wrapper.

Comment 9

[Jason Orendorff [:jorendorff]]

(has creeping feeling Xray wrappers can blow all this up)

Comment 10

[Boris Zbarsky [:bzbarsky]]

This is correct because GetPrototypeFromConstructor is called exclusively from the builtin constructors themselves.

It's called from the builtin constructors, but the thing passed to GetPrototypeFromConstructor is not args.callee(); it's args.newTarget(). Which could be absolutely anything, including proxy or bound function, no?

The fact that args.callee() was originally C is not carried forward into the nested invocations

Sure, if you're invoking C via new C. But here's the testcase I'm thinking about:

let g = newGlobal({newCompartment:true});
let C = g.Function.prototype.bind.call(Object);
let obj = Reflect.construct(Object, [], C);
assertEq(obj.__proto__, Object.prototype);

That assert should pass per spec, afaict. I believe it fails with this patch.

Comment 11

[Jason Orendorff [:jorendorff]]

Continuing reply to comment 7:

Is that the part that's going to another bug? What is that other bug?

The other bug is bug 1515167, and it does cover at least bound functions.

(Also, we still need jsapi for this stuff to fix bug 1317658... I was under the impression that it would get added here, but maybe we need a separate bug for that too?)

Maybe that's bug 1275312?

Comment 12

[Boris Zbarsky [:bzbarsky]]

Maybe that's bug 1275312?

Yes, perfect, thanks.

Comment 13

[Jason Orendorff [:jorendorff]]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #10)

This is correct because GetPrototypeFromConstructor is called exclusively from the builtin constructors themselves.

It's called from the builtin constructors, but the thing passed to GetPrototypeFromConstructor is not args.callee(); it's args.newTarget(). Which could be absolutely anything, including proxy or bound function, no?

Yes, any callable object. Sorry, I was looking at the happy case in GetPrototypeFromBuiltinConstructor, which isn't even the code affected by this patch.

You're right, this was a half measure. The patch does not do a proper implementation of GetFunctionRealm. I just wanted to get intrinsicDefaultProto threaded through so I could scratch off a bunch of test262 tests.

Your test case in comment 10 does fail, as you expect.