Bug 78145 – updateTopCallframe in the baseline JIT doesn't provide enough information to the stubs

• Status: REOPENED
• Component: New Bugs

[Oliver Hunt]

updateTopCallframe in the baseline JIT doesn't provide enough information to the stubs

[Oliver Hunt]

Created attachment 126138 [details]
Patch

[Oliver Hunt]

Committed r107126: <http://trac.webkit.org/changeset/107126>

[Csaba Osztrogonác]

Reopen, because it broke zillion tests on Qt ARM, see https://bugs.webkit.org/show_bug.cgi?id=78237 for details.

[Oliver Hunt]

Ossy, can you attach the text of the assertion failure?  It should include correct offset info -- all this should require is changing a single number, I thought i had it right already

[Csaba Osztrogonác]

(In reply to comment #4)
> Ossy, can you attach the text of the assertion failure?  It should include correct offset info -- all this should require is changing a single number, I thought i had it right already

Unfortunately I can't run tests on ARM board (and the bot didn't give us useable logs), only Zoltán and Gábor can, but they went home. 

I hope they can check it tomorrow.

[Oliver Hunt]

(In reply to comment #5)
> (In reply to comment #4)
> > Ossy, can you attach the text of the assertion failure?  It should include correct offset info -- all this should require is changing a single number, I thought i had it right already
> 
> Unfortunately I can't run tests on ARM board (and the bot didn't give us useable logs), only Zoltán and Gábor can, but they went home. 
> 
> I hope they can check it tomorrow.

Gah,  I'll work through the maths again and see if I get a different size for the ARM path.

[Zoltan Herczeg]

Comment on attachment 126138 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=126138&action=review

> Source/JavaScriptCore/jit/JITInlineMethods.h:269
> +    ASSERT(static_cast<int>(m_bytecodeOffset) >= 0);
> +    if (m_bytecodeOffset)
> +        store32(Imm32(m_bytecodeOffset + 1), intTagFor(RegisterFile::ArgumentCount));

This is not necessary fixed length in ARM. (And what happens if m_bytecodeOffset == 0?)

Would it be possible to move this to a point where patchOffsetGetByIdSlowCaseCall unaffected?

[Oliver Hunt]

(In reply to comment #7)
> (From update of attachment 126138 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=126138&action=review
> 
> > Source/JavaScriptCore/jit/JITInlineMethods.h:269
> > +    ASSERT(static_cast<int>(m_bytecodeOffset) >= 0);
> > +    if (m_bytecodeOffset)
> > +        store32(Imm32(m_bytecodeOffset + 1), intTagFor(RegisterFile::ArgumentCount));
> 
> This is not necessary fixed length in ARM. (And what happens if m_bytecodeOffset == 0?)

You hit m_bytecodeOffset = 0 in some of the very early codeine, basically stack bounds checking, etc - you're guaranteed not to be in a fixed size region.

The only times we depend on exact code lengths are inside blocks where we have told the assembler to emit fixed size instructions.  If the assembler is not doing that, it's a bug in the assembler.


> 
> Would it be possible to move this to a point where patchOffsetGetByIdSlowCaseCall unaffected?

Nope, we need it in the general updateTopCallFrame case.