Open Bug 598225 Opened 14 years ago Updated 2 years ago

CTypes Function values are no longer read-only

Categories

(Core :: js-ctypes, defect)

x86_64
Linux
defect

Tracking

()

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:
blocking2.0: --- → beta7+
Blocks: 541212
Blocks: 492849
Assignee: nobody → dwitte
This is my top priority.
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.
JS_FreezeObject shouldn't have any effect on own setters.
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.
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
Re-noming for final -- see comment 5.
blocking2.0: beta7+ → ?
blocking2.0: ? → betaN+
blocking2.0: betaN+ → -
Reassigning to nobody. If anyone wants to work on this, feel free!
Assignee: dwitte → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.