Bug 124251 – Change callToJavaScript thunk into an offline assembled stub

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Michael Saboff]

After r158751: <http://trac.webkit.org/changeset/158751>, JavaScript execution requires the ability to JIT a thunk to enter JavaScript.  The thunks callToJavaScript and throwNotCaught should be changed into offline assembled stubs.

[Michael Saboff]

Created attachment 216754 [details]
Patch

The changes for ARM64, MIPS and SH4 have not been compiled or tested.

[EFL EWS Bot]

Comment on attachment 216754 [details]
Patch

Attachment 216754 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/22868904

[Michael Saboff]

Created attachment 216760 [details]
Updated patch

Speculative fix for EFL build failure.

[Build Bot]

Comment on attachment 216760 [details]
Updated patch

Attachment 216760 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/22888894

[Michael Saboff]

Created attachment 216815 [details]
Updated with speculative windows build fix

[Build Bot]

Comment on attachment 216815 [details]
Updated with speculative windows build fix

Attachment 216815 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/22779954

[Michael Saboff]

Created attachment 216836 [details]
Patch with fix for windows

Since the LLInt doesn't work on Windows, I reverted the Windows stubs back to inline assembly in JITStubsX86.h and JITStubsMSVC64.asm.

[WebKit Commit Bot]

Attachment 216836 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/dfg/DFGDriver.cpp', u'Source/JavaScriptCore/jit/GPRInfo.h', u'Source/JavaScriptCore/jit/JITCode.cpp', u'Source/JavaScriptCore/jit/JITExceptions.cpp', u'Source/JavaScriptCore/jit/JITStubs.h', u'Source/JavaScriptCore/jit/JITStubsMSVC64.asm', u'Source/JavaScriptCore/jit/JITStubsX86.h', u'Source/JavaScriptCore/jit/ThunkGenerators.cpp', u'Source/JavaScriptCore/jit/ThunkGenerators.h', u'Source/JavaScriptCore/llint/LLIntThunks.h', u'Source/JavaScriptCore/llint/LowLevelInterpreter.asm', u'Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm', u'Source/JavaScriptCore/llint/LowLevelInterpreter64.asm', u'Source/JavaScriptCore/offlineasm/arm.rb', u'Source/JavaScriptCore/offlineasm/arm64.rb', u'Source/JavaScriptCore/offlineasm/instructions.rb', u'Source/JavaScriptCore/offlineasm/mips.rb', u'Source/JavaScriptCore/offlineasm/registers.rb', u'Source/JavaScriptCore/offlineasm/sh4.rb', u'Source/JavaScriptCore/offlineasm/x86.rb', u'Source/JavaScriptCore/runtime/VM.cpp', u'Source/JavaScriptCore/runtime/VM.h']" exit_code: 1
Source/JavaScriptCore/jit/JITStubsX86.h:222:  Extra space before [  [whitespace/braces] [5]
Source/JavaScriptCore/jit/JITStubsX86.h:223:  Extra space before [  [whitespace/braces] [5]
Total errors found: 2 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Geoffrey Garen]

Comment on attachment 216836 [details]
Patch with fix for windows

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

r=me

Please test on 32bit, 64bit, and Windows before landing.

> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:424
> +macro makeCallToJavaScript(entry, newCallFrame, previousCFR)

50% of this function is #ifdef'd, and it is confusingly named synonymously with doCallToJavaScript. So, I think you should just move the relevant code into the right places by hand, and remove this helper function.

[EFL EWS Bot]

Comment on attachment 216836 [details]
Patch with fix for windows

Attachment 216836 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/22600136

[Michael Saboff]

Created attachment 216903 [details]
Patch for Landing

Made the changes suggested.  Made minor fixes to the X86 windows stubs.

Built and ran JavaScript tests on Mac 32 & 64 and Windows 32.

[Michael Saboff]

Committed r159276: <http://trac.webkit.org/changeset/159276>