Summary: | Object.defineProperty should be able to create a PropertyDescriptor where m_attributes == 0 | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Hahnenberg <mhahnenberg> | ||||
Component: | JavaScriptCore | Assignee: | Mark Hahnenberg <mhahnenberg> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | darin | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Mark Hahnenberg
2013-08-26 10:03:46 PDT
Created attachment 209660 [details]
Patch
Comment on attachment 209660 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209660&action=review > Source/JavaScriptCore/ChangeLog:17 > + This is due to the fact that the lowest non-zero attribute (ReadOnly) is represented as 1 << 1 > + instead of 1 << 0. We then calculate the default attributes as ((1 << 3) << 1) - 1, which is 0xF, > + but only the top three bits mean anything. Even in the case above, the top three bits are set > + to 0 but the bottom bit remains set, which causes us to think m_attributes is non-zero. I think the real problem is in the code that calculates the default attributes. Where is that code? Is there some reason it has to do "((1 << 3) << 1) - 1" instead of just or'ing together the specific attributes? (In reply to comment #2) > (From update of attachment 209660 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=209660&action=review > > > Source/JavaScriptCore/ChangeLog:17 > > + This is due to the fact that the lowest non-zero attribute (ReadOnly) is represented as 1 << 1 > > + instead of 1 << 0. We then calculate the default attributes as ((1 << 3) << 1) - 1, which is 0xF, > > + but only the top three bits mean anything. Even in the case above, the top three bits are set > > + to 0 but the bottom bit remains set, which causes us to think m_attributes is non-zero. > > I think the real problem is in the code that calculates the default attributes. Where is that code? Is there some reason it has to do "((1 << 3) << 1) - 1" instead of just or'ing together the specific attributes? It's in PropertyDescriptor.cpp: unsigned PropertyDescriptor::defaultAttributes = (DontDelete << 1) - 1; I agree that it would be better style to or the default attributes together. I'll make that change. (In reply to comment #2) > (From update of attachment 209660 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=209660&action=review > > > Source/JavaScriptCore/ChangeLog:17 > > + This is due to the fact that the lowest non-zero attribute (ReadOnly) is represented as 1 << 1 > > + instead of 1 << 0. We then calculate the default attributes as ((1 << 3) << 1) - 1, which is 0xF, > > + but only the top three bits mean anything. Even in the case above, the top three bits are set > > + to 0 but the bottom bit remains set, which causes us to think m_attributes is non-zero. > > I think the real problem is in the code that calculates the default attributes. Where is that code? Is there some reason it has to do "((1 << 3) << 1) - 1" instead of just or'ing together the specific attributes? Hmm, a couple of these constants are exposed as part of the JavaScriptCore framework's C API in JSObjectRef.h (e.g. kJSPropertyAttributeReadOnly). Perhaps it would be better to just change how the default attributes are calculated internally and leave the other values alone. Committed r154630: <http://trac.webkit.org/changeset/154630> (In reply to comment #4) > Hmm, a couple of these constants are exposed as part of the JavaScriptCore framework's C API in JSObjectRef.h (e.g. kJSPropertyAttributeReadOnly). Please add a COMPILE_ASSERT to keep these in sync. |