[GTK][Stable] Missing implementation of callToJavaScript/callToNativeFunction with msys/mingw32

• Component: JavaScriptCore
• Status: RESOLVED WONTFIX

[Milan Crha]

I'm trying to build webkit under windows using mingw32 and msys, and I configure webkit 2.4.1 as this:
   ./configure --prefix=$PREFIX --enable-win32-target --enable-spellcheck --enable-jit --disable-geolocation --disable-video
     --disable-web-audio --disable-webgl --disable-accelerated-compositing --disable-glx --disable-egl --disable-gles2 --disable-webkit2
     --disable-gtk-doc --disable-gtk-doc-html --disable-gtk-doc-pdf

When compiling JIT, I get an error about missing callToJavaScript/callToNativeFunction/returnFromJavaScript when linking. I found special implementation of those functions in Source/JavaScriptCore/jit/JITStubsX86.h, but they are used only for MSVC compiler, while I use gcc.

What would be a correct way of providing implementation of those functions in the COMPILER(GCC) part of the JITStubsX86.h with added block:
   #if OS(WINDOWS) && ENABLE(JIT)
   #endif //OS(WINDOWS) && ENABLE(JIT)
under that GCC conditional compile? I'm not good in assembler, which is an obstacle.

[Milan Crha]

Created attachment 231674 [details]
Unfinished patch

Alternatively, the --disable-jit can be used, though the code requires fixing too. This patch shows two changes I made to move forward in compilation (but not in runtime). The TODO in the patch needs finishing.

[Mark Lam]

Comment on attachment 231674 [details]
Unfinished patch

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

> webkitgtk-2.4.1/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:494
> +#if ENABLE(COMPUTED_GOTO_OPCODES)
>          goto llint_generic_return_point;
> +#else
> +	// TODO: do something sane here
> +#endif

Why does this need to be conditional on ENABLE(COMPUTED_GOTO_OPCODES)?

[Milan Crha]

(In reply to comment #2)
> (From update of attachment 231674 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=231674&action=review
> 
> > webkitgtk-2.4.1/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:494
> > +#if ENABLE(COMPUTED_GOTO_OPCODES)
> >          goto llint_generic_return_point;
> > +#else
> > +	// TODO: do something sane here
> > +#endif
> 
> Why does this need to be conditional on ENABLE(COMPUTED_GOTO_OPCODES)?

I thought that there is a reason why the label is used and it is defined only when the ENABLE(COMPUTED_GOTO_OPCODES) is defined as well. If there can be used a code in "TODO" always, then even better.

[Milan Crha]

Created attachment 233444 [details]
asm implementation of the functions

This makes jsc-3.exe work (2+3 is 5) with msys/mingw32.

[WebKit Commit Bot]

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


Total errors found: 0 in 0 files


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

[Milan Crha]

I forgot to add, the second patch is for --enable-jit configure option.

[Alexey Pavlov]

Will be available patch also for 64-bit mingw?

[Carlos Garcia Campos]

Comment on attachment 233444 [details]
asm implementation of the functions

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

Could you provide a ChangeLog entry? r- because of the missing ChangeLog.

> webkitgtk-2.4.1/Source/JavaScriptCore/jit/JITStubsX86.h:202
> +#if OS(WINDOWS) && ENABLE(JIT)

I don't think you need the ENABLE(JIT), this files is only included from JITStubs.cpp inside a #if ENABLE(JIT) block

[Carlos Garcia Campos]

*** Bug 143757 has been marked as a duplicate of this bug. ***

[Milan Crha]

Created attachment 251065 [details]
wk patch ][

I built webkit-2.4 branch a revision 182543 with this patch.

[WebKit Commit Bot]

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


ERROR: Source/JavaScriptCore/jit/JITStubsX86.h:416:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 2 files


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

[LRN]

According to alexey, the attachment 231674 [details] is still needed for 64-bit builds (which have to disable JIT, and thus must use llint; attachment 231674 [details] "fixes" llint, apparently).

[Michael Catanzaro]

WONTFIXing this because I don't believe we have any port that builds with MinGW anymore.

[Michael Catanzaro]

By the way, I don't mean to suggest that we don't ever want to support GTK on Windows ever again. Just that currently, we don't, and if nothing else is using MinGW, then this patch on its own is not useful.

[Carlos Garcia Campos]

We support windows in the 2.4 branch, the "stable" in the bug title here actually means 2.4 branch.

[Michael Catanzaro]

Surely you do not plan to support the 2.4 branch anymore?

[Carlos Garcia Campos]

(In reply to comment #16)
> Surely you do not plan to support the 2.4 branch anymore?

Yes, we keep 2.4 until wk2 has windows support. It's actually open for people still using wk1 because of windows support. But it's true I don't work on it unless someone ask me to merge a commit or whatever.