Eliminating duplicate symbols on Windows clang component debug builds

[Mark Dittmer]

I've been trying to land this issue for quite some time now. I've managed to make most build configurations happy. So far as I can tell, the only unhappy ones left are Windows clang component debug builds.

Here's an excerpt from some build failures:

FAILED: E:/b/depot_tools/python276_bin/python.exe gyp-win-tool link-wrapper environment.x64 False link.exe /nologo /OUT:gpu_ipc_service_unittests.exe /PDB:gpu_ipc_service_unittests.exe.pdb @gpu_ipc_service_unittests.exe.rsp

gpu.dll.lib(gpu.dll) : error LNK2005: "public: union gpu::ValueState const * __cdecl gpu::ValueStateMap::GetState(unsigned int)const " (?GetState@ValueStateMap@gpu@@QEBAPEBTValueState@2@I@Z) already defined in value_state.obj

What I think is going on: When attempting to build gpu_ipc_service_unittests, Windows is linking against both value_state.obj and gpu.dll.lib, which have conflicting symbols. (This is evidenced by locally inspecting gpu_ipc_service_unittests.exe.rsp and output from "dumpbin /exports out/Debug/gpu.dll.lib"). I suspect that two or more dependencies of gpu_ipc_service_unittests (including itself) depend publicly on gpu/command_buffer/common:common_sources (which contains value_state.obj's source).

Outstanding issues/what I've tried: But, so far as I can tell, I've eliminated any such cases. Only gpu depends publicly on this. I also thought that, perhaps, the symbols being provided by gpu collide with private symbols in gpu_ipc_service_unittests. So, I replacedall //gpu/* deps in gpu_ipc_service_unittests with one dep, //gpu, (which, among other things, eliminates a direct private dependency on gpu/command_buffer/common:common_sources). This did not resolve the issue.

Final note: It looks like gpu/command_buffer/common:common_sources is not the only culprit for doubly defined symbols. See full text of sample build failures for details.

Even more final note: It looks like some of my recent changes that helped the situation for Windows clang component debug builds of gpu_ipc_service_unittests made things worse for other builds (e.g., cut out necessary dependencies).

Any suggestions about "the right way" to approach fixing up these dependencies? Clearly my recent incantations have failed to appease the gods of meta build.

//Mark

[Nico Weber]

Is this only broken with clang? I would've expected that a gn msvc component build is just as broken, it's just that we have no try bot in that config.

What do you mean with "publicly depends"? Nothing but gpu should depend on that target, right?

[Jeremy Roman]

Found one potential issue (:test_support depends directly on the source_set target); commented on the CL.

[Brett Wilson]

When there is a component/shared_library that is composed of other targets, anything that links to stuff inside the component must do so through the component boundary.

In GYP it was almost impossible to create such a composed shared library and this caused a lot of headaches. In GN it's easy to do, but this creates the problem of duplicating code both inside and outside of the component. I suspect this is what is happening here.

In content I have carefully set visibility on the internal targets to make sure people can't depend on the wrong things directly (note: need to add comments about why or people will just whitelist more targets when they want to!). The layout of GPU is a disaster though and I've had great difficulty in wrangling it into anything sane. My only hope is that when GYP is deleted we can refactor the GN GPU build layout to be more sane.

Brett

[Will Harris]

Bruce recently helped me with a link conflict issue and he clearly know the arcane arts of linkers so +BruceDawson

Will

[Mark Dittmer]

Jeremy,

Do you mean gpu/ipc/service:BUILD.gn:

source_set("test_support") {                                                                                                                                   
  [...]                                                                                                                                                           
  deps = [                                                                                                                                                     
    "//gpu/ipc/common:ipc_common_sources",                                                                                                                     
  ]                                                                                                                                                            
}

?

Not sure whether that's one of the offending source sets, but you're right, it should probably depend on //gpu/ipc/common instead.

//Mark

[Mark Dittmer]

Jeremy, nvm. Referred to CL to answer my own question.

[Bruce Dawson]

I highly recommend link /verbose for investigating these. First grab the .rsp file and command line with "ninja -v -d keeprsp -C out\MyOutputDirectory" then get link.exe in the path:

C:\src\depot_tools\win_toolchain\vs_files\95ddda401ec5678f15eeed01d2bee08fcbc5ee97\win_sdk\bin\SetEnv.cmd

Then rerun the link command with /verbose, redirect to a file, and troll through it. You can control what goes in the verbose output (https://msdn.microsoft.com/en-us/library/wdsk6as6.aspx) but I usually just turn it all on.

Even just looking at the .rsp file can be quite informative - it summarizes everything that the linker is trying to pull in. All the .obj files get pulled in first, and then .lib files get pulled in as needed. In the gn world with source sets instead of .lib files that means that a lot of .obj files get pulled in that wouldn't previously (because .lib files are consumed as needed).

[Mark Dittmer]

Thanks Bruce. I'll probably delve into that tomorrow. It's clear from the rsp file why I'm getting duplicate symbols.

I guess what I'm asking for is helping to debug why gn/ninja is including both the cc file's .obj, and a .lib that contains the same. Is there a way of inspecting the deps that will indicate why both the "dep-on-source_set method" and the "dep-on-library" strategy are being used in the same target?

//Mark

[Brett Wilson]

On Tue, Apr 5, 2016 at 5:57 PM, Mark Dittmer <markd...@google.com> wrote:

Thanks Bruce. I'll probably delve into that tomorrow. It's clear from the rsp file why I'm getting duplicate symbols.

I guess what I'm asking for is helping to debug why gn/ninja is including both the cc file's .obj, and a .lib that contains the same. Is there a way of inspecting the deps that will indicate why both the "dep-on-source_set method" and the "dep-on-library" strategy are being used in the same target?

As I described in the post above, you're linking to this source set directly as well as via the gpu component. In the component build this means you're linking to it twice. The structure in the latest patch looks right to me.

Brett