Closed
Bug 750582
Opened 12 years ago
Closed 12 years ago
IonMonkey: eager write on global non-writable properties. (jaeger/bug584647.js:2: Error: Assertion failed: got false, expected true)
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: nbp, Assigned: nbp)
References
Details
Attachments
(1 file, 1 obsolete file)
14.56 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
This bug can be reproduced with ./js --ion-eager ./jit-test/tests/jaeger/bug584647.js ./jit-test/tests/jaeger/bug584647.js:2: Error: Assertion failed: got false, expected true
Assignee | ||
Comment 1•12 years ago
|
||
Assignee: general → nicolas.b.pierron
Status: NEW → ASSIGNED
Attachment #619807 -
Flags: review?(dvander)
Assignee | ||
Comment 2•12 years ago
|
||
Another similar issue which implies changes of set-property (JSOP_SETNAME) code is: ./js --ion-eager ./jit-test/tests/jaeger/bug735161.js => ./jit-test/tests/jaeger/bug735161.js:4: Error: Assertion failed: got true, expected false As it look similar and modify similar code I am adding the changes to the current patch in addition to the suggestion to clean-up SetPropertyCache made by dvander on IRC.
Assignee | ||
Comment 3•12 years ago
|
||
* Check if the shape is writable. * Clean-up SetPropertyCache and extract isSetPropertyInlinable and isPropertyInlinable, rename the other predicate to isAddPropertyInlinable to clarify. * Add VMFunction to handle the bottom of SetPropertyOperation and use it inside SetPropertyCache to fix wrong use of SETNAME on the global object.
Attachment #619807 -
Attachment is obsolete: true
Attachment #619807 -
Flags: review?(dvander)
Attachment #620135 -
Flags: review?(dvander)
Comment on attachment 620135 [details] [diff] [review] Remove unexpected properties from the global object. Review of attachment 620135 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for doing this refactoring! This is much more readable than the existing code. ::: js/src/ion/IonCaches.cpp @@ +449,5 @@ > return true; > } > > static bool > +isPropertyInlinable(JSObject *obj, IonCacheSetProperty &cache) nit: IsPropertyInlineable @@ +466,5 @@ > + return true; > +} > + > +static bool > +isSetPropertyInlinable(JSContext *cx, JSObject *obj, JSAtom *atom, jsid *pId, const Shape **pShape) nit: IsPropertySetInlineable @@ +492,5 @@ > +} > + > +static bool > +isAddPropertyInlinable(JSContext *cx, JSObject *obj, jsid id, uint32_t oldSlots, > + const Shape **pShape) nit: IsPropertyAddInlineable @@ +527,5 @@ > return true; > } > > bool > +js::ion::SetPropertyCache(JSContext *cx, size_t cacheIndex, JSObject *obj, const Value& value, We should probably make this gc-safe while we're here: change |JSObject *obj| to be |HandleObject obj| and |const Value &value| to be |HandleValue value|. ::: js/src/ion/VMFunctions.cpp @@ +288,5 @@ > } > > +bool > +SetProperty(JSContext *cx, JSObject *obj, JSAtom *atom, const Value &value, > + bool strict, bool isSetName) nit: bool should align with JSContext @@ +292,5 @@ > + bool strict, bool isSetName) > +{ > + Value v = value; > + jsid id = ATOM_TO_JSID(atom); > + RootObject objRoot(cx, &obj); |obj| isn't rooted, so we can't use RootObject (wow these names are confusing!) Instead, change |JSObject *obj| to |HandleObject obj|.
Attachment #620135 -
Flags: review?(dvander) → review+
Err -- sorry, we actually *can* use RootObject there, but I think we're supposed to propagate HandleObject instead
Assignee | ||
Comment 6•12 years ago
|
||
https://hg.mozilla.org/projects/ionmonkey/rev/68777651999a
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•