Bug 165728 – REGRESSION(r209653) Crash in CallFrameShuffler::snapshot()

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Michael Saboff]

* thread #1: tid = 0x1c5c6c9, 0x000000011031d738 JavaScriptCore`JSC::CachedRecovery::recovery(this=0x0000000000000000) const + 8 at CachedRecovery.h:115, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x000000011031d738 JavaScriptCore`JSC::CachedRecovery::recovery(this=0x0000000000000000) const + 8 at CachedRecovery.h:115
  * frame #1: 0x000000011101f2dd JavaScriptCore`JSC::CallFrameShuffler::snapshot(this=0x0000000113bb8500, argumentsLocation=RegisterArgs4InRegisters) const + 653 at CallFrameShuffler.h:122
    frame #2: 0x000000011101ddb1 JavaScriptCore`JSC::linkPolymorphicCall(exec=0x00007fff4fda2590, callLinkInfo=0x0000000113b82480, newVariant=CallVariant @ 0x00007fff4fda1bc8) + 7233 at Repatch.cpp:929
    frame #3: 0x0000000110be0648 JavaScriptCore`::operationLinkPolymorphicCall(execCallee=0x00007fff4fda2590, callLinkInfo=0x0000000113b82480) + 152 at JITOperations.cpp:1091
    frame #4: 0x00003c83a9c00f7a
...

snapshot() can't handle a ValueRecovery targeted to multiple argument registers.

[Michael Saboff]

Created attachment 296809 [details]
Patch

[Filip Pizlo]

Comment on attachment 296809 [details]
Patch

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

This change is right.  It needs a careful rewording of the comment above m_newRegisters.  Currently that comment says that m_newRegisters lists *all* of the registers that the shuffler will write to, which would imply that the old code before this change would have been correct.  That comment needs to indicate that because the shuffler broadcasts to the "extra targets" after the fact, m_newRegisters only need to know about the master register for a recovery.

> Source/JavaScriptCore/jit/CallFrameShuffler.h:228
> -    static const bool verbose = false;
> +    static const bool verbose = true;

Revert.

[Build Bot]

Comment on attachment 296809 [details]
Patch

Attachment 296809 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2690343

New failing tests:
workers/bomb.html

[Build Bot]

Created attachment 296813 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Michael Saboff]

Created attachment 296814 [details]
Patch for Landing

[Build Bot]

Comment on attachment 296809 [details]
Patch

Attachment 296809 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2690372

New failing tests:
workers/bomb.html

[Build Bot]

Created attachment 296815 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Michael Saboff]

Committed r209673: <http://trac.webkit.org/changeset/209673>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 165739

[Michael Saboff]

Relanded with fix in r209725: <http://trac.webkit.org/changeset/209725>