Closed
Bug 1463163
Opened 7 years ago
Closed 6 years ago
Make ArraySpeciesCreate realm check work with same-compartment realms
Categories
(Core :: JavaScript Engine, enhancement, P3)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
References
Details
Attachments
(1 file)
(deleted),
patch
|
anba
:
review+
|
Details | Diff | Splinter Review |
If I understand the spec correctly, this involves:
* IsArraySpecies can take the fast path if cx->realm() != constructor->realm().
* IsWrappedArrayConstructor should be renamed to IsCrossRealmArrayConstructor and it should also check for non-wrapped array constructors with realm() != cx->realm().
* Ion inlining of this intrinsic needs to check group->realm() in addition to checking for proxy classes.
This should be straight-forward but we should just wait until we can actually test this or get test failures.
Assignee | ||
Updated•7 years ago
|
Priority: -- → P3
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Assignee | ||
Updated•6 years ago
|
Summary: Make CreateArraySpecies realm check work with same-compartment realms → Make ArraySpeciesCreate realm check work with same-compartment realms
Assignee | ||
Comment 1•6 years ago
|
||
This makes the changes described in comment 0.
Note that TemporaryTypeSet::getKnownRealm is based on TemporaryTypeSet::getKnownClass, maybe that makes it easier to review.
This also fixes the remaining jsreftest failures when I change the test262 shell harness to create same-compartment realms for $262.createRealm().
Attachment #8989145 -
Flags: review?(andrebargull)
Comment 2•6 years ago
|
||
Comment on attachment 8989145 [details] [diff] [review]
Make ArraySpeciesCreate realm check work with same-compartment realms
Review of attachment 8989145 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM!
::: js/src/builtin/Array.cpp
@@ +1028,3 @@
> return obj->is<JSFunction>() &&
> obj->as<JSFunction>().isNative() &&
> obj->as<JSFunction>().native() == ArrayConstructor;
Pre-existing: Can you change this to call |IsNativeFunction| to avoid a bit of code duplication? And always using IsNativeFunction for these check should also help to discover more places where realm checks are needed for same-compartment-realms.
::: js/src/vm/TypeInference.cpp
@@ +2367,5 @@
> + // return |false| so fail now before attaching any constraints.
> + if (clasp->isProxy() || getObject(i)->unknownProperties())
> + return nullptr;
> +
> + Realm* nrealm = hasSingleton(i) ? getSingleton(i)->nonCCWRealm() : getGroup(i)->realm();
Maybe assert `MOZ_ASSERT(hasSingleton(i) || hasGroup(i));` to give the reader a hint that only these two options can occur here?
Attachment #8989145 -
Flags: review?(andrebargull) → review+
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/07b0a9838f2d
Make ArraySpeciesCreate realm check work with same-compartment realms. r=anba
Comment 4•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in
before you can comment on or make changes to this bug.
Description
•