Closed Bug 1463163 Opened 6 years ago Closed 6 years ago

Make ArraySpeciesCreate realm check work with same-compartment realms

Categories

(Core :: JavaScript Engine, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(1 file)

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.
Priority: -- → P3
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 jandemooij@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/07b0a9838f2d Make ArraySpeciesCreate realm check work with same-compartment realms. r=anba
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: