Bug 78145

Created attachment 126138 [details] Patch

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

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

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

(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.

(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.

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?

(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.