Bug 25809 – decorations __declspec(dlimport) and __declspec(dlexport) break linking on win32 with mingw

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Fridrich Strba]

Since we are building webkit by building several static libraries that we link into one shared library, the decorations break the linking on win32 with mingw. The win32 linker cannot link the symbols into a static library as __imp___whatever, so, it links them as _whatever only. But at the time when we are creating the libwebkit shared library, the linker looks for the __imp___whatever symbols because the decorations force him to do so.
Now, the win32 linker that comes with GNU binutils has a nice feature that is called autoimport. It is activated by default and does the RightThing(tm) without the decorations. When building with gcc on windows, it is a safe practice to rely on this nifty feature, because getting the __declspec(dl*) decorations right is just a big pain and sometime not even humanly possible.
Following patch solves that for win32 builds with mingw32

[Fridrich Strba]

Created attachment 30359 [details]
don't use __declspec(dlimport) and __declspec(dlexport) decorations when building with mingw

[Fridrich Strba]

Comment on attachment 30359 [details]
don't use __declspec(dlimport) and __declspec(dlexport) decorations when building with mingw

> --- webkit-1.1.7/JavaScriptCore/config.h	(revision 43188)
> +++ webkit-1.1.7/JavaScriptCore/config.h	(working copy)
> @@ -25,7 +25,7 @@
>  
>  #include <wtf/Platform.h>
>  
> -#if PLATFORM(WIN_OS) && !defined(BUILDING_WX__)
> +#if PLATFORM(WIN_OS) && !defined(BUILDING_WX__) && !COMPILER(GCC)
>  #if defined(BUILDING_JavaScriptCore) || defined(BUILDING_WTF)
>  #define JS_EXPORTDATA __declspec(dllexport)
>  #else
> --- webkit-1.1.7/WebCore/config.h	(revision 43188)
> +++ webkit-1.1.7/WebCore/config.h	(working copy)
> @@ -24,7 +24,7 @@
>  
>  #include <wtf/Platform.h>
>  
> -#if PLATFORM(WIN_OS) && !defined(BUILDING_WX__)
> +#if PLATFORM(WIN_OS) && !defined(BUILDING_WX__) && !COMPILER(GCC)
>  #if defined(BUILDING_JavaScriptCore) || defined(BUILDING_WTF)
>  #define JS_EXPORTDATA __declspec(dllexport)
>  #else
> --- webkit-1.1.7/WebKitTools/DumpRenderTree/config.h	(revision 43188)
> +++ webkit-1.1.7/WebKitTools/DumpRenderTree/config.h	(working copy)
> @@ -26,7 +26,7 @@
>  
>  #include <wtf/Platform.h>
>  
> -#if PLATFORM(WIN_OS)
> +#if PLATFORM(WIN_OS) && !defined(__MINGW32__)
>  #define JS_EXPORTDATA __declspec(dllimport)
>  #define WEBKIT_EXPORTDATA __declspec(dllimport)
>  #else

[Fridrich Strba]

Comment on attachment 30359 [details]
don't use __declspec(dlimport) and __declspec(dlexport) decorations when building with mingw

>--- webkit-1.1.7/JavaScriptCore/config.h	(revision 43188)
>+++ webkit-1.1.7/JavaScriptCore/config.h	(working copy)
>@@ -25,7 +25,7 @@
> 
> #include <wtf/Platform.h>
> 
>-#if PLATFORM(WIN_OS) && !defined(BUILDING_WX__)
>+#if PLATFORM(WIN_OS) && !defined(BUILDING_WX__) && !COMPILER(GCC)
> #if defined(BUILDING_JavaScriptCore) || defined(BUILDING_WTF)
> #define JS_EXPORTDATA __declspec(dllexport)
> #else
>--- webkit-1.1.7/WebCore/config.h	(revision 43188)
>+++ webkit-1.1.7/WebCore/config.h	(working copy)
>@@ -24,7 +24,7 @@
> 
> #include <wtf/Platform.h>
> 
>-#if PLATFORM(WIN_OS) && !defined(BUILDING_WX__)
>+#if PLATFORM(WIN_OS) && !defined(BUILDING_WX__) && !COMPILER(GCC)
> #if defined(BUILDING_JavaScriptCore) || defined(BUILDING_WTF)
> #define JS_EXPORTDATA __declspec(dllexport)
> #else
>--- webkit-1.1.7/WebKitTools/DumpRenderTree/config.h	(revision 43188)
>+++ webkit-1.1.7/WebKitTools/DumpRenderTree/config.h	(working copy)
>@@ -26,7 +26,7 @@
> 
> #include <wtf/Platform.h>
> 
>-#if PLATFORM(WIN_OS)
>+#if PLATFORM(WIN_OS) && !defined(__MINGW32__)
> #define JS_EXPORTDATA __declspec(dllimport)
> #define WEBKIT_EXPORTDATA __declspec(dllimport)
> #else

[Fridrich Strba]

Created attachment 30360 [details]
use !COMPILER(GCC) instead of !defined(__MINGW32__)

[Fridrich Strba]

Created attachment 30379 [details]
A proper standard-compliant patch with ChangeLog entries

[Jan Alonzo]

Comment on attachment 30379 [details]
A proper standard-compliant patch with ChangeLog entries

> Index: JavaScriptCore/config.h
> ===================================================================
> --- JavaScriptCore/config.h	(revision 43762)
> +++ JavaScriptCore/config.h	(working copy)
> @@ -25,7 +25,7 @@
>  
>  #include <wtf/Platform.h>
>  
> -#if PLATFORM(WIN_OS) && !defined(BUILDING_WX__)
> +#if PLATFORM(WIN_OS) && !defined(BUILDING_WX__) && !COMPILER(GCC)
>  #if defined(BUILDING_JavaScriptCore) || defined(BUILDING_WTF)

Is there a reason why you're not using COMPILER(MINGW32) here?

Since this is specific to mingw, I think we should use COMPILER(MINGW32) here.

[Fridrich Strba]

The COMPILER(GCC) encompassed also the Cygwin gcc compiler that is likely to hit this issue too because of the way we link the shared webkit library.

[Maciej Stachowiak]

Comment on attachment 30379 [details]
A proper standard-compliant patch with ChangeLog entries

Are the decorations MSVC-specific, or do they fail only in GCC? Would it be better to use COMPILER(MSVC) instead of !COMPILER(GCC)? (Honestly not sure; r- for reply to question but feel free to just reflag this patch if the way it is now is right.)

[Fridrich Strba]

The decorations don't work well when the compiler is GCC. That is why !COMPILER(GCC)
They work well with MSVC, but they might also work well with other compilers for Windows, I don't have them and that is why I did this solution, because it is covering MinGW and CygWin where this problem arises.

[Maciej Stachowiak]

Comment on attachment 30379 [details]
A proper standard-compliant patch with ChangeLog entries

r=me

[Gustavo Noronha (kov)]

Landed as r44184.