[Win] error LNK2005: WebCore::JSNode::checkSubClassPatchpoint() already defined in WebKit.lib

[Fujii Hironori]

AppleWin port and WinCairo port builds are broken.

> FAILED: bin64/TestWebKitLib.dll lib64/TestWebKitLib.lib 
> cmd.exe /C "cd . && C:\tools\cmake\bin\cmake.exe -E vs_link_dll --intdir=Tools\TestWebKitAPI\CMakeFiles\TestWebKitLib.dir --manifests  -- "C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\VC\Tools\MSVC\14.10.25017\bin\HostX64\x64\link.exe" /nologo @CMakeFiles/TestWebKitLib.rsp  /out:bin64\TestWebKitLib.dll /implib:lib64\TestWebKitLib.lib /pdb:bin64\TestWebKitLib.pdb /dll /version:0.0 /machine:x64 /DEBUG /OPT:ICF /OPT:REF /INCREMENTAL:NO   && cd ."
> WebCore.lib(JSNodeDOMJIT.cpp.obj) : error LNK2005: "public: static class WTF::RefPtr<class JSC::DOMJIT::Patchpoint> __cdecl WebCore::JSNode::checkSubClassPatchpoint(void)" (?checkSubClassPatchpoint@JSNode@WebCore@@SA?AV?$RefPtr@VPatchpoint@DOMJIT@JSC@@@WTF@@XZ) already defined in WebKit.lib(WebKit.dll)
> 
> WebCore.lib(JSDocumentDOMJIT.cpp.obj) : error LNK2005: "public: static class WTF::RefPtr<class JSC::DOMJIT::Patchpoint> __cdecl WebCore::JSDocument::checkSubClassPatchpoint(void)" (?checkSubClassPatchpoint@JSDocument@WebCore@@SA?AV?$RefPtr@VPatchpoint@DOMJIT@JSC@@@WTF@@XZ) already defined in WebKit.lib(WebKit.dll)
> 
>    Creating library lib64\TestWebKitLib.lib and object lib64\TestWebKitLib.exp
> 
> bin64\TestWebKitLib.dll : fatal error LNK1169: one or more multiply defined symbols found
> 
> LINK failed. with 1169
> [60/60] Linking CXX shared library bin64\TestWebCoreLib.dll
> FAILED: bin64/TestWebCoreLib.dll lib64/TestWebCoreLib.lib 
> cmd.exe /C "cd . && C:\tools\cmake\bin\cmake.exe -E vs_link_dll --intdir=Tools\TestWebKitAPI\CMakeFiles\TestWebCoreLib.dir --manifests  -- "C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\VC\Tools\MSVC\14.10.25017\bin\HostX64\x64\link.exe" /nologo @CMakeFiles/TestWebCoreLib.rsp  /out:bin64\TestWebCoreLib.dll /implib:lib64\TestWebCoreLib.lib /pdb:bin64\TestWebCoreLib.pdb /dll /version:0.0 /machine:x64 /DEBUG /OPT:ICF /OPT:REF /INCREMENTAL:NO   && cd ."
> WebCore.lib(JSNodeDOMJIT.cpp.obj) : error LNK2005: "public: static class WTF::RefPtr<class JSC::DOMJIT::Patchpoint> __cdecl WebCore::JSNode::checkSubClassPatchpoint(void)" (?checkSubClassPatchpoint@JSNode@WebCore@@SA?AV?$RefPtr@VPatchpoint@DOMJIT@JSC@@@WTF@@XZ) already defined in WebKit.lib(WebKit.dll)
> 
> WebCore.lib(JSDocumentDOMJIT.cpp.obj) : error LNK2005: "public: static class WTF::RefPtr<class JSC::DOMJIT::Patchpoint> __cdecl WebCore::JSDocument::checkSubClassPatchpoint(void)" (?checkSubClassPatchpoint@JSDocument@WebCore@@SA?AV?$RefPtr@VPatchpoint@DOMJIT@JSC@@@WTF@@XZ) already defined in WebKit.lib(WebKit.dll)
> 
>    Creating library lib64\TestWebCoreLib.lib and object lib64\TestWebCoreLib.exp

[Fujii Hironori]

WEBCORE_EXPORT is defined for dllexport in WebCore. This is strange becuase WebCore is a static library in Windows port.

[Fujii Hironori]

This has happened since https://trac.webkit.org/changeset/217031

[Yusuke Suzuki]

