Bug 25809 - decorations __declspec(dlimport) and __declspec(dlexport) break linking on win32 with mingw
Summary: decorations __declspec(dlimport) and __declspec(dlexport) break linking on wi...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-14 14:54 PDT by Fridrich Strba
Modified: 2009-05-27 05:05 PDT (History)
0 users

See Also:


Attachments
don't use __declspec(dlimport) and __declspec(dlexport) decorations when building with mingw (1.12 KB, patch)
2009-05-14 14:55 PDT, Fridrich Strba
no flags Details | Formatted Diff | Diff
use !COMPILER(GCC) instead of !defined(__MINGW32__) (1.10 KB, patch)
2009-05-14 15:04 PDT, Fridrich Strba
no flags Details | Formatted Diff | Diff
A proper standard-compliant patch with ChangeLog entries (3.61 KB, patch)
2009-05-15 02:04 PDT, Fridrich Strba
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fridrich Strba 2009-05-14 14:54:03 PDT
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
Comment 1 Fridrich Strba 2009-05-14 14:55:05 PDT
Created attachment 30359 [details]
don't use __declspec(dlimport) and __declspec(dlexport) decorations when building with mingw
Comment 2 Fridrich Strba 2009-05-14 15:02:09 PDT
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
Comment 3 Fridrich Strba 2009-05-14 15:02:58 PDT
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
Comment 4 Fridrich Strba 2009-05-14 15:04:34 PDT
Created attachment 30360 [details]
use !COMPILER(GCC) instead of !defined(__MINGW32__)
Comment 5 Fridrich Strba 2009-05-15 02:04:12 PDT
Created attachment 30379 [details]
A proper standard-compliant patch with ChangeLog entries
Comment 6 Jan Alonzo 2009-05-17 01:37:37 PDT
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.
Comment 7 Fridrich Strba 2009-05-18 00:42:32 PDT
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.
Comment 8 Maciej Stachowiak 2009-05-22 00:39:53 PDT
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.)
Comment 9 Fridrich Strba 2009-05-22 00:51:06 PDT
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.
Comment 10 Maciej Stachowiak 2009-05-22 00:59:00 PDT
Comment on attachment 30379 [details]
A proper standard-compliant patch with ChangeLog entries

r=me
Comment 11 Gustavo Noronha (kov) 2009-05-27 05:05:55 PDT
Landed as r44184.