Bug 78145 - updateTopCallframe in the baseline JIT doesn't provide enough information to the stubs
Summary: updateTopCallframe in the baseline JIT doesn't provide enough information to ...
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords:
Depends on: 78237
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-08 13:02 PST by Oliver Hunt
Modified: 2012-02-10 10:03 PST (History)
1 user (show)

See Also:


Attachments
Patch (9.09 KB, patch)
2012-02-08 13:05 PST, Oliver Hunt
barraclough: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2012-02-08 13:02:10 PST
updateTopCallframe in the baseline JIT doesn't provide enough information to the stubs
Comment 1 Oliver Hunt 2012-02-08 13:05:31 PST
Created attachment 126138 [details]
Patch
Comment 2 Oliver Hunt 2012-02-08 13:23:11 PST
Committed r107126: <http://trac.webkit.org/changeset/107126>
Comment 3 Csaba Osztrogonác 2012-02-09 07:30:12 PST
Reopen, because it broke zillion tests on Qt ARM, see https://bugs.webkit.org/show_bug.cgi?id=78237 for details.
Comment 4 Oliver Hunt 2012-02-09 09:08:36 PST
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
Comment 5 Csaba Osztrogonác 2012-02-09 09:12:34 PST
(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.
Comment 6 Oliver Hunt 2012-02-09 09:15:02 PST
(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 7 Zoltan Herczeg 2012-02-10 03:28:59 PST
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?
Comment 8 Oliver Hunt 2012-02-10 10:03:34 PST
(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.