linker warnings in broadcast_channel.mojom-blink.obj |
||||||||
Issue descriptionI see these warnings when compiling (on windows, with gn, component build) after syncing: broadcast_channel.mojom-blink.obj : warning LNK4217: locally defined symbol ?deref@?$RefCounted@VSecurityOrigin@blink@@@ WTF@@QEBAXXZ (public: void __cdecl WTF::RefCounted<class blink::SecurityOrigin>::deref(void)const ) imported in function "void __cdecl WTF::derefIfNotNull<class blink::SecurityOrigin>(class blink::SecurityOrigin *)" (??$derefIfNotNull@VSecu rityOrigin@blink@@@WTF@@YAXPEAVSecurityOrigin@blink@@@Z) broadcast_channel.mojom-blink.obj : warning LNK4217: locally defined symbol ?createUnique@SecurityOrigin@blink@@SA?AV?$P assRefPtr@VSecurityOrigin@blink@@@WTF@@XZ (public: static class WTF::PassRefPtr<class blink::SecurityOrigin> __cdecl bli nk::SecurityOrigin::createUnique(void)) imported in function "public: static bool __cdecl mojo::StructTraits<class url:: mojom::blink::Origin,class WTF::RefPtr<class blink::SecurityOrigin> >::Read(class url::mojom::blink::OriginDataView,clas s WTF::RefPtr<class blink::SecurityOrigin> *)" (?Read@?$StructTraits@VOrigin@blink@mojom@url@@V?$RefPtr@VSecurityOrigin@ blink@@@WTF@@@mojo@@SA_NVOriginDataView@blink@mojom@url@@PEAV?$RefPtr@VSecurityOrigin@blink@@@WTF@@@Z) broadcast_channel.mojom-blink.obj : warning LNK4217: locally defined symbol ?create@SecurityOrigin@blink@@SA?AV?$PassRef Ptr@VSecurityOrigin@blink@@@WTF@@AEBVString@4@0H@Z (public: static class WTF::PassRefPtr<class blink::SecurityOrigin> __ cdecl blink::SecurityOrigin::create(class WTF::String const &,class WTF::String const &,int)) imported in function "publ ic: static bool __cdecl mojo::StructTraits<class url::mojom::blink::Origin,class WTF::RefPtr<class blink::SecurityOrigin > >::Read(class url::mojom::blink::OriginDataView,class WTF::RefPtr<class blink::SecurityOrigin> *)" (?Read@?$StructTrai ts@VOrigin@blink@mojom@url@@V?$RefPtr@VSecurityOrigin@blink@@@WTF@@@mojo@@SA_NVOriginDataView@blink@mojom@url@@PEAV?$Ref Ptr@VSecurityOrigin@blink@@@WTF@@@Z) broadcast_channel.mojom-blink.obj : warning LNK4217: locally defined symbol ?protocol@SecurityOrigin@blink@@QEBA?AVStrin g@WTF@@XZ (public: class WTF::String __cdecl blink::SecurityOrigin::protocol(void)const ) imported in function "public: static class WTF::String __cdecl mojo::StructTraits<class url::mojom::blink::Origin,class WTF::RefPtr<class blink::Secur ityOrigin> >::scheme(class WTF::RefPtr<class blink::SecurityOrigin> const &)" (?scheme@?$StructTraits@VOrigin@blink@mojo m@url@@V?$RefPtr@VSecurityOrigin@blink@@@WTF@@@mojo@@SA?AVString@WTF@@AEBV?$RefPtr@VSecurityOrigin@blink@@@4@@Z) broadcast_channel.mojom-blink.obj : warning LNK4217: locally defined symbol ?host@SecurityOrigin@blink@@QEBA?AVString@WT F@@XZ (public: class WTF::String __cdecl blink::SecurityOrigin::host(void)const ) imported in function "public: static c lass WTF::String __cdecl mojo::StructTraits<class url::mojom::blink::Origin,class WTF::RefPtr<class blink::SecurityOrigi n> >::host(class WTF::RefPtr<class blink::SecurityOrigin> const &)" (?host@?$StructTraits@VOrigin@blink@mojom@url@@V?$Re fPtr@VSecurityOrigin@blink@@@WTF@@@mojo@@SA?AVString@WTF@@AEBV?$RefPtr@VSecurityOrigin@blink@@@4@@Z) broadcast_channel.mojom-blink.obj : warning LNK4217: locally defined symbol ?effectivePort@SecurityOrigin@blink@@QEBAGXZ (public: unsigned short __cdecl blink::SecurityOrigin::effectivePort(void)const ) imported in function "public: static unsigned short __cdecl mojo::StructTraits<class url::mojom::blink::Origin,class WTF::RefPtr<class blink::SecurityOrigin> >::port(class WTF::RefPtr<class blink::SecurityOrigin> const &)" (?port@?$StructTraits@VOrigin@blink@mojom@url@@V?$RefP tr@VSecurityOrigin@blink@@@WTF@@@mojo@@SAGAEBV?$RefPtr@VSecurityOrigin@blink@@@WTF@@@Z) broadcast_channel.mojom-blink.obj : warning LNK4217: locally defined symbol ?isUnique@SecurityOrigin@blink@@QEBA_NXZ (pu blic: bool __cdecl blink::SecurityOrigin::isUnique(void)const ) imported in function "public: static bool __cdecl mojo:: StructTraits<class url::mojom::blink::Origin,class WTF::RefPtr<class blink::SecurityOrigin> >::Read(class url::mojom::bl ink::OriginDataView,class WTF::RefPtr<class blink::SecurityOrigin> *)" (?Read@?$StructTraits@VOrigin@blink@mojom@url@@V? $RefPtr@VSecurityOrigin@blink@@@WTF@@@mojo@@SA_NVOriginDataView@blink@mojom@url@@PEAV?$RefPtr@VSecurityOrigin@blink@@@WT F@@@Z) I'm suspecting this might be due to https://codereview.chromium.org/2158913006 ? If this suggests a missing gn dependency, we should probably fix it to avoid flaky compiles in the future.
,
Jul 27 2016
I haven't tried a clean build. My assumption was that this meant missing EXPORTs, but maybe I'm wrong.
,
Jul 27 2016
Yeah, looks like some kind of declspec issue, I guess having to do with the typemaps? I can repro as follows: del out\d\blink_platform.dll ninja -C out/d blink_platform.dll
,
Jul 28 2016
,
Jul 28 2016
,
Jul 28 2016
hey guys, sorry I just saw this. Yeah I've been seeing this locally since my cl landed, but I didn't know how important it is. It's related to component build only.
,
Jul 29 2016
It should definitely be fixed. The first warning is basically saying that this function: ?deref@?$RefCounted@VSecurityOrigin@blink@@@ WTF@@QEBAXXZ (public: void __cdecl WTF::RefCounted<class blink::SecurityOrigin>::deref(void)const ) is both defined locally and imported. That's almost an ODR violation, and certainly undesirable. As nick@ says it is presumably some sort of DLLIMPORT/DLLEXPORT issue.
,
Jul 29 2016
Yeah I'm looking into this. It's more "fun" to follow because of the generated targets.
,
Jul 29 2016
a little update: this is turning into a rabbit hole. The problem is that r406852 introduced a dependency from //third_party/WebKit/public:mojo_bindings to //url/mojo:url_mojom_origin which the typemap translated to webkit's SecurityOrigin. Many targets depend on //third_party/WebKit/public:mojo_bindings, specifically: "//content/app:both", "//content/app:content_main_runner_both", "//content/browser:browser", "//content/common:mojo_bindings", "//content/common:mojo_bindings_blink", "//content/public/browser:browser_sources", "//content/renderer:renderer", "//content/test:content_unittests", "//content/utility:utility", "//device/geolocation:device_geolocation", "//third_party/WebKit/Source/modules:modules", "//third_party/WebKit/Source/platform", "//third_party/WebKit/Source/web:webkit_unit_tests", SecurityOrigin itself is defined in third_party/WebKit/Source/platform, so we have a circular dependency. I tried to separate out the platform/weborigin into a separate component, which both public:mojo_bindings and Source/platform depend on. However the weborigin code uses RuntimeEnabledFeatures which is part of platform. So then either we'd have to split out RuntimeEnabledFeatures as well, or we can push the runtime flag state to weborigin code.
,
Jul 29 2016
Another option is that we can make the generated mojo bindings to be exported. The targets in webkit that currently depend on public:mojo_bindings would then only depend on Source/platform. However, since the bindings are also used in the browser process, we would have to make the exporting only happen for the blink version of the binding. I'm currently not inclined to go down this path, as it adds more magic to the binding generation.
,
Jul 30 2016
I experimented with the idea of inserting export macros into bindings before. (https://codereview.chromium.org/1641163005/) With that approach, the targets who are outside of platform and would like to use public:mojo_bindings cannot directly depend on public:mojo_bindings, because it is built as "implementation". I feel that it may be a little confusing for users.
,
Aug 1 2016
,
Aug 3 2016
Where does the circular dependency factor in? If broadcast_channel.mojom-blink.obj is being linked into blink_platform.dll, oughtn't it have BLINK_PLATFORM_IMPLEMENTATION set to true? If I edit the generated broadcast_channel.mojom-blink.cc file so that I force "#define BLINK_PLATFORM_IMPLEMENTATION 1" at the top, it seems like the linker warnings for blink_platform.dll go away.
,
Aug 4 2016
Upon inspecting the generated ninja files, it seems like obj/third_party/WebKit/public/mojo_bindings_blink_cpp_sources/broadcast_channel.mojom-blink.obj is specified as a linker input into both of the following dll steps: build ./blink_platform.dll ./blink_platform.dll.lib: solink build ./blink_modules.dll ./blink_modules.dll.lib: solink Is that really the intended behavior?
,
Aug 4 2016
Nick: it's a source set, and it's also linked into webkit_unit_tests. This, and the above, are working as intended since platform and tests can use the mojo interfaces. BLINK_PLATFORM_IMPLEMENTATION can't be set for this source_set since otherwise there would be linker errors for the non-platform targets when they link with the source set.
,
Aug 5 2016
We're getting a bunch of the same stuff in web_bluetooth.mojom.obj now (regarding our wondering if this was going to come up frequently) web_bluetooth.mojom.obj : warning LNK4217: locally defined symbol ??0WebBluetoothDeviceId@content@@QAE@XZ (public: __thiscall content::WebBluetoothDeviceId::WebBluetoothDeviceId(void)) imported in function "public: __thiscall blink::mojom::WebBluetoothDevice::WebBluetoothDevice(void)" (??0WebBluetoothDevice@mojom@blink@@QAE@XZ) web_bluetooth.mojom.obj : warning LNK4217: locally defined symbol ??0WebBluetoothDeviceId@content@@QAE@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@Z (public: __thiscall content::WebBluetoothDeviceId::WebBluetoothDeviceId(class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> >)) imported in function "public: static bool __cdecl mojo::StructTraits<class blink::mojom::WebBluetoothDeviceId,class content::WebBluetoothDeviceId>::Read(class blink::mojom::WebBluetoothDeviceIdDataView,class content::WebBluetoothDeviceId *)" (?Read@?$StructTraits@VWebBluetoothDeviceId@mojom@blink@@V1content@@@mojo@@SA_NVWebBluetoothDeviceIdDataView@mojom@blink@@PAVWebBluetoothDeviceId@content@@@Z) web_bluetooth.mojom.obj : warning LNK4217: locally defined symbol ??1WebBluetoothDeviceId@content@@QAE@XZ (public: __thiscall content::WebBluetoothDeviceId::~WebBluetoothDeviceId(void)) imported in function "public: __thiscall blink::mojom::WebBluetoothDevice::~WebBluetoothDevice(void)" (??1WebBluetoothDevice@mojom@blink@@QAE@XZ) web_bluetooth.mojom.obj : warning LNK4217: locally defined symbol ?str@WebBluetoothDeviceId@content@@QBEABV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@XZ (public: class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const & __thiscall content::WebBluetoothDeviceId::str(void)const ) imported in function "public: static class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const & __cdecl mojo::StructTraits<class blink::mojom::WebBluetoothDeviceId,class content::WebBluetoothDeviceId>::device_id(class content::WebBluetoothDeviceId const &)" (?device_id@?$StructTraits@VWebBluetoothDeviceId@mojom@blink@@V1content@@@mojo@@SAABV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@ABVWebBluetoothDeviceId@content@@@Z) web_bluetooth.mojom.obj : warning LNK4217: locally defined symbol ?IsValid@WebBluetoothDeviceId@content@@SA_NABV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@Z (public: static bool __cdecl content::WebBluetoothDeviceId::IsValid(class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const &)) imported in function "public: static bool __cdecl mojo::StructTraits<class blink::mojom::WebBluetoothDeviceId,class content::WebBluetoothDeviceId>::Read(class blink::mojom::WebBluetoothDeviceIdDataView,class content::WebBluetoothDeviceId *)" (?Read@?$StructTraits@VWebBluetoothDeviceId@mojom@blink@@V1content@@@mojo@@SA_NVWebBluetoothDeviceIdDataView@mojom@blink@@PAVWebBluetoothDeviceId@content@@@Z) web_bluetooth.mojom.obj : warning LNK4217: locally defined symbol ??4WebBluetoothDeviceId@content@@QAEAAV01@ABV01@@Z (public: class content::WebBluetoothDeviceId & __thiscall content::WebBluetoothDeviceId::operator=(class content::WebBluetoothDeviceId const &)) imported in function "public: static bool __cdecl mojo::StructTraits<class blink::mojom::WebBluetoothDeviceId,class content::WebBluetoothDeviceId>::Read(class blink::mojom::WebBluetoothDeviceIdDataView,class content::WebBluetoothDeviceId *)" (?Read@?$StructTraits@VWebBluetoothDeviceId@mojom@blink@@V1content@@@mojo@@SA_NVWebBluetoothDeviceIdDataView@mojom@blink@@PAVWebBluetoothDeviceId@content@@@Z)
,
Aug 5 2016
@scottmg: that one has a different cause. I believe the issue with it is that a typemapping was added for a mojo type that's defined in blink (blink.mojom.WebBluetoothDeviceIdd) that mapped it to a type inside content (content::WebBluetoothDeviceId). That means that the source_set for blink's mojo_bindings target which content links with has web_bluetooth_device_id.h as part of the source_set. That is compiled without CONTENT_IMPLEMENTATION set. When this source_set is linked with content.dll, which has CONTENT_IMPLEMENTATION set, then the linker complains since content.dll does actually link in this file (with CONTENT_IMPLEMENTATION set). As a tangent, the reason this slipped through is that gn check isn't turned on for webkit/public, which is where the mojom_bindings target is. In the fix for this bug (not bluetooth) https://codereview.chromium.org/2209883002/, I moved the mojo_bindings target from webkit/public to webkit/source which is analyze checks. This meant I had to allow content:export to be depended on by the binding generated target in blink, which is odd. As for solutions: -the above cl won't work for the bluetooth warning, since it depends on having the mojom target in the same build file as the one that's linking to it -I wondered about making mojom template be a component instead of a source_set, but then there'll be a circular dependency between that component and blink or content (since the mojoms use types defined by them) -it's almost like we want the mojom template to output a list of source files that aren't compiled, but are compiled with whatever is including them (i.e. an automatic version of what the cl above is doing). I don't think GN has such a way right now though. Thoughts?
,
Aug 5 2016
So I chatted with Brett about this. His recommendation is that we should make mojo_bindings get passed in the export macro (different for each variation). visibility rules would ensure that only blink's platform or content's main target depended on each of the variants. Any other target in blink that depended on it now would depend on blink, and same for content. This seems like the only sane solution (Ken and I also brainstormed about other solutions but all had problems).
,
Aug 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b97b473d63918a01716254be849d248a7d3eefa8 commit b97b473d63918a01716254be849d248a7d3eefa8 Author: jam <jam@chromium.org> Date: Mon Aug 08 20:48:29 2016 Support exporting Mojo bindings for the component build. The problem is when using typemaps, the source_set for the generated bindings may include headers from components they're linking with. The export macros for those components need to match other files in the component, or else the linker warnings in the bug below are encountered. A couple of other solutions were attempted first, since they seemed simpler, but they didn't work out: -make the typemapped types be separate components: https://codereview.chromium.org/2194973002/. This wouldn't work for the weborigin example as future plans render making it a leaf-node not possible. -make the other components compile the generated bindings as part of their source: https://codereview.chromium.org/2209883002/. This solution requires the mojom target to be in the same build file as the component, which doesn't work when the issue is hit with both variants. BUG= 632082 Review-Url: https://codereview.chromium.org/2225673002 Cr-Commit-Position: refs/heads/master@{#410451} [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/content/common/BUILD.gn [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/content/test/BUILD.gn [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/mojo/public/tools/bindings/generators/cpp_templates/enum_serialization_declaration.tmpl [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/mojo/public/tools/bindings/generators/cpp_templates/interface_declaration.tmpl [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/mojo/public/tools/bindings/generators/cpp_templates/interface_proxy_declaration.tmpl [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/mojo/public/tools/bindings/generators/cpp_templates/interface_request_validator_declaration.tmpl [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/mojo/public/tools/bindings/generators/cpp_templates/interface_response_validator_declaration.tmpl [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/mojo/public/tools/bindings/generators/cpp_templates/interface_stub_declaration.tmpl [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/mojo/public/tools/bindings/generators/cpp_templates/module-internal.h.tmpl [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_declaration.tmpl [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/mojo/public/tools/bindings/generators/cpp_templates/union_data_view_declaration.tmpl [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/mojo/public/tools/bindings/generators/cpp_templates/union_serialization_declaration.tmpl [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/mojo/public/tools/bindings/generators/cpp_templates/wrapper_union_class_declaration.tmpl [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/mojo/public/tools/bindings/generators/mojom_cpp_generator.py [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/mojo/public/tools/bindings/mojom.gni [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/mojo/public/tools/bindings/mojom_bindings_generator.py [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/mojo/public/tools/bindings/pylib/mojom/generate/generator.py [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/third_party/WebKit/Source/modules/BUILD.gn [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/third_party/WebKit/Source/platform/BUILD.gn [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/third_party/WebKit/Source/web/BUILD.gn [modify] https://crrev.com/b97b473d63918a01716254be849d248a7d3eefa8/third_party/WebKit/public/BUILD.gn
,
Aug 9 2016
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by nick@chromium.org
, Jul 27 2016