x = "" Object.freeze(this) for (var a = 0; a < 10; ++a) { --x } crashes js debug shell on TM changeset 9123f97f059c with -j when passed in as a CLI argument at JSString::length, and crashes js opt shell with -j at js_StringToNumber. Seems to be a null deref but locking s-s just-in-case. dbg console output: Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_PROTECTION_FAILURE at address: 0x00000000 0x000182c5 in JSString::length (this=0x0) at jsstr.h:252 252 return mLengthAndFlags >> FLAGS_LENGTH_SHIFT; (gdb) bt #0 0x000182c5 in JSString::length (this=0x0) at jsstr.h:252 #1 0x00246bbc in js::StringToNumberType<double> (cx=0x60aa00, str=0x0) at jsnum.h:649 #2 0x00246db8 in js_StringToNumber (cx=0x60aa00, str=0x0) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-58010-9123f97f059c/compilePath/jsbuiltins.cpp:173 #3 0x007dcf98 in ?? () #4 0x001cabe1 in js::ExecuteTrace (cx=0x60aa00, f=0x86a154, state=@0xbfffe784) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-58010-9123f97f059c/compilePath/jstracer.cpp:6410 #5 0x001d89d8 in js::ExecuteTree (cx=0x60aa00, f=0x86a154, inlineCallCount=@0xbffff4f8, innermostNestedGuardp=0xbfffe878, lrp=0xbfffe87c) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-58010-9123f97f059c/compilePath/jstracer.cpp:6512 #6 0x001ff7a2 in js::RecordLoopEdge (cx=0x60aa00, inlineCallCount=@0xbffff4f8) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-58010-9123f97f059c/compilePath/jstracer.cpp:7061 #7 0x001ff988 in js::MonitorLoopEdge (cx=0x60aa00, inlineCallCount=@0xbffff4f8) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-58010-9123f97f059c/compilePath/jstracer.cpp:16944 #8 0x000c59df in js::Interpret () at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-58010-9123f97f059c/compilePath/jsinterp.cpp:3542 #9 0x000e3c74 in js::RunScript (cx=0x60aa00, script=0x612ed0, fp=0x1000030) at jsinterp.cpp:657 #10 0x000e4216 in js::Execute (cx=0x60aa00, chain=0x1502028, script=0x612ed0, prev=0x0, flags=0, result=0x0) at jsinterp.cpp:1005 #11 0x00022da1 in JS_ExecuteScript (cx=0x60aa00, obj=0x1502028, script=0x612ed0, rval=0x0) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-58010-9123f97f059c/compilePath/jsapi.cpp:4837 #12 0x0001555c in Process (cx=0x60aa00, obj=0x1502028, filename=0xbffff94f "uinAssert.js", forceTTY=0) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-58010-9123f97f059c/compilePath/shell/js.cpp:453 #13 0x0001646a in ProcessArgs (cx=0x60aa00, obj=0x1502028, argv=0xbffff830, argc=2) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-58010-9123f97f059c/compilePath/shell/js.cpp:952 #14 0x000165a8 in Shell (cx=0x60aa00, argc=2, argv=0xbffff830, envp=0xbffff83c) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-58010-9123f97f059c/compilePath/shell/js.cpp:5363 #15 0x0001670f in main (argc=2, argv=0xbffff830, envp=0xbffff83c) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-58010-9123f97f059c/compilePath/shell/js.cpp:5471 (gdb) x/i $eip 0x182c5 <_ZNK8JSString6lengthEv+9>: mov (%eax),%eax (gdb) x/b $eax 0x0: Cannot access memory at address 0x0

Adding a print function turns into an assert: x = "" Object.freeze(this) for (var a = 0; a < 10; ++a) { print(--x) } at Assertion failure: *(uint32 *)slot != 0

Due to cross compile breakage, attached is the regression window.

The first bad revision is: changeset: 19f70f8c2b88 user: Boris Zbarsky date: Thu Nov 04 16:37:44 2010 -0400 summary: Bug 605858. Trace inc() for all primitive values, not just numbers. r=dvander

