Closed Bug 1463163 Opened Last year Closed Last year
Species Create realm check work with same-compartment realms
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: nobody → jdemooij
Status: NEW → ASSIGNED
Summary: Make CreateArraySpecies realm check work with same-compartment realms → Make ArraySpeciesCreate realm check work with same-compartment realms
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 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 firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/07b0a9838f2d Make ArraySpeciesCreate realm check work with same-compartment realms. r=anba
You need to log in before you can comment on or make changes to this bug.