New issue
Advanced search Search tips

Issue 919229 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 8
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

Source file is both #included and in BUILD.gn

Project Member Reported by brucedaw...@chromium.org, Jan 4

Issue description

When building chrome with these settings I hit multiple duplicate symbol errors. The full error messages are attached. This bug is for one of the issues.

use_lld = false
is_component_build = false
is_debug = false
target_cpu = "x86"
enable_nacl = false
symbol_level = 2
use_jumbo_build = true

Some are because taskbar_decorator_win.cc is both #included (???) and is also in the BUILD.gn file, leading to it getting compiled twice (unrelated to jumbo builds). This was caused by crrev.com/c/1353049. I think it is the use of link.exe which has exposed these latent (but real!) bugs. The fix is to exclude taskbar_decorator_win.cc from one of those two places. If the #include is retained then please have a comment explaining why, both at the #include location and in the BUILD.gn file.

 
duplicate_symbol_errors.txt
8.0 KB View Download
Owner: harrisjay@chromium.org
Status: Assigned (was: Untriaged)
https://bugs.llvm.org/show_bug.cgi?id=40094 tracks getting lld-link to report this error.
This bug only seems to show up in jumbo builds, but that is just luck. In non-jumbo builds the linker happens to pull in badge_service_delegate_impl.obj first and therefore it never needs to pull in taskbar_decorator_win.obj and therefore there are no symbol duplication errors. However this is not guaranteed and if it happens to pull in taskbar_decorator_win.obj first then (in link.exe, and lld-link.exe once the feature-request/bug is fixed) there will be duplicate symbol errors.
Thanks for reporting. I assume this isn't urgent enough to revert the CL (which now landed over 3 weeks ago), and we can just make the fix in the code. Seems like an innocent typo (".cc" instead of ".h") but I'll let Jay deal with this. (Should be back tomorrow.)
I agree that reverting would be unnecessarily disruptive. As long as a fix is landed fairly soon (which should be easy) I'll be content.
Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ccbba773dd5e3114ee88ef84a2dc5b0f30dcb3ad

commit ccbba773dd5e3114ee88ef84a2dc5b0f30dcb3ad
Author: Jay Harris <harrisjay@chromium.org>
Date: Tue Jan 08 00:57:25 2019

Fixes an accidental include of taskbar_decorator_win.cc

Bug:  919229 
Change-Id: I8be00e0d222081707e1f8e0ef3903e5a9feae457
Reviewed-on: https://chromium-review.googlesource.com/c/1399681
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Jay Harris <harrisjay@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620558}
[modify] https://crrev.com/ccbba773dd5e3114ee88ef84a2dc5b0f30dcb3ad/chrome/browser/ui/views/badge_service_delegate_impl.cc

Status: Fixed (was: Started)

Sign in to add a comment