Closed Bug 617171 Opened 9 years ago Closed 9 years ago

JOF_GLOBAL opcodes can change the value of a non-writable property

Categories

(Core :: JavaScript Engine, defect)

Other Branch
defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 618007
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: jorendorff, Unassigned)

References

Details

(Whiteboard: softblocker)

var a = 6;
Object.defineProperty(this, "a", {writable: false});
a = 7;  // JSOP_SETGLOBAL
assertEq(a, 6);

jsemit sees the global `var a` and (in compile-n-go with a suitable global) can tell that `a` will be a plain old var at run time. So we emit a special SETGLOBAL opcode for `a = 7` and burn the slot index right into the bytecode.

Unfortunately the implementations of these GLOBAL opcodes incorrectly assume that the target property will remain writable. So we write the slot without checking. The bug exists in the interpreter, methodjit, and tracer.

It is easiest to fix in the tracer, because global shape changes force us off trace. The situation thus can be detected or ruled out at compile time: no run time perf hit.
Blocks: es5
In methodjit, a JOF_GLOBAL op needs a one-slot cache for the expected shape of the global. At run time, a guard checks the global shape. The slow (mismatch) path checks that the property is still writable, and if so, refills the cache.

Is there code in MonoIC or elsewhere that I should be reusing for this?
Yes, this is exactly how the JSOP_GNAME caches behave.  I think that for now JSOP_GLOBAL opcodes should be compiled by JM using the jsop_setgname etc. methods (main issue is the bytecode immediate format is different).  Once we have invalidation/recompilation (post-4.0, though the JM branch already does this for type inference), JSOP_GLOBAL opcodes can be made fast again (along with, for that matter, JSOP_GNAME opcodes).
Duplicate of this bug: 620761
Without this, the purportedly-nonwritable undefined, NaN, and Infinity global properties are writable.  The same goes for any non-writable properties on the global object.  This is a pretty bad hole in spec compliance, seems to me.
blocking2.0: --- → ?
blocking2.0: ? → betaN+
Whiteboard: softblocker
(In reply to comment #4)
> Without this, the purportedly-nonwritable undefined, NaN, and Infinity global
> properties are writable.  The same goes for any non-writable properties on the
> global object.  This is a pretty bad hole in spec compliance, seems to me.

Interestingly, I find that as of tip 'var NaN = 7' changes the value of NaN, but no other assignment does, including 'var NaN; NaN = 7' and 'NaN = 7;' without any var declaration. (This the shell, without any jits running.)

The original test case still fails in all modes.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 618007
Test case (it passes now, comment 5 must have been before the fix):
https://hg.mozilla.org/tracemonkey/rev/f5b3294a3180
You need to log in before you can comment on or make changes to this bug.