Interesting. In a 64-bit build, I see 0xbff0000000000000 being passed in for the JSString*, not 0x00000000 as in the 32-bit builds here...

If I do the print() thing, on the other hand, I don't get any crashes or asserts. I just get -1 printed 10 times. I have no idea what that value in 64-bit builds means... I assume the issue here is that the write failing somehow confuses the tracer.

So.... Is it expected that this testcase: x = 0; Object.freeze(this) for (var a = 0; a < 10; ++a) { x--; } print(x); prints -9? If I use |x = x - 1| instead then I get 0... For that matter, how come the |var a| thing and modifying |a| works at all after freezing the global? I'd expect this to go into an infinite loop, but it doesn't seem to even in v8 (where I do have to move the |var a| to before the freeze() call; v8 prints |0| for the above testcase with that modification). Clearly gnamedec is doing something weird here that bypasses the freeze... Addressing that may well fix this bug, since I assume the problem is that the tracer and interp end up disagreeing on the type of x after one iteration.

Oh, for extra fun mjit _does_ print 0 for the testcase in comment 6. Both tjit and interp print -9.

And -m -j prints -13. :)

For what it's worth, if I change the string to be "-1" instead of "", then the crash is at address 0xc000000000000000 so it really does seem like we're just sticking the integer in there in some sort of mangled way.

At least for interp, the issue seems to be the fast path in JSOP_GNAMEDEC and company for this case: if (obj == obj2 && entry->vword.isSlot()) { If I disable that fast path, I start getting 0 for interp on the comment 6 testcase. tjit starts giving me -2. I have no idea what tjit is doing that's also forgetting to check the READONLY flag.... And I still don't understand how the ++a thing manages to work! In v8, it depends on whether I make the line before the freeze() call |var a| or |var a = 0| (the latter gives the infinite loop I kinda expected; the former does not).

This is a real comedy of errors. The loop in comment 0 shouldn't execute at all, because > Object.freeze(this) > for (var a = 0; a < 10; ++a) { > --x > } First the declaration of a results in a global, writable, non-configurable property `a`. Its value, initially, is undefined. Then Object.freeze() makes this property non-writable. So far so good. However, the `a = 0` should do nothing, and then `a < 10` should be false, because a === undefined. The bug with `--x` is easy, but this bug with assigning to `a` is a pain. I don't see how to fix it without slowing down SETGLOBAL in the methodjit and interpreter by at least a shape check (tracer is unaffected). The easiest fix might be not to emit SETGLOBAL.

Do we also have this bug if we move the code into a function so that |a| is a local but |x| remains global? The loop would execute then, right?

This doesn't fix the problem with {SET,FOR,INC,DEC}GLOBAL that it writes even if the target global has become non-writable. But the crash, afaict, can only be triggered by the INCGNAME instructions; this fixes that

For the record: The first time the interpreter sees SETGNAME, it puts a bogus entry into the property cache. However, because the value of global.x is a string, the interpreter always ignores the property cache entry. So the interpreter's behavior is correct, apart from the bogus entry. At record time, we emit code for `--x` that takes the string value of x, converts it to a number, decrements it, and stores it back. However the interpreter again does not store. The tracer does not remember that it has emitted code to write a number to x. Instead it relies on the real x (which is a string) to match the emitted code (it doesn't). When we get to the end of the trace, the tracer joins the loop, because the typemaps match. The tracer has forgotten that it emitted code to change a type. At run time, the first iteration assigns 1 to the tracer's version of the global x. The second crashes, trying to treat that 1 as a string pointer.

(In reply to comment #12) > Do we also have this bug if we move the code into a function so that |a| is a > local but |x| remains global? The loop would execute then, right? Yes. The test in the patch uses a different workaround: it uses Object.defineProperty rather than Object.freeze to make x non-writable.

http://hg.mozilla.org/tracemonkey/rev/68de0c5b6255 Follow-up bug 617171 is about the GLOBAL opcodes.

JSBugMon: This bug has been automatically verified fixed.