Here is an example: var x = "PASS"; try { x = (new Number(1)).toString(0); } catch (e) { } print(x); On trunk, it prints PASS, but on SquirrelFish it prints an exception. This is because the VM_CHECK_EXCEPTION(); statements in many opcodes in Machine.cpp are after the writes to registers, which may store local variables. It shouldn't be too hard to move all of the checks out, and hopefully it won't be another performance regression due to compiler insanity.
This has been fixed in r32756 except for the exception check on returns from native calls.
Created attachment 20920 [details] Proposed patch
Since r32756 is implicated in a performance regression we should probably hold off on doing more of the same.
(In reply to comment #1) > This has been fixed in r32756 except for the exception check on returns from > native calls. > That rev only fixed op_resolve, given there are multiple resolves, and basically every other op can throw this is not anywhere near fixed :-(
(In reply to comment #4) > (In reply to comment #1) > > This has been fixed in r32756 except for the exception check on returns from > > native calls. > > That rev only fixed op_resolve, given there are multiple resolves, and > basically every other op can throw this is not anywhere near fixed :-( Oh yeah, I was only checking for VM_CHECK_EXCEPTION() in Machine::privateExecute(), but there are other exception checks in external functions.
It also effects most of the math operators, etc, etc The problem is a number of the places where we do bogus assignments are also in opcodes where we don't do sufficient/any exception checks. (all the numeric operations for instance.
Created attachment 20965 [details] Test case for various and exciting combinations of throwing and arithmetic Here is a test case that triggers (and tests many exciting issues
Created attachment 20966 [details] Better testcase Now in html, and with tests for execution of the right hand side of the expressions
Firefox fails in the *, -, /, and % cases
Created attachment 20977 [details] Test that can be run as JS or HTML
SadEagle's sequencing test: http://websvn.kde.org/*checkout*/trunk/tests/khtmltests/regression/tests/js/sequencing.js?revision=782984&content-type=text%2Fplain
Created attachment 21097 [details] Yet more insane testing
Fixed extraneous evaluation of RHS type conversions M JavaScriptCore/API/JSCallbackObjectFunctions.h M JavaScriptCore/ChangeLog M JavaScriptCore/kjs/object.cpp Committed r33372
Created attachment 21104 [details] More coverage
Created attachment 21105 [details] Even more coverage
More fixes (all toNumber, and numerous other places now have correct checks) M JavaScriptCore/ChangeLog M JavaScriptCore/VM/Machine.cpp M JavaScriptCore/VM/Opcode.h M JavaScriptCore/kjs/value.h M LayoutTests/ChangeLog A LayoutTests/fast/js/exception-sequencing-expected.txt A LayoutTests/fast/js/exception-sequencing.html Committed r33393
Committing to http://svn.webkit.org/repository/webkit/branches/squirrelfish ... M JavaScriptCore/ChangeLog M JavaScriptCore/VM/Machine.cpp M LayoutTests/ChangeLog A LayoutTests/fast/js/resources/tostring-exception-in-property-access.js A LayoutTests/fast/js/tostring-exception-in-property-access-expected.txt A LayoutTests/fast/js/tostring-exception-in-property-access.html Committed r33566