New issue
Advanced search Search tips

Issue 771710 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug

Blocked on:
issue 769761



Sign in to add a comment

media_unittests doesn't link in clang/win component builds

Project Member Reported by thakis@chromium.org, Oct 4 2017

Issue description

https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.fyi%2FClangToTWin_dbg_%2F8796%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout

FAILED: media_unittests.exe media_unittests.exe.pdb 
C:/b/depot_tools/win_tools-2_7_6_bin/python/bin/python.exe ../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x86 False link.exe /nologo /OUT:./media_unittests.exe /PDB:./media_unittests.exe.pdb @./media_unittests.exe.rsp
source_buffer_stream_unittest.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) public: void __thiscall media::SourceBufferStream<class media::SourceBufferRangeByDts>::OnStartOfCodedFrameGroup(class media::DecodeTimestamp,class base::TimeDelta)" (__imp_?OnStartOfCodedFrameGroup@?$SourceBufferStream@VSourceBufferRangeByDts@media@@@media@@QAEXVDecodeTimestamp@2@VTimeDelta@base@@@Z) referenced in function "public: virtual void __thiscall media::SourceBufferStreamTest_Audio_SpliceTrimming_ExistingTrimming_Test::TestBody(void)" (?TestBody@SourceBufferStreamTest_Audio_SpliceTrimming_ExistingTrimming_Test@media@@UAEXXZ)
source_buffer_stream_unittest.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) public: void __thiscall media::SourceBufferStream<class media::SourceBufferRangeByPts>::OnStartOfCodedFrameGroup(class media::DecodeTimestamp,class base::TimeDelta)" (__imp_?OnStartOfCodedFrameGroup@?$SourceBufferStream@VSourceBufferRangeByPts@media@@@media@@QAEXVDecodeTimestamp@2@VTimeDelta@base@@@Z) referenced in function "public: virtual void __thiscall media::SourceBufferStreamTest_Audio_SpliceTrimming_ExistingTrimming_Test::TestBody(void)" (?TestBody@SourceBufferStreamTest_Audio_SpliceTrimming_ExistingTrimming_Test@media@@UAEXXZ)
./media_unittests.exe : fatal error LNK1120: 2 unresolved externals

Started here:
https://build.chromium.org/p/chromium.fyi/builders/ClangToTWin%28dbg%29/builds/8791

I bet this is from  https://chromium-review.googlesource.com/691431 and the test is calling something that isn't exported.
 
Actually, I bet this is due to https://chromium-review.googlesource.com/c/chromium/src/+/678443 somehow
Probably due to the template bits in https://chromium-review.googlesource.com/c/chromium/src/+/678443/41/media/filters/source_buffer_stream.cc

Hans, does clang-cl still inline non-exported things differently? I thought we did the same thing as cl now.

(but this is still very likely a code bug, not a compiler one)
Blocking: 769761

Comment 4 by h...@chromium.org, Oct 4 2017

> Hans, does clang-cl still inline non-exported things differently? I thought we did the same thing as cl now.

We try not to. Still not sure exactly what's happening here..

Comment 5 by h...@chromium.org, Oct 4 2017

Cc: -h...@chromium.org wolenetz@chromium.org
Owner: h...@chromium.org
Status: Assigned (was: Unconfirmed)
Looking

Comment 6 by h...@chromium.org, Oct 4 2017

Status: Started (was: Assigned)
It's a clang-cl bug :-(

We're failing to dllexport the specialization of the class-member:


template <typename> struct __declspec(dllexport) S {
  void foo();
};

struct T;
template<> void S<T>::foo() {}   <--- clang-cl fails to export this :-(

Comment 7 by h...@chromium.org, Oct 4 2017

Here's a temporary work-around: https://chromium-review.googlesource.com/c/chromium/src/+/701456

Comment 8 by h...@chromium.org, Oct 4 2017

Interestingly, free functions and member functions are different here:

template <typename T> __declspec(dllexport) void f(T*);
template<> void f<int>(int*) {} // Not exported.


template <typename> struct __declspec(dllexport) S { void foo(); };
template<> void S<int>::foo() {} // Should export!
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 4 2017

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

commit 6489c3bf719ff00615a7946c0a4bb72bb8e44b2e
Author: Hans Wennborg <hans@chromium.org>
Date: Wed Oct 04 23:05:07 2017

Work around clang-cl bug to fix media_unittests link failure

TBR=wolenetz

Bug:  771710 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Id2de82dfda1fc003ed3a8095ad000e7a21f3feb9
Reviewed-on: https://chromium-review.googlesource.com/701456
Commit-Queue: Hans Wennborg <hans@chromium.org>
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506554}
[modify] https://crrev.com/6489c3bf719ff00615a7946c0a4bb72bb8e44b2e/media/filters/source_buffer_stream.cc

Cc: zmo@chromium.org
 Issue 771651  has been merged into this issue.

Comment 11 by h...@chromium.org, Oct 5 2017

Blockedon: 769761
Blocking: -769761
Should be fixed with Clang r315025. Once we've rolled past that, we can remove the work-around.

Comment 12 by h...@chromium.org, Oct 10 2017

That got reverted. Better fix in r315330
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 18 2017

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

commit bb14695b51d970c06844f8c283359eae3a113fc4
Author: Hans Wennborg <hans@chromium.org>
Date: Wed Oct 18 03:54:29 2017

Revert "Work around clang-cl bug to fix media_unittests link failure"

This reverts commit 6489c3bf719ff00615a7946c0a4bb72bb8e44b2e.

Reason for revert:
Clang has rolled past r315330 (in #508804), so the work-around
is not necessary anymore.

Original change's description:
> Work around clang-cl bug to fix media_unittests link failure
> 
> TBR=wolenetz
> 
> Bug:  771710 
> Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
> Change-Id: Id2de82dfda1fc003ed3a8095ad000e7a21f3feb9
> Reviewed-on: https://chromium-review.googlesource.com/701456
> Commit-Queue: Hans Wennborg <hans@chromium.org>
> Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#506554}

TBR=thakis@chromium.org,wolenetz@chromium.org,hans@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  771710 
Change-Id: I6916002fd569007a006ccf25285ee1dcf33159d7
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/724339
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509679}
[modify] https://crrev.com/bb14695b51d970c06844f8c283359eae3a113fc4/media/filters/source_buffer_stream.cc

Status: Fixed (was: Started)

Sign in to add a comment