Bug 69346 - Assertion hit in JSC::DFG::SpeculativeJIT::compile on SL bots
Summary: Assertion hit in JSC::DFG::SpeculativeJIT::compile on SL bots
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-04 10:20 PDT by Ryosuke Niwa
Modified: 2011-10-05 15:59 PDT (History)
4 users (show)

See Also:


Attachments
the patch (6.97 KB, patch)
2011-10-05 15:13 PDT, Filip Pizlo
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2011-10-04 10:20:01 PDT
inspector/console/console-format.html is hitting an assertion in JSC::DFG::SpeculativeJIT::compile on Snow Leopard Intel Debug (Tests).

crash log: http://build.webkit.org/results/SnowLeopard%20Intel%20Debug%20(Tests)/r96591%20(2432)/inspector/console/console-format-crash-log.txt
Comment 1 Filip Pizlo 2011-10-04 17:20:38 PDT
Aha!  This looks like a broken assertion.  Fix forthcoming.
Comment 2 Filip Pizlo 2011-10-05 15:10:05 PDT
Full diagnosis:

DFG's OSR machinery assumes that each bytecode operation sets only one bytecode virtual register, and only does so when the operation has completed successfully.  But this is not the case for op_post_inc, which supports statements of the form "x = y++" where x and y are different variables.  This statement involves setting x to y's old value, then incrementing y (i.e. adding 1 to its old value and then setting it).

The assertion being hit in this failure was asserting that a SetLocal constitutes the end of a bytecode statement.  In op_post_inc, there are two SetLocals, with an ArithAdd in between.  Hence, it appears that OSR's assumptions, as well as the assertion, are wrong.

It turns out that the assumptions of OSR are pretty much right, but just need to be restated: in this case, x is dead prior to op_post_inc anyway.  So from OSR's standpoint, it's OK to execute the first SetLocal even if the ArithAdd or subsequent SetLocal will fail.

This what OSR is really assuming is that either a SetLocal constitutes the end of a bytecode operation, or its effect is harmless if the bytecode operation is executed again.

I don't know of an easy way to put this into an ASSERT statement, so I've removed the old ASSERT statement and am now verifying that this fixes the problem.
Comment 3 Filip Pizlo 2011-10-05 15:13:27 PDT
Created attachment 109872 [details]
the patch
Comment 4 Filip Pizlo 2011-10-05 15:59:17 PDT
Landed in r96762.