Bug 196866 – [WinCairo] Non-unified build fails to link Tools

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Ross Kirsling]

As mentioned in bug 196845 comment 2, building WinCairo with clang-cl / lld and -DENABLE_UNIFIED_BUILDS=OFF, we fail to link DumpRenderTree and TestRunnerInjectedBundle.

> lld-link.exe: error: undefined symbol: __imp_?toJS@WebCore@@YA?AVJSValue@JSC@@PEAVExecState@3@PEAVJSDOMGlobalObject@1@AEAVTextTrackCueGeneric@1@@Z
> >>> referenced by C:\GitHub\neko\WebKitBuild\Release\DerivedSources\WebCore\JSInternals.cpp:6803
> >>>               WebCoreTestSupport.lib(JSInternals.cpp.obj):(?jsInternalsPrototypeFunctionCreateGenericCue@WebCore@@YA_JPEAVExecState@JSC@@@Z)
> 
> 
> lld-link.exe: error: undefined symbol: ?notifyNetworkStateChange@ServiceWorkerThreadProxy@WebCore@@QEAAX_N@Z
> >>> referenced by C:\GitHub\neko\WebKitBuild\Release\WTF\Headers\wtf\Function.h:102
> >>>               WebCoreTestSupport.lib(ServiceWorkerInternals.cpp.obj):(?call@?$CallableWrapper@V<lambda_0>@?0??setOnline@ServiceWorkerInternals@WebCore@@QEAAX_N@Z@@?$Function@$$A6AXXZ@WTF@@UEAAXXZ)
> 
> 
> lld-link.exe: error: undefined symbol: __imp_?parseHEVCCodecParameters@WebCore@@YA?AV?$Optional@UHEVCParameterSet@WebCore@@@WTF@@AEBVString@3@@Z
> >>> referenced by C:\GitHub\neko\Source\WebCore\testing\Internals.cpp:4962
> >>>               WebCoreTestSupport.lib(Internals.cpp.obj):(?parseHEVCCodecParameters@Internals@WebCore@@QEAA?AV?$Optional@UHEVCParameterSet@WebCore@@@WTF@@AEBVString@4@@Z)
> 
> 
> lld-link.exe: error: undefined symbol: __imp_?doDispatchMessageOnFrontendPage@InspectorClient@WebCore@@SAXPEAVPage@2@AEBVString@WTF@@@Z
> >>> referenced by C:\GitHub\neko\Source\WebCore\testing\Internals.cpp:372
> >>>               WebCoreTestSupport.lib(Internals.cpp.obj):(?sendMessageToFrontend@InspectorStubFrontend@WebCore@@EEAAXAEBVString@WTF@@@Z)

[Don Olmstead]

