Bug 196866

Summary: [WinCairo] Non-unified build fails to link Tools
Product: WebKit Reporter: Ross Kirsling <ross.kirsling>
Component: Tools / TestsAssignee: Ross Kirsling <ross.kirsling>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, don.olmstead, fujii.hironori, 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 Flags
Patch none

Ross Kirsling
Reported 2019-04-12 10:24:24 PDT
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)
Attachments
Patch (2.90 KB, patch)
2019-04-18 14:24 PDT, Ross Kirsling
no flags
Don Olmstead
Comment 1 2019-04-12 10:27:42 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
Ross Kirsling
Comment 2 2019-04-12 10:37:08 PDT
(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
Comment 3 2019-04-12 11:25:33 PDT
(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
Comment 4 2019-04-18 13:57:16 PDT
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
Comment 5 2019-04-18 14:24:51 PDT
Fujii Hironori
Comment 6 2019-04-18 19:01:10 PDT
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
Comment 7 2019-04-18 20:01:25 PDT
(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 8 2019-04-18 20:28:40 PDT
Comment on attachment 367756 [details] Patch Clearing flags on attachment: 367756 Committed r244448: <https://trac.webkit.org/changeset/244448>
WebKit Commit Bot
Comment 9 2019-04-18 20:28:42 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 10 2019-04-18 20:29:17 PDT
Fujii Hironori
Comment 11 2019-04-18 20:57:46 PDT
Oh. GTK port stopped using -fvisibility=hidden in Bug 181438. All symbols are exported.
Fujii Hironori
Comment 12 2019-04-18 21:16:07 PDT
(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.
Note You need to log in before you can comment on or make changes to this bug.