New issue
Advanced search Search tips

Issue 723893 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug

Blocking:
issue 683729



Sign in to add a comment

Bad code-gen in Chrome's mojo_public_bindings_unittests

Project Member Reported by brucedaw...@chromium.org, May 17 2017

Issue description

This is a tracking bug for a VC++ 2017 code-gen bug, filed here:

https://developercommunity.visualstudio.com/content/problem/40904/bad-code-gen-in-chromes-mojo-public-bindings-unitt.html

Repro steps: Generate an out\vs2017_test directory with these gn args: is_debug = true target_cpu = "x86"

> ninja -C out\vs2017_test mojo_public_bindings_unittests
> out\vs2017_test\mojo_public_bindings_unittests.exe --gtest_filter=PickleTest.BlinkProxyTo* --single-process-tests

If you run the test binary under the debugger it will crash and be caught by the debugger. A partial call stack is:
mojo_public_bindings_unittests.exe!scoped_refptr<base::internal::BindStateBase>::operator->() Line 484 C++
mojo_public_bindings_unittests.exe!base::internal::CallbackBase<0>::polymorphic_invoke() Line 127 C++
mojo_public_bindings_unittests.exe!base::Callback<void __cdecl(mojo::test::PickledStructBlink),1,1>::Run(mojo::test::PickledStructBlink <args_0>) Line 79 C++
mojo_public_bindings_unittests.exe!mojo::test::`anonymous namespace'::BlinkPicklePasserImpl::PassPickledStruct(mojo::test::PickledStructBlink pickle, const base::Callback<void __cdecl(mojo::test::PickledStructBlink),1,1> & callback) Line 126 C++
mojo_public_bindings_unittests.exe!mojo::test::blink::PicklePasserStubDispatch::AcceptWithResponder(mojo::test::blink::PicklePasser impl, mojo::Message message, std::unique_ptr<mojo::MessageReceiverWithStatus,std::default_delete<mojo::MessageReceiverWithStatus> > responder) Line 907 C++
mojo_public_bindings_unittests.exe!mojo::test::blink::PicklePasserStub<mojo::RawPtrImplRefTraits<mojo::test::blink::PicklePasser> >::AcceptWithResponder(mojo::Message message, std::unique_ptr<mojo::MessageReceiverWithStatus,std::default_delete<mojo::MessageReceiverWithStatus> > responder) Line 238 C++
mojo_public_bindings_unittests.exe!mojo::InterfaceEndpointClient::HandleValidatedMessage(mojo::Message message) Line 383 C++
mojo_public_bindings_unittests.exe!mojo::InterfaceEndpointClient::HandleIncomingMessageThunk::Accept(mojo::Message message) Line 130 C++
mojo_public_bindings_unittests.exe!mojo::FilterChain::Accept(mojo::Message message) Line 41 C++

The bug occurs with is_component_build = false or = true.

The discovery and analysis of this bug is discussed on comment #76 of the Chromium VS 2017 tracking bug, here:

https://bugs.chromium.org/p/chromium/issues/detail?id=683729#c76
 
Note that crrev.com/2826713003 disabled these tests so that they no longer visibly fail. However this disabling is specific to an exact compiler version so the tests are automatically re-enabled when building with VS 2017 Update 3 Preview. And, unfortunately, with that compiler (_MSC_FULL_VER equal to 191125303) the tests still fail.

Comment 2 by r...@chromium.org, May 22 2017

Cc: r...@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, May 26 2017

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

commit 6d1398efad86b7c862e83719602aed55c35d0e28
Author: brucedawson <brucedawson@chromium.org>
Date: Fri May 26 16:00:53 2017

Disable tests on VS 2017 Preview, code-gen bug

VS 2017 has a code-gen bug, discussed and reported here:

https://developercommunity.visualstudio.com/content/problem/40904/bad-code-gen-in-chromes-mojo-public-bindings-unitt.html

As of VS 2017 Update 3 Preview 1 the bug is still not fixed, so this CL
adjusts the checks to continue disabling the failing tests to allow
testing with this VC version. The next preview should have the fix -
probably. crrev.com/2826713003 disabled the tests for VS 2017 RTM.

R=jam@chromium.org
BUG= 723893 

Review-Url: https://codereview.chromium.org/2909593002
Cr-Commit-Position: refs/heads/master@{#475017}

[modify] https://crrev.com/6d1398efad86b7c862e83719602aed55c35d0e28/mojo/public/cpp/bindings/tests/pickle_unittest.cc

Status: Fixed (was: Assigned)
I have confirmed that with cl.exe 19.11.25325 (VS 2017 Update 3 Preview 2, released June 8, https://www.visualstudio.com/en-us/news/releasenotes/vs2017-preview-relnotes) this code-gen bug is fixed.

When running the test command-line from the initial repro using the recommend gn args the two tests run successfully. On the previous version (cl.exe version 19.11.25303, VS 2017 Update 3 Preview 1) they failed until they were disabled. The disabling was version specific so that the tests were automatically re-enabled for the new compiler version.

Sign in to add a comment