New issue
Advanced search Search tips

Issue 882997 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug

Blocking:
issue 330260



Sign in to add a comment

Windows component build broken, at least with goma disabled

Project Member Reported by thakis@chromium.org, Sep 11

Issue description

Fails on:

https://ci.chromium.org/buildbot/chromium.clang/ToTWin%28dbg%29/
https://ci.chromium.org/buildbot/chromium.clang/ToTWin(dll)
https://ci.chromium.org/buildbot/chromium.clang/ToTWin64(dll)
https://ci.chromium.org/buildbot/chromium.clang/ToTWin64(dbg)

Error:
https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.clang%2FToTWin64_dll_%2F1083%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout

[46443/52219] LINK(DLL) blink_modules.dll blink_modules.dll.lib blink_modules.dll.pdb
FAILED: blink_modules.dll blink_modules.dll.lib blink_modules.dll.pdb 
ninja -t msvc -e environment.x64 -- ../../third_party/llvm-build/Release+Asserts/bin/lld-link.exe /nologo /IMPLIB:./blink_modules.dll.lib /DLL /OUT:./blink_modules.dll /PDB:./blink_modules.dll.pdb @./blink_modules.dll.rsp
lld-link: error: undefined symbol: __imp_??4?$Supplement@VLocalFrame@blink@@@blink@@QEAAAEAV01@AEBV01@@Z
>>> referenced by C:\b\c\b\ToTWin64_dll_\src\third_party\blink\renderer\modules\audio_output_devices\audio_output_device_client.h:19
>>>               obj/third_party/blink/renderer/modules/modules/modules_initializer.obj:(??4AudioOutputDeviceClient@blink@@QEAAAEAV01@AEBV01@@Z)
lld-link: error: undefined symbol: __imp_??0?$Supplement@VLocalFrame@blink@@@blink@@QEAA@AEBV01@@Z
>>> referenced by C:\b\c\b\ToTWin64_dll_\src\third_party\blink\renderer\modules\audio_output_devices\audio_output_device_client.h:19
>>>               obj/third_party/blink/renderer/modules/modules/modules_initializer.obj:(??0AudioOutputDeviceClient@blink@@QEAA@AEBV01@@Z)
lld-link: error: undefined symbol: __imp_??4?$Supplement@VLocalFrame@blink@@@blink@@QEAAAEAV01@AEBV01@@Z
>>> referenced by obj/third_party/blink/renderer/modules/audio_output_devices/audio_output_devices/audio_output_device_client.obj
lld-link: error: undefined symbol: __imp_??0?$Supplement@VLocalFrame@blink@@@blink@@QEAA@AEBV01@@Z
>>> referenced by obj/third_party/blink/renderer/modules/audio_output_devices/audio_output_devices/audio_output_device_client.obj
lld-link: error: undefined symbol: __imp_??4?$Supplement@VLocalFrame@blink@@@blink@@QEAAAEAV01@AEBV01@@Z
>>> referenced by obj/third_party/blink/renderer/modules/audio_output_devices/audio_output_devices/audio_output_device_client_impl.obj
lld-link: error: undefined symbol: __imp_??0?$Supplement@VLocalFrame@blink@@@blink@@QEAA@AEBV01@@Z
>>> referenced by obj/third_party/blink/renderer/modules/audio_output_devices/audio_output_devices/audio_output_device_client_impl.obj
lld-link: error: undefined symbol: __imp_??4?$Supplement@VLocalFrame@blink@@@blink@@QEAAAEAV01@AEBV01@@Z
>>> referenced by obj/third_party/blink/renderer/modules/audio_output_devices/audio_output_devices/html_media_element_audio_output_device.obj
lld-link: error: undefined symbol: __imp_??0?$Supplement@VLocalFrame@blink@@@blink@@QEAA@AEBV01@@Z
>>> referenced by obj/third_party/blink/renderer/modules/audio_output_devices/audio_output_devices/html_media_element_audio_output_device.obj



Probably only happens with precompiled headers enabled, and they are off if goma is on. I do see this locally. Not clear from the blamelist what broke it, but since I see it with pinned clang it's due to a Chromium change.


I'll see if I can bisect or something, but one bisect step takes like 50 min, so it'll be a while :-(
 
Blocking: 330260
This is very likely due to https://chromium-review.googlesource.com/1215370

The build was red and turned green when that was reverted in https://ci.chromium.org/buildbot/chromium.clang/ToTWin64%28dll%29/1079 (and there isn't really any other functional change in that build) and then turned red again when it relanded.
Cc: szager@chromium.org
Components: -Blink>WebAudio Blink>WebRTC
Updating component since the directory modules/audio_output_devices OWNER is WebRTC.
Reverting 61b3108056747ea20d02627ec65a02c550e42c59  locally does let blink_modules.dll link locally again.
Cc: tkent@chromium.org
This is almost certainly related to:

https://bugs.chromium.org/p/chromium/issues/detail?id=881537

I followed the suggestion from thakis@ there to add a line to execution_context.h

FYI, tkent@ is also poking around this:

https://chromium-review.googlesource.com/c/chromium/src/+/1218388
Owner: tkent@chromium.org
Status: Started (was: Untriaged)
I can confirm that https://chromium-review.googlesource.com/c/chromium/src/+/1218388 fixes this too.

I don't understand why the problem is happening or why that helps, though.
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 11

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

commit 8c0f2dd7e749214e1cf96de4ba9c353005f26967
Author: Kent Tamura <tkent@chromium.org>
Date: Tue Sep 11 23:15:00 2018

Reduce the size of execution_context.h.

It is used in 4,300+ compilation units.
http://crrev.com/590155 increased its pre-processed size by 3.72MB, and
this CL fixes it.

Bug: 242216, 881537,  882997 
Change-Id: If723b9d77dbd06fdde184498aa6ed7aa3e5e88e2
Reviewed-on: https://chromium-review.googlesource.com/1218388
Commit-Queue: Kent Tamura <tkent@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590526}
[modify] https://crrev.com/8c0f2dd7e749214e1cf96de4ba9c353005f26967/third_party/blink/renderer/core/execution_context/execution_context.h
[modify] https://crrev.com/8c0f2dd7e749214e1cf96de4ba9c353005f26967/third_party/blink/renderer/core/intersection_observer/intersection_observer_controller.cc

I also don't understand the logic precisely.
We expect Supplement<LocalFrame> is inlined entirely.  https://chromium-review.googlesource.com/c/chromium/src/+/1215370 added local_frame.h and supplementable.h to precompile_core.h, and it prevented the inlining somehow?

Status: Fixed (was: Started)
I haven't looked closely at the Chromium change, but if local_frame.h did indeed become part of precompile_core.h, that was probably the trigger.

I do think I understand the bug from Clang's point of view though: https://bugs.llvm.org/show_bug.cgi?id=38934
Clang r342240 should handle this.

Sign in to add a comment