Closed Bug 1006718 Opened 11 years ago Closed 11 years ago

Be safer about our slot counts for DOM objects

Categories

(Core :: DOM: Core & HTML, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Details

Attachments

(1 file)

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+
Flags: in-testsuite-
Whiteboard: [need review]
Status: NEW → RESOLVED
Closed: 11 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.

Attachment

General

Created:
Updated:
Size: