Open
Bug 598225
Opened 14 years ago
Updated 2 years ago
CTypes Function values are no longer read-only
Categories
(Core :: js-ctypes, defect)
Tracking
()
NEW
Tracking | Status | |
---|---|---|
blocking2.0 | --- | - |
People
(Reporter: jimb, Unassigned)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
With the patch from 492849 applied, CType function values are no longer treated as read-only: assignments to 'value' succeed. The following code in FunctionType::ConstructData seems to expect that calling JS_FreezeObject will prevent such assignments:
// Seal the CData object, to prevent modification of the function pointer.
// This permanently associates this object with the closure, and avoids
// having to do things like reset SLOT_REFERENT when someone tries to
// change the pointer value.
// XXX This will need to change when bug 541212 is fixed -- CData::ValueSetter
// could be called on a frozen object.
return JS_FreezeObject(cx, dataObj);
This code used to call the (now removed) JS_SealObject, which made even JSPROP_READONLY|JSPROP_SHARED properties read-only. JS_FreezeObject marks existing properties read-only, but doesn't manage to do so for readonly+shared properties.
This causes regressions like the following:
TEST-UNEXPECTED-FAIL | /home/jimb/moz/tm/obj-bug/_tests/xpcshell/toolkit/components/ctypes/tests/unit/test_jsctypes.js | expected Error exception, none thrown - See following stack:
Updated•14 years ago
|
blocking2.0: --- → beta7+
Updated•14 years ago
|
Assignee: nobody → dwitte
Comment 1•14 years ago
|
||
This is my top priority.
Comment 2•14 years ago
|
||
Something's strange here. We have an object 'foo' which we define a property 'bar' on, with a getter and setter and flags JSPROP_SHARED | JSPROP_PERMANENT. If we attempt to assign to 'foo.bar', the setter is called.
If we then freeze 'foo', the setter no longer gets called, and the assignment silently returns. But from what you said above, JS_FreezeObject should not have any effect on JSPROP_SHARED | JSPROP_PERMANENT properties, no? In this case, we want the setter to be called.
Reporter | ||
Comment 3•14 years ago
|
||
JS_FreezeObject shouldn't have any effect on own setters.
Comment 4•14 years ago
|
||
OK. So, in js_SetPropertyHelper, shape->attrs has JSPROP_READONLY set for this case. Which makes the function bail before calling the setter.
<jimb> So...
<jimb> JSObject::sealOrFreeze
<jimb> see "make data attributes read-only"
<jimb> It's thinking that property is a data property, and setting its read-only bit
<jimb> the assignment thus fails silently, as per ES5.
[...]
<jimb> But the other question is whether value should get JSPROP_READONLY set.
<jimb> It has a getter/setter at the JSScope level, but at the JS level I think it's an ordinary data property.
<jimb> In the inherited case, we're treating it as if it had a setter. In the own case, we're checking the read-only bit.
I'm not really sure if this is desired behavior, but regardless, I don't think it really matters: this is a corner case for ctypes tests, and we can just run in strict mode to get the old behavior back. It doesn't really affect any important functionality.
Comment 5•14 years ago
|
||
We need bug 599139 in order to seal things properly here without breaking. In the meantime, the relevant tests have been disabled, and we weren't sealing properly before anyway, so I don't think this needs to block beta7. Should block final, though.
Bug 492849 also doesn't need to depend on this (but it might need to depend on bug 599139!).
Depends on: 599139
Updated•14 years ago
|
blocking2.0: ? → betaN+
Updated•14 years ago
|
blocking2.0: betaN+ → -
Comment 7•14 years ago
|
||
Reassigning to nobody. If anyone wants to work on this, feel free!
Assignee: dwitte → nobody
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•