New issue
Advanced search Search tips

Issue 727993 link

Starred by 3 users

Issue metadata

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

Blocking:
issue 732652



Sign in to add a comment

MediaRouterUITest.SetsForcedCastModeWithPresentationURLs fails on CFI Linux ToT bot

Project Member Reported by p...@chromium.org, May 31 2017

Issue description

Chrome Version: trunk
OS: Linux

What steps will reproduce the problem?
(1) cat args.gn
is_cfi = true
is_clang = true
is_component_build = false
is_debug = false
use_cfi_cast = true
use_cfi_diag = true
use_thin_lto = true
use_lld = true
(2) ninja unit_tests
(3) ./unit_tests --gtest_filter=MediaRouterUITest.SetsForcedCastModeWithPresentationURLs --single-process-tests

What is the expected result?

test passes

What happens instead?

../../chrome/browser/ui/webui/media_router/media_router_ui.cc:770:10: runtime error: control flow integrity check for type 'media_router::MediaRouterMojoImpl' failed during base-to-derived cast (vtable address 0x000000817030)
0x000000817030: note: vtable is of type 'media_router::MockMediaRouter'
 00 00 00 00  30 bf 09 09 00 00 00 00  30 c3 09 09 00 00 00 00  10 29 0c 0b 00 00 00 00  60 23 d9 06
              ^ 
(gdb) bt
#0  0x00000000069da804 in __ubsan_handle_cfi_check_fail ()
#1  0x000000000b62972b in media_router::MediaRouterUI::GetRouteProviderExtensionId() const ()
#2  0x000000000b62dac0 in media_router::MediaRouterWebUIMessageHandler::RoutesToValue(std::vector<media_router::MediaRoute, std::allocator<media_router::MediaRoute> > const&, std::vector<std::string, std::allocator<std::string> > const&, std::unordered_map<std::string, media_router::MediaCastMode, std::hash<std::string>, std::equal_to<std::string>, std::allocator<std::pair<std::string const, media_router::MediaCastMode> > > const&) const ()
#3  0x000000000b62d9ff in media_router::MediaRouterWebUIMessageHandler::UpdateRoutes(std::vector<media_router::MediaRoute, std::allocator<media_router::MediaRoute> > const&, std::vector<std::string, std::allocator<std::string> > const&, std::unordered_map<std::string, media_router::MediaCastMode, std::hash<std::string>, std::equal_to<std::string>, std::allocator<std::pair<std::string const, media_router::MediaCastMode> > > const&) ()
#4  0x000000000b627660 in media_router::MediaRouterUI::OnRoutesUpdated(std::vector<media_router::MediaRoute, std::allocator<media_router::MediaRoute> > const&, std::vector<std::string, std::allocator<std::string> > const&) ()
#5  0x000000000b62748e in media_router::MediaRouterUI::InitCommon(content::WebContents*) ()
#6  0x000000000b6278d0 in media_router::MediaRouterUI::InitForTest(media_router::MediaRouter*, content::WebContents*, media_router::MediaRouterWebUIMessageHandler*, std::unique_ptr<media_router::CreatePresentationConnectionRequest, std::default_delete<media_router::CreatePresentationConnectionRequest> >) ()
#7  0x0000000007308071 in media_router::MediaRouterUITest_SetsForcedCastModeWithPresentationURLs_Test::TestBody() ()

Please use labels and text to provide additional information.

Example failure: https://build.chromium.org/p/chromium.fyi/builders/CFI%20Linux%20ToT/builds/6321
Suspected CL: https://codereview.chromium.org/2862393002

mfoltz, can you please take a look?
 

Comment 1 by p...@chromium.org, Jun 12 2017

Ping.

Comment 2 by mfo...@chromium.org, Jun 12 2017

What is the "CFI Linux ToT" bot?  Do you have a link to documentation for it?

Comment 3 by p...@chromium.org, Jun 12 2017

This bot runs tests with control flow integrity (CFI) enabled, including bad cast checks.

Documentation: https://www.chromium.org/developers/testing/control-flow-integrity

Comment 4 by mfo...@chromium.org, Jun 12 2017