Hmmmm, it is very storange. These functions are only defined once in JSNodeDOMJIT.cpp & JSDocumentDOMJIT.cpp. And other ports just build it correctly.
Do you know the special things done for WebCore.lib & WebKit.lib in Windows?

[Yusuke Suzuki]

(In reply to Yusuke Suzuki from comment #3)
> Hmmmm, it is very storange. These functions are only defined once in
> JSNodeDOMJIT.cpp & JSDocumentDOMJIT.cpp. And other ports just build it
> correctly.
> Do you know the special things done for WebCore.lib & WebKit.lib in Windows?

And I also note that JSEventDOMJIT.cpp, JSElementDOMJIT.cpp and JSDocumentFragmentDOMJIT.cpp also have similar `checkSubClassPatchpoint` functions. But they do not cause linker errors according to the log.

[Fujii Hironori]

By checking with dumpbin, I've confirmed checkSubClassPatchpoint was exported from WebKit.dll.
I'm guessing a following simple scenario.

1) JSDocumentDOMJIT.cpp was compiled with dllexport in WebCore
2) The linker included JSDocumentDOMJIT.cpp.obj into WebKit.dll
3) The linker found some dllexport-marked symbols in JSDocumentDOMJIT.cpp.obj and exported them from WebKit.dll

I admit that this hypothesis doesn't explain comment 4.

[Per Arne Vollan]

Perhaps we shouldn't link with both WebKit.lib and WebCore.lib? In theory, I guess TestWebCoreLib only needs to link with WebCore.lib, and TestWebKitLib only needs to link with WebKit.lib. I would expect to get many other linker errors if this was the case, though.

[Yusuke Suzuki]

(In reply to Fujii Hironori from comment #5)
> By checking with dumpbin, I've confirmed checkSubClassPatchpoint was
> exported from WebKit.dll.
> I'm guessing a following simple scenario.
> 
> 1) JSDocumentDOMJIT.cpp was compiled with dllexport in WebCore
> 2) The linker included JSDocumentDOMJIT.cpp.obj into WebKit.dll
> 3) The linker found some dllexport-marked symbols in
> JSDocumentDOMJIT.cpp.obj and exported them from WebKit.dll
> 
> I admit that this hypothesis doesn't explain comment 4.

Right. I've just checked the generated JSNode.cpp in DerivedSoure.
And I've found that, whole JSNode class is annotated by WEBCORE_EXPORT,
while JSDocumentFragment is not.

[Fujii Hironori]

Created attachment 310635 [details]
WIP patch

