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

RESOLVED FIXED in Firefox 66

Status

()

defect
P3
normal
RESOLVED FIXED
3 years ago
6 months ago

People

(Reporter: anba, Assigned: jorendorff)

Tracking

(Blocks 1 bug, {triage-deferred})

Trunk
mozilla66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(1 attachment)

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
Blocks: 1317658
Duplicate of this bug: 1346071
Keywords: triage-deferred
Priority: -- → P3
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
I have a patch that mostly fixes this. I'm going to move the remainder to another bug.
Assignee: nobody → jorendorff
Pushed by jorendorff@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1ce15a2a79e0
GetPrototypeFromConstructor needs to retrieve the fallback prototype from the constructor's realm. r=jandem
Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

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?)

Flags: needinfo?(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.

Flags: needinfo?(jorendorff) → needinfo?(bzbarsky)

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

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.

Flags: needinfo?(bzbarsky) → needinfo?(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?

Maybe that's bug 1275312?

Yes, perfect, thanks.

(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.

Flags: needinfo?(jorendorff)
You need to log in before you can comment on or make changes to this bug.