Closed Bug 1388110 Opened 4 years ago Closed 4 years ago
Set Reserved Slot self-hosting intrinsic is less safe than expected
The implementation of intrinsic_UnsafeSetReservedSlot uses setReservedSlot to set the slot, but when it's inlined by the JIT MStoreFixedSlot is used. This breaks for objects with > 16 reserved slots. The same applies to the associated get intrinsics.
I don't think we call these functions on arbitrary objects, so it doesn't really matter if some embedding has objects with more than 16 reserved slots.
That sounds right, these are self-hosted intrinsic and self-hosting code is not extendable through any API yet.
(In reply to Nicolas B. Pierron [:nbp] from comment #1) > I do not think we have any in the JS Shell I bumped into this adding more reserved slots to ModuleObject in bug 1374239.
This is not an issue for the DOM because the DOM doesn't really do self-hosted code so far. If we were to start doing that, it could absolutely be an issue; there is nothing preventing a DOM object from having more than 16 slots. In addition to that, some DOM objects are proxies and I'm quite sure this intrinsic does the wrong thing for a proxy, for which the slot-to-fixes-slot mapping is different from a normal object... I can't speak to Servo's situation; jdm might know.
No self-hosted code in Servo yet either.
Here's a minimal fix that stops us inlining in this case.
Assignee: nobody → jcoppeard
Attachment #8894941 - Flags: review?(nicolas.b.pierron)
Comment on attachment 8894941 [details] [diff] [review] bug1388110-slot-access-intrinsics Review of attachment 8894941 [details] [diff] [review]: ----------------------------------------------------------------- Sounds ok ::: js/src/jit/MCallOptimize.cpp @@ +2890,5 @@ > return InliningStatus_NotInlined; > uint32_t slot = uint32_t(arg->toConstant()->toInt32()); > > + // Don't inline if it's not a fixed slot. > + if (slot >= 16) nit: use NativeObject::MAX_FIXED_SLOT
Attachment #8894941 - Flags: review?(nicolas.b.pierron) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/e0d805b314ba Fix slot access intrinsics for objects with > 16 reserved slots r=nbp
This bug has a quality title.
You need to log in before you can comment on or make changes to this bug.