(In reply to Per Arne Vollan from comment #6)
> Perhaps we shouldn't link with both WebKit.lib and WebCore.lib? In theory, I
> guess TestWebCoreLib only needs to link with WebCore.lib, and TestWebKitLib
> only needs to link with WebKit.lib. I would expect to get many other linker
> errors if this was the case, though.

Sounds good!
Unfortunately, my WIP patch reports a following linkage error.

> ------ Build started: Project: TestWebCoreLib, Configuration: Debug x64 ------
>      Creating library C:/webkit/ga/WebKitBuild/Debug/lib64/TestWebCoreLib.lib and object C:/webkit/ga/WebKitBuild/Debug/lib64/TestWebCoreLib.exp
> WebCore.lib(ImageWin.obj) : error LNK2019: unresolved external symbol "class WTF::RefPtr<class WebCore::SharedBuffer> __cdecl loadResourceIntoBuffer(char const *)" (?loadResourceIntoBuffer@@YA?AV?$RefPtr@VSharedBuffer@WebCore@@@WTF@@PEBD@Z) referenced in function "public: static class WTF::Ref<class WebCore::Image> __cdecl WebCore::Image::loadPlatformResource(char const *)" (?loadPlatformResource@Image@WebCore@@SA?AV?$Ref@VImage@WebCore@@@WTF@@PEBD@Z)
> C:\webkit\ga\WebKitBuild\Debug\bin64\TestWebCoreLib.dll : fatal error LNK1120: 1 unresolved externals
> ========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

There is a layer violation.

[Per Arne Vollan]

(In reply to Fujii Hironori from comment #8)
> Created attachment 310635 [details]
> WIP patch
> 
> (In reply to Per Arne Vollan from comment #6)
> > Perhaps we shouldn't link with both WebKit.lib and WebCore.lib? In theory, I
> > guess TestWebCoreLib only needs to link with WebCore.lib, and TestWebKitLib
> > only needs to link with WebKit.lib. I would expect to get many other linker
> > errors if this was the case, though.
> 
> Sounds good!
> Unfortunately, my WIP patch reports a following linkage error.
> 
> > ------ Build started: Project: TestWebCoreLib, Configuration: Debug x64 ------
> >      Creating library C:/webkit/ga/WebKitBuild/Debug/lib64/TestWebCoreLib.lib and object C:/webkit/ga/WebKitBuild/Debug/lib64/TestWebCoreLib.exp
> > WebCore.lib(ImageWin.obj) : error LNK2019: unresolved external symbol "class WTF::RefPtr<class WebCore::SharedBuffer> __cdecl loadResourceIntoBuffer(char const *)" (?loadResourceIntoBuffer@@YA?AV?$RefPtr@VSharedBuffer@WebCore@@@WTF@@PEBD@Z) referenced in function "public: static class WTF::Ref<class WebCore::Image> __cdecl WebCore::Image::loadPlatformResource(char const *)" (?loadPlatformResource@Image@WebCore@@SA?AV?$Ref@VImage@WebCore@@@WTF@@PEBD@Z)
> > C:\webkit\ga\WebKitBuild\Debug\bin64\TestWebCoreLib.dll : fatal error LNK1120: 1 unresolved externals
> > ========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========
> 
> There is a layer violation.

Nice work! I suggest soft linking the loadResourceIntoBuffer function.

[Yusuke Suzuki]

(In reply to Per Arne Vollan from comment #9)
> (In reply to Fujii Hironori from comment #8)
> > Created attachment 310635 [details]
> > WIP patch
> > 
> > (In reply to Per Arne Vollan from comment #6)
> > > Perhaps we shouldn't link with both WebKit.lib and WebCore.lib? In theory, I
> > > guess TestWebCoreLib only needs to link with WebCore.lib, and TestWebKitLib
> > > only needs to link with WebKit.lib. I would expect to get many other linker
> > > errors if this was the case, though.
> > 
> > Sounds good!
> > Unfortunately, my WIP patch reports a following linkage error.
> > 
> > > ------ Build started: Project: TestWebCoreLib, Configuration: Debug x64 ------
> > >      Creating library C:/webkit/ga/WebKitBuild/Debug/lib64/TestWebCoreLib.lib and object C:/webkit/ga/WebKitBuild/Debug/lib64/TestWebCoreLib.exp
> > > WebCore.lib(ImageWin.obj) : error LNK2019: unresolved external symbol "class WTF::RefPtr<class WebCore::SharedBuffer> __cdecl loadResourceIntoBuffer(char const *)" (?loadResourceIntoBuffer@@YA?AV?$RefPtr@VSharedBuffer@WebCore@@@WTF@@PEBD@Z) referenced in function "public: static class WTF::Ref<class WebCore::Image> __cdecl WebCore::Image::loadPlatformResource(char const *)" (?loadPlatformResource@Image@WebCore@@SA?AV?$Ref@VImage@WebCore@@@WTF@@PEBD@Z)
> > > C:\webkit\ga\WebKitBuild\Debug\bin64\TestWebCoreLib.dll : fatal error LNK1120: 1 unresolved externals
> > > ========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========
> > 
> > There is a layer violation.
> 
> Nice work! I suggest soft linking the loadResourceIntoBuffer function.

Yeah, that's great!
In the meantime, I'll land the original patch by extracting checkSubClassPatchpoint from JSNode / JSDocument and move it to WebCore::checkSubClassPatchpointFor{JSNode,JSDocument}.

[Fujii Hironori]

Created attachment 310639 [details]
Patch

This is a layer violation and TestWebCore is just a unit test. I'd like to suggest to add a stub in TestWebCoreLib. How do you think this?

[Per Arne Vollan]

(In reply to Fujii Hironori from comment #11)
> Created attachment 310639 [details]
> Patch
> 
> This is a layer violation and TestWebCore is just a unit test. I'd like to
> suggest to add a stub in TestWebCoreLib. How do you think this?

That sounds good! It seems the build problem on Win EWS is unrelated to this patch.

[Fujii Hironori]

Win EWS gets green. Could anyone review this?

[Per Arne Vollan]

Comment on attachment 310639 [details]
Patch

Looks good! R=me.

[WebKit Commit Bot]

Comment on attachment 310639 [details]
Patch

Clearing flags on attachment: 310639

Committed r217184: <http://trac.webkit.org/changeset/217184>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.