New issue
Advanced search Search tips

Issue 843976 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

PixelInspectTileManagerTest/LowResHasNoImage use after free due to TestSoftwareRasterBufferProvider going out of scope

Project Member Reported by kkinnu...@nvidia.com, May 17 2018

Issue description

Chrome Version: 
OS: (e.g. Win10, MacOS 10.12, etc...)

What steps will reproduce the problem?
(1) compile cc_unittests (with MSVC if it would work)
gn gen out\dm --args="is_debug=true is_component_build=true is_clang=false" && ninja -C out\dm cc_unittests -k0 && out\dm\cc_unittests --gtest_filter=*LowResHasNoImage
(2) run

What is the expected result?
pass

What happens instead?
purevirtual exception on MSVC due to use-after-free
should be use-after-free on clang too.

in my version of the tree it's:
(calls   raster_buffer_provider_->Shutdown(); on stale pointer)
        cc::TileManager::FinishTasksAndCleanUp [0x00007FF99D198B6E+718] (
	cc::LayerTreeHostImpl::CleanUpTileManagerResources [0x00007FF99D258F4C+28] 
	cc::LayerTreeHostImpl::ReleaseLayerTreeFrameSink [0x00007FF99D266C62+402] 
	cc::FakeLayerTreeHostImpl::~FakeLayerTreeHostImpl [0x00007FF698CE34C3+131] 
	cc::`anonymous namespace'::TileManagerTest::MockLayerTreeHostImpl::~MockLayerTreeHostImpl [0x00007FF69838DA1C+76]
	testing::NiceMock<cc::`anonymous namespace'::TileManagerTest::MockLayerTreeHostImpl>::~NiceMock<cc::`anonymous namespace'::TileManagerTest::MockLayerTreeHostImpl> [0x00007FF69838C654+148] 
	testing::NiceMock<cc::`anonymous namespace'::TileManagerTest::MockLayerTreeHostImpl>::`scalar deleting destructor' [0x00007FF698390CF7+23]
	std::default_delete<cc::FakeLayerTreeHostImpl>::operator() [0x00007FF697CBA572+66] 
	std::unique_ptr<cc::FakeLayerTreeHostImpl,std::default_delete<cc::FakeLayerTreeHostImpl> >::~unique_ptr<cc::FakeLayerTreeHostImpl,std::default_delete<cc::FakeLayerTreeHostImpl> > [0x00007FF697CB9B81+65] 
	cc::TestLayerTreeHostBase::~TestLayerTreeHostBase [0x00007FF698D11419+41] 
	cc::`anonymous namespace'::TileManagerTest::~TileManagerTest [0x00007FF69838DDF3+19]
	cc::`anonymous namespace'::PixelInspectTileManagerTest::~PixelInspectTileManagerTest [0x00007FF69838DB54+36]
	cc::`anonymous namespace'::PixelInspectTileManagerTest_LowResHasNoImage_Test::~PixelInspectTileManagerTest_LowResHasNoImage_Test [0x00007FF69838DB83+19]
	cc::`anonymous namespace'::PixelInspectTileManagerTest_LowResHasNoImage_Test::`scalar deleting destructor' [0x00007FF698392327+23]
	testing::Test::DeleteSelf_ [0x00007FF698993CE9+57] 

PixelInspectTileManagerTest sets SetRasterBufferProviderForTesting(
        &raster_buffer_provider_);
superclass ~TileManagerTest will use |raster_buffer_provider_| after |raster_buffer_provider_| has been destroyed

The bug was added in "cc: Allocate shared mem directly in BitmapRasterBufferProvider"

My repro version is few days older, but I still see the same code structure in master.
I'm sorry, at the moment unable to build the exact master, and it does not seem to build with MSVC for me. So please forgive if the bug is already fixed..
 
Summary: PixelInspectTileManagerTest/LowResHasNoImage use after free due to TestSoftwareRasterBufferProvider going out of scope (was: PixelInspectTileManagerTest/LowResHasNoImage use after free due to )
Components: Internals>Compositing>Rasterization
Owner: danakj@chromium.org
Status: Assigned (was: Untriaged)
Project Member

Comment 5 by bugdroid1@chromium.org, May 22 2018

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

commit dcfaa9607905caa05e184dff6d9298886769ecc4
Author: Kimmo Kinnunen <kkinnunen@nvidia.com>
Date: Tue May 22 16:05:59 2018

cc: Avoid using destroyed object in PixelInspectTileManagerTest.LowResHasNoImage

Avoid using destroyed FakeRasterBufferProvider in
PixelInspectTileManagerTest.LowResHasNoImage

Bug:  843976 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ic2bfd90a1fefc3cedd7e992bd89ac27e6b4c36f6
Reviewed-on: https://chromium-review.googlesource.com/1066011
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560623}
[modify] https://crrev.com/dcfaa9607905caa05e184dff6d9298886769ecc4/cc/tiles/tile_manager_unittest.cc

Comment 6 by danakj@chromium.org, May 22 2018

Status: Fixed (was: Assigned)

Sign in to add a comment