Cc: mfo...@chromium.org
Components: Internals>Cast>UI
Labels: Hotlist-Fixit-PE2017 OS-Linux
Owner: ----
Status: Available (was: Untriaged)

Comment 5 by mfo...@chromium.org, Jun 13 2017

 Issue 731883  has been merged into this issue.

Comment 6 by mfo...@chromium.org, Jun 13 2017

Cc: -mfo...@chromium.org
Owner: mfo...@chromium.org
Status: Started (was: Available)

Comment 7 by mfo...@chromium.org, Jun 13 2017

Cc: mfo...@chromium.org
Owner: ----
Status: Available (was: Started)
I'm unable to build with the provided instructions, I hit an unrelated error compiling V8 at 73af.

$ ninja -j 320 -C out/CFI unit_tests
[24487/34868] ACTION //v8:run_mksnapshot(//build/toolchain/linux:clang_x64)
FAILED: gen/v8/snapshot.cc snapshot_blob.bin 
python ../../v8/tools/run.py ./mksnapshot --startup_src gen/v8/snapshot.cc --random-seed 314159265 --startup_blob snapshot_blob.bin
../../build/linux/debian_jessie_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/ext/new_allocator.h:104:9: runtime error: control flow integrity check for type 'std::_Sp_counted_ptr_inplace<v8::internal::Counters, std::allocator<v8::internal::Counters>, __gnu_cxx::_Lock_policy::_S_atomic>' failed during cast to unrelated type (vtable address 0x000000000000)
0x000000000000: note: invalid vtable
<memory cannot be printed>
[24488/34868] LINK ./ui.service
ninja: build stopped: subcommand failed.

Comment 8 by p...@chromium.org, Jun 13 2017

You can bypass that error by adding "use_cfi_recover = true" to your args.gn.
Labels: -Pri-3 Pri-2
This bug also occurs on ubsan builds, so increasing the priority so I can run linux_chromium_ubsan_rel_ng :)
Cc: thomasanderson@chromium.org

Comment 11 by p...@chromium.org, Jun 20 2017

Blocking: 732652
This is going to block adding a CFI trybot to the CQ.
Owner: mfo...@chromium.org
There is even a TODO for this,  crbug.com/597778 .
To reproduce, build unit_tests with gn args
  is_ubsan = true
  is_ubsan_vptr = true
  is_debug = false


Project Member

Comment 13 by bugdroid1@chromium.org, Jul 10 2017

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

commit bae32143ddf7c4cf69f0f71e260a194648e0b27b
Author: takumif <takumif@chromium.org>
Date: Mon Jul 10 23:24:07 2017

[Media Router] Remove the extension ID getter from MRMojoImpl

This CL removes the media_route_provider_extension_id() getter from
MediaRouterMojoImpl, which was previously being accessed by static-casting
MediaRouter into MediaRouterMojoImpl. The getter in EventPageRequestManager is
now used.

It seems that we cannot simply hard-code the ID, since it's different between
dev and release builds of the component extension.

BUG= 597778 
BUG= 727993 

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

[modify] https://crrev.com/bae32143ddf7c4cf69f0f71e260a194648e0b27b/chrome/browser/media/router/mojo/media_router_mojo_impl.cc
[modify] https://crrev.com/bae32143ddf7c4cf69f0f71e260a194648e0b27b/chrome/browser/media/router/mojo/media_router_mojo_impl.h
[modify] https://crrev.com/bae32143ddf7c4cf69f0f71e260a194648e0b27b/chrome/browser/ui/toolbar/media_router_contextual_menu.cc
[modify] https://crrev.com/bae32143ddf7c4cf69f0f71e260a194648e0b27b/chrome/browser/ui/webui/cast/cast_ui.cc
[modify] https://crrev.com/bae32143ddf7c4cf69f0f71e260a194648e0b27b/chrome/browser/ui/webui/media_router/media_router_ui.cc
[modify] https://crrev.com/bae32143ddf7c4cf69f0f71e260a194648e0b27b/chrome/browser/ui/webui/media_router/media_router_ui.h

Status: Fixed (was: Available)
Checked locally with TOT that the test in the original post passes.
Labels: -Hotlist-Fixit-PE2017

Sign in to add a comment