| Summary: | [WinCairo] Non-unified build fails to link Tools | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ross Kirsling <ross.kirsling> | ||||
| Component: | Tools / Tests | Assignee: | Ross Kirsling <ross.kirsling> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | commit-queue, don.olmstead, Hironori.Fujii, lforschler, stephan.szabo, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=200820 | ||||||
| Attachments: |
|
||||||
|
Description
Ross Kirsling
2019-04-12 10:24:24 PDT
(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 (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. (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. 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!
Created attachment 367756 [details]
Patch
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 (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 Comment on attachment 367756 [details] Patch Clearing flags on attachment: 367756 Committed r244448: <https://trac.webkit.org/changeset/244448> All reviewed patches have been landed. Closing bug. Oh. GTK port stopped using -fvisibility=hidden in Bug 181438. All symbols are exported. (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. |