[webkit-reviews] review requested: [Bug 28317] Assertion being hit on layout tests in debug build : [Attachment 38646] Speculative fix

[bugzilla-daemon at webkit.org]

Gavin Barraclough <barraclough at apple.com> has asked  for review:
Bug 28317: Assertion being hit on layout tests in debug build
https://bugs.webkit.org/show_bug.cgi?id=28317

Attachment 38646: Speculative fix
https://bugs.webkit.org/attachment.cgi?id=38646&action=review

------- Additional Comments from Gavin Barraclough <barraclough at apple.com>
// stubCall.addArgument(ImmPtr(ident));
0x7fffe66e34d1: movq   $0xd0c308,0x10(%rsp)

Hmmm, okay, this is the problem, we're relying on a slightly OS X specific
behaviour here (this accounts for the 6 byte difference - this instuction is 9
bytes & we expect 15).

64-bit Mac OS applications have a 4Gb zero page, so pointers are never
representable as a 32-bit integer, and always have to be represented by a
separate immediate load instruction, rather than within the immediate field of
an arithmetic or memory operation.  The problem is that you're hitting a
pointer low in memory, and the MacroAssembler is finding a tighter encoding.

We explicitly check for a couple of cases where a value might be representable
in 32-bit, but these probably never kick in on Mac OS, and only kick in on GTK
to screw you over and break you here (sorry!). :-)  For now, I think we can
just cheerfully remove these, and hopefully this will fix the problem for you
(rather than coming up with any more complex solution  - since we don't expect
these to really be hit right now, we don't really expect these to be
benefitting us right now, either performance or memory wise).  On x86-64 we
probably just want to stick to a general policy of never trying to compress
pointers (on Arm plarforms we have to do something move complex, since ImmPtrs
are always converted to Imm32s, so we have to track within the Imm32 whether
the value is allowed to be compressed in a tighter encoding).

I don't have a linux box to test on, but I think the attacked patch should fix
things for you.