Be safer about our slot counts for DOM objects

RESOLVED FIXED in mozilla32

Status

()

defect
RESOLVED FIXED
5 years ago
5 months ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

unspecified
mozilla32
x86
macOS
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

There are a few issues here:

1)  Nothing ensures that DOM_INSTANCE_RESERVED_SLOTS over in JSSlots.h matches INSTANCE_RESERVED_SLOTS in codegen.  And in fact, they're out of sync: the latter is still 3, while the former is 1.

2)  Nothing ensures that for globals the number of cached/storeinslot things fits in the three reserved slots globals have.

While I was here I discovered that we don't correctly flag a StoreInSlot prop on a non-leaf interface if that interface is not concrete, which is clearly wrong: nothing will know to store the prop in the slot.

I ended up introducing a #define for the number of application slots on JS globals.  There was no need for more asserts in the engine, since there was already a static_assert about JSCLASS_GLOBAL_SLOT_COUNT matching an internal enum.
Jason, could you just look at the Class.h bit?
Attachment #8418227 - Flags: review?(peterv)
Attachment #8418227 - Flags: review?(jorendorff)
Comment on attachment 8418227 [details] [diff] [review]
Add some sanity static asserts about DOM object slot counts

Review of attachment 8418227 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/public/Class.h
@@ +423,5 @@
>  //
> +// JSCLASS_GLOBAL_APPLICATION_SLOTS is the number of slots reserved at
> +// the beginning of every global object's slots for use by the
> +// application.
> +#define JSCLASS_GLOBAL_APPLICATION_SLOTS 3

Great. Please also change the following code in js/src/vm/GlobalObject.h to use this macro as well:

>    /* Count of slots set aside for application use. */
>    static const unsigned APPLICATION_SLOTS = 3;
Attachment #8418227 - Flags: review?(jorendorff) → review+
Attachment #8418227 - Flags: review?(peterv) → review+
https://hg.mozilla.org/mozilla-central/rev/b766e7d111b9
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.