[Webkit-unassigned] [Bug 86330] Javascript doesn't behave the same with and without the debugger

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=86330





--- Comment #1 from Filip Pizlo <fpizlo at apple.com>  2012-05-13 22:13:57 PST ---
(In reply to comment #0)
> Using r116899.
> 
> I hope you won't hate me too much for this bug report. I've been told on #webkit to report it anyways.
> 
> I'm working on a Playstation emulator written in Javascript. It generates Javascript code from MIPS instructions and executes them; but when the debugger is not attached, the execution goes wrong. At some point, the value 0x80054664 should be assigned to element #31 of a Uint32Array (called "this.gpr") through a very simple statement:
> 
>     this.gpr[31] = 0x8005465c;
> 
> But the statement is either not executed or does not write the correct value, because instead I get this.gpr[31] == 0x80000000, which is the value it had before the assignment.
> 
> When the code is run with the Javascript debugger enabled, it does write 0x80054664, so the problem seems to be with the "production" JS compiler.
> 
> Also, when this.gpr[31] is not a typed array element (like if I make this.gpr an object with getters and setters for indices 0-33), everything goes right.
> 
> The bug is also resilient to mocking. Setting up the state of the emulator to something that simply "looks like" the conditions in which the bug happens (mocking the GPR state and the function generated) doesn't trigger it. I would very much like to give you a complete test case, but for this I would need to send a PlayStation BIOS, and I can't do that.
> 
> Still, I've joined the URL of the mock, so you can get an idea of what's going on. The generated Javascript code that doesn't work under obscure circumstances can be found in mock.js (the "psx.memory.compiled.compiled[0x8005465c]" part). For all practical purposes, the only part of this function that should be executed is from line 230 to line 233.
> 
> If there's any way I could be more helpful, please tell me how, I'll gladly do it.

Hi Felix!  Thanks for the detailed bug report!  I think I just found the bug.  In dfg/DFGSpeculativeJIT.cpp, there is the following code:

        JSValue jsValue = valueOfJSConstant(valueUse.index());
        if (!jsValue.isNumber()) {
            terminateSpeculativeExecution(Uncountable, JSValueRegs(), NoNode);
            noResult(m_compileIndex);
            return;
        }
        double d = jsValue.asNumber();
        if (rounding == ClampRounding) {
            ASSERT(elementSize == 1);
            d = clampDoubleToByte(d);
        }
        GPRTemporary scratch(this);
        GPRReg scratchReg = scratch.gpr();
        m_jit.move(Imm32(static_cast<int>(d)), scratchReg);
        value.adopt(scratch);
        valueGPR = scratchReg;

This is in the compilePutByValForIntTypedArray() method.  I believe that the offending line is "m_jit.move(Imm32(static_cast<int>(d)), scratchReg);".  Basically, we're doing a C++ cast from a double value (we will treat 0x8005465c as a double because it is outside the int32 range) to an int.  But C++ casting, at least on most common architectures (x86), implies that double values outside the int32 range result in 0x80000000, which is a special signaling value indicating that the cast overflowed.

I believe that the correct fix is to change "static_cast<int>(d)" to "toInt32(d)".

But to confirm that this is really the problem, can you tell me if the following hold true:

1) The real code (not the mocked version) does not have a switch statement.  Currently our optimizing compiler does not yet support switch statements.  This might be the reason why your mocked code does not trigger the bug.

2) The real code does indeed store a constant into the UInt32Array, as opposed to storing a non-constant value.  In particular, the code does this:

this.gpr[31] = 0x8005465c;

and not this:

var x = 0x8005465c;
... bunch of stuff
this.gpr[31] = x;

Does that sound about right?

I'm working on a reduced test case and a fix now, assuming that my guess is right.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.