544874 - Jit incorrectly marks result of OP_urshift as int instead of uint

Status: VERIFIED FIXED

(Tamarin Graveyard :: Baseline JIT (CodegenLIR), defect, P2)

Description

[Edwin Smith]

The verifier corrects it later when it calls FrameState::setType(), but it's still a bug because the debugger will see the wrong type when debugging JIT'd code, and when the JIT is decoupled from the verifier, it will simply be wrong.

jit:
        case OP_urshift:
            emit(state, opcode, 0, 0, INT_TYPE);    <-- should be UINT_TYPE
            break;

verifier:
        case OP_urshift:
            checkStack(2,1);
            emitCoerce(INT_TYPE, sp-1);       <-- should be UINT_TYPE, but benign
            emitCoerce(INT_TYPE, sp);
            coder->write(state, pc, opcode);
            state->pop_push(2, UINT_TYPE);    <-- already correct
            break;

Comment 1

[Edwin Smith]

Attachment: result of jit urshift is uint, at least for the debugger's sake.

Comment 2

[Edwin Smith]

pushed http://hg.mozilla.org/tamarin-redux/rev/5a6a0a97d224

Comment 3

[Edwin Smith]

without the patch applied, it should be possible to make a test that detects the uint/int difference for OP_urshift, by doing (x >>> 0) where x is some value with its high bit set.  if you stop in the debugger after the urshift, and inspect the value (when the method is jitted), the bug is you'll see a negative number when you should see a large positive number.  tricky but doable.

Comment 4

[Chris Peyer]

Since a debugger testcase is not possible, can we create an abcasm testcase?  I was unable to create a testcase with limited trying on my part.

Comment 5

[Edwin Smith]

The debugger was the only thing potentially exposed to this bug.  I beleive we have asserts in place now that adequately cover the potential for a future problem, so i'm clearing the in-testsuite flag.