(In reply to Ross Kirsling from comment #0)
> As mentioned in bug 196845 comment 2, building WinCairo with clang-cl / lld
> and -DENABLE_UNIFIED_BUILDS=OFF, we fail to link DumpRenderTree and
> TestRunnerInjectedBundle.
> 
> > lld-link.exe: error: undefined symbol: __imp_?toJS@WebCore@@YA?AVJSValue@JSC@@PEAVExecState@3@PEAVJSDOMGlobalObject@1@AEAVTextTrackCueGeneric@1@@Z
> > >>> referenced by C:\GitHub\neko\WebKitBuild\Release\DerivedSources\WebCore\JSInternals.cpp:6803
> > >>>               WebCoreTestSupport.lib(JSInternals.cpp.obj):(?jsInternalsPrototypeFunctionCreateGenericCue@WebCore@@YA_JPEAVExecState@JSC@@@Z)
> > 
> > 
> > lld-link.exe: error: undefined symbol: ?notifyNetworkStateChange@ServiceWorkerThreadProxy@WebCore@@QEAAX_N@Z
> > >>> referenced by C:\GitHub\neko\WebKitBuild\Release\WTF\Headers\wtf\Function.h:102
> > >>>               WebCoreTestSupport.lib(ServiceWorkerInternals.cpp.obj):(?call@?$CallableWrapper@V<lambda_0>@?0??setOnline@ServiceWorkerInternals@WebCore@@QEAAX_N@Z@@?$Function@$$A6AXXZ@WTF@@UEAAXXZ)
> > 
> > 
> > lld-link.exe: error: undefined symbol: __imp_?parseHEVCCodecParameters@WebCore@@YA?AV?$Optional@UHEVCParameterSet@WebCore@@@WTF@@AEBVString@3@@Z
> > >>> referenced by C:\GitHub\neko\Source\WebCore\testing\Internals.cpp:4962
> > >>>               WebCoreTestSupport.lib(Internals.cpp.obj):(?parseHEVCCodecParameters@Internals@WebCore@@QEAA?AV?$Optional@UHEVCParameterSet@WebCore@@@WTF@@AEBVString@4@@Z)
> > 
> > 
> > lld-link.exe: error: undefined symbol: __imp_?doDispatchMessageOnFrontendPage@InspectorClient@WebCore@@SAXPEAVPage@2@AEBVString@WTF@@@Z
> > >>> referenced by C:\GitHub\neko\Source\WebCore\testing\Internals.cpp:372
> > >>>               WebCoreTestSupport.lib(Internals.cpp.obj):(?sendMessageToFrontend@InspectorStubFrontend@WebCore@@EEAAXAEBVString@WTF@@@Z)

This is clang-cl specific right? If so update the description

[Ross Kirsling]

(In reply to Don Olmstead from comment #1)
> This is clang-cl specific right? If so update the description

Not sure, I can try combining regular cl with lld.

The MSVC linker is unable to handle the size of WebCore.lib as mentioned in bug 196762 comment 2.

[Ross Kirsling]

(In reply to Ross Kirsling from comment #2)
> (In reply to Don Olmstead from comment #1)
> > This is clang-cl specific right? If so update the description
> 
> Not sure, I can try combining regular cl with lld.
> 
> The MSVC linker is unable to handle the size of WebCore.lib as mentioned in
> bug 196762 comment 2.

Confirmed that this does not require clang-cl.

[Ross Kirsling]

Evidently the issue here is that certain object files are unused when linking WK[L] and thus their symbols don't get exported.

One fix for this is to add the following line to WebKit[Legacy]/PlatformWin.cmake:
> set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} /WHOLEARCHIVE:WebCore.lib")

But Fujii-san pointed out that we can alternatively tell Cmake to build WebCore as an "object library", and it turns out this makes the non-unified build work with not only lld but also with the MSVC linker!

[Ross Kirsling]

Created attachment 367756 [details]
Patch

[Fujii Hironori]

Comment on attachment 367756 [details]
Patch

I prefer the Object library approach to /WHOLEARCHIVE and --whole-archive switches.
Looks good to me.

I have some questions.

Do you know the reason why GTK port doesn't have the same issue?

According to the document, use need to use $<TARGET_OBJECTS:WebCore> in order to use Object Library. Do you know the reason why you don't need it actually?
https://cmake.org/cmake/help/latest/command/add_library.html#object-libraries

[Ross Kirsling]

(In reply to Fujii Hironori from comment #6)
> Comment on attachment 367756 [details]
> Patch
> 
> I prefer the Object library approach to /WHOLEARCHIVE and --whole-archive
> switches.
> Looks good to me.
> 
> I have some questions.
> 
> Do you know the reason why GTK port doesn't have the same issue?

I'm not 100% certain. Steph suggested it could be a behavioral difference between declspec on Windows and visibility on Unix, but then --whole-archive is originally a GCC option...

In fact, a whole-archived WebCore gets added to WebKitPluginProcess2, though I don't see how this would help TestRunnerInjectedBundle:
https://github.com/WebKit/webkit/blob/master/Source/WebKit/PlatformGTK.cmake#L682

It's interesting that GTK is the only platform on which WebCore is added to WebKitTestRunner_LIBRARIES, but it's not added to WebKitTestRunnerInjectedBundle_LIBRARIES, so that might not matter:
https://github.com/WebKit/webkit/blob/master/Tools/WebKitTestRunner/PlatformGTK.cmake#L27-L35

I now wonder whether WPE has any issues here separate from GTK...

> According to the document, use need to use $<TARGET_OBJECTS:WebCore> in
> order to use Object Library. Do you know the reason why you don't need it
> actually?
> https://cmake.org/cmake/help/latest/command/add_library.html#object-libraries

I believe that syntax is only required when interpolating with the explicit list of items? Notice the second example here:
https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#object-libraries

[WebKit Commit Bot]

Comment on attachment 367756 [details]
Patch

Clearing flags on attachment: 367756

Committed r244448: <https://trac.webkit.org/changeset/244448>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/50038638>

[Fujii Hironori]

Oh. GTK port stopped using -fvisibility=hidden in Bug 181438. All symbols are exported.

[Fujii Hironori]

(In reply to Fujii Hironori from comment #11)
> Oh. GTK port stopped using -fvisibility=hidden in Bug 181438. All symbols
> are exported.

Err. Irrelevant. Ignore my comment.