exo_unittests's SurfaceTest.RequestFrameCallback, ShellSurfaceTest.ConfigureCallback fails under asan due to stack-use-after-scope |
|||||
Issue descriptionI'm trying to add exo_unittests to the asan bot. 4 tests fail: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_chromeos_asan_rel_ng/15672 SurfaceTest.RequestFrameCallback/1 SurfaceTest.RequestFrameCallback/0 ShellSurfaceTest.ConfigureCallback SurfaceTest.RequestFrameCallback/2 Stacks: WRITE of size 8 at 0x7f166bd1e820 thread T0 #0 0x6d1b4d in exo::(anonymous namespace)::SetFrameTime(base::TimeTicks*, base::TimeTicks) components/exo/surface_unittest.cc:167:11 #1 0x7e593a in Run base/callback.h:125:12 #2 0x7e593a in exo::Surface::~Surface() components/exo/surface.cc:212 #3 0x6d1945 in operator() buildtools/third_party/libc++/trunk/include/memory:2321:5 #4 0x6d1945 in reset buildtools/third_party/libc++/trunk/include/memory:2634 #5 0x6d1945 in ~unique_ptr buildtools/third_party/libc++/trunk/include/memory:2588 #6 0x6d1945 in exo::(anonymous namespace)::SurfaceTest_RequestFrameCallback_Test::TestBody() components/exo/surface_unittest.cc:180 #7 0x2d62262 in testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc #8 0x2d641c4 in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2667:11 #9 0x2d65596 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2785:28 #10 0x2d8ac96 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:5047:43 #11 0x2d89ee5 in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc #12 0x300e16a in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2329:46 #13 0x300e16a in base::TestSuite::Run() base/test/test_suite.cc:275 #14 0x301334b in Run base/callback.h:96:12 #15 0x301334b in base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, bool, base::OnceCallback<void ()>) base/test/launcher/unit_test_launcher.cc:225 #16 0x30143a1 in base::LaunchUnitTestsSerially(int, char**, base::OnceCallback<int ()>) base/test/launcher/unit_test_launcher.cc:585:10 #17 0x51f39c in main components/exo/test/run_all_unittests.cc:20:10 #18 0x7f166f52cf44 in __libc_start_main /build/eglibc-ripdx6/eglibc-2.19/csu/libc-start.c:287 Address 0x7f166bd1e820 is located in stack of thread T0 at offset 32 in frame #0 0x6d14cf in exo::(anonymous namespace)::SurfaceTest_RequestFrameCallback_Test::TestBody() components/exo/surface_unittest.cc:170 This frame has 6 object(s): [32, 40) 'frame_time' (line 173) <== Memory access at offset 32 is inside this variable [64, 72) 'ref.tmp' (line 174) [96, 112) 'gtest_ar_' (line 179) [128, 136) 'ref.tmp10' (line 179) [160, 168) 'temp.lvalue' [192, 216) 'ref.tmp11' (line 179) WRITE of size 8 at 0x7f2afa70e120 thread T0 #0 0x6bb76d in exo::(anonymous namespace)::Configure(gfx::Size*, ash::mojom::WindowStateType*, bool*, bool*, gfx::Size const&, ash::mojom::WindowStateType, bool, bool, gfx::Vector2d const&) components/exo/shell_surface_unittest.cc:453:19 #1 0x6bb879 in Invoke<unsigned int (*const &)(gfx::Size *, ash::mojom::WindowStateType *, bool *, bool *, const gfx::Size &, ash::mojom::WindowStateType, bool, bool, const gfx::Vector2d &), gfx::Size *, ash::mojom::WindowStateType *, bool *, bool *, const gfx::Size &, ash::mojom::WindowStateType, bool, bool, const gfx::Vector2d &> base/bind_internal.h:407:12 #2 0x6bb879 in MakeItSo<unsigned int (*const &)(gfx::Size *, ash::mojom::WindowStateType *, bool *, bool *, const gfx::Size &, ash::mojom::WindowStateType, bool, bool, const gfx::Vector2d &), gfx::Size *, ash::mojom::WindowStateType *, bool *, bool *, const gfx::Size &, ash::mojom::WindowStateType, bool, bool, const gfx::Vector2d &> base/bind_internal.h:607 #3 0x6bb879 in RunImpl<unsigned int (*const &)(gfx::Size *, ash::mojom::WindowStateType *, bool *, bool *, const gfx::Size &, ash::mojom::WindowStateType, bool, bool, const gfx::Vector2d &), const std::__1::tuple<base::internal::UnretainedWrapper<gfx::Size>, base::internal::UnretainedWrapper<ash::mojom::WindowStateType>, base::internal::UnretainedWrapper<bool>, base::internal::UnretainedWrapper<bool> > &, 0, 1, 2, 3> base/bind_internal.h:681 #4 0x6bb879 in base::internal::Invoker<base::internal::BindState<unsigned int (*)(gfx::Size*, ash::mojom::WindowStateType*, bool*, bool*, gfx::Size const&, ash::mojom::WindowStateType, bool, bool, gfx::Vector2d const&), base::internal::UnretainedWrapper<gfx::Size>, base::internal::UnretainedWrapper<ash::mojom::WindowStateType>, base::internal::UnretainedWrapper<bool>, base::internal::UnretainedWrapper<bool> >, unsigned int (gfx::Size const&, ash::mojom::WindowStateType, bool, bool, gfx::Vector2d const&)>::Run(base::internal::BindStateBase*, gfx::Size const&, ash::mojom::WindowStateType, bool, bool, gfx::Vector2d const&) base/bind_internal.h:663 #5 0x7cb0ba in Run base/callback.h:125:12 #6 0x7cb0ba in exo::ShellSurfaceBase::Configure() components/exo/shell_surface_base.cc:1339 #7 0x7cdcab in exo::ShellSurfaceBase::EndDrag(bool) components/exo/shell_surface_base.cc:1618:5 #8 0x7cd044 in exo::ShellSurfaceBase::~ShellSurfaceBase() components/exo/shell_surface_base.cc:402:5 #9 0x7c736d in exo::ShellSurface::~ShellSurface() components/exo/shell_surface.cc:83:31 #10 0x6bac99 in operator() buildtools/third_party/libc++/trunk/include/memory:2321:5 #11 0x6bac99 in reset buildtools/third_party/libc++/trunk/include/memory:2634 #12 0x6bac99 in ~unique_ptr buildtools/third_party/libc++/trunk/include/memory:2588 #13 0x6bac99 in exo::(anonymous namespace)::ShellSurfaceTest_ConfigureCallback_Test::TestBody() components/exo/shell_surface_unittest.cc:509 #14 0x2d62262 in testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc #15 0x2d641c4 in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2667:11 #16 0x2d65596 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2785:28 #17 0x2d8ac96 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:5047:43 #18 0x2d89ee5 in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc #19 0x300e16a in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2329:46 #20 0x300e16a in base::TestSuite::Run() base/test/test_suite.cc:275 #21 0x301334b in Run base/callback.h:96:12 #22 0x301334b in base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, bool, base::OnceCallback<void ()>) base/test/launcher/unit_test_launcher.cc:225 #23 0x30143a1 in base::LaunchUnitTestsSerially(int, char**, base::OnceCallback<int ()>) base/test/launcher/unit_test_launcher.cc:585:10 #24 0x51f39c in main components/exo/test/run_all_unittests.cc:20:10 #25 0x7f2afdbd8f44 in __libc_start_main /build/eglibc-ripdx6/eglibc-2.19/csu/libc-start.c:287 Address 0x7f2afa70e120 is located in stack of thread T0 at offset 288 in frame #0 0x6b8aef in exo::(anonymous namespace)::ShellSurfaceTest_ConfigureCallback_Test::TestBody() components/exo/shell_surface_unittest.cc:460 This frame has 49 object(s): [32, 56) 'ref.tmp.i339' [96, 120) 'ref.tmp1.i340' [160, 184) 'ref.tmp.i' [224, 248) 'ref.tmp1.i' [288, 296) 'suggested_size' (line 464) <== Memory access at offset 288 is inside this variable [320, 324) 'has_state_type' (line 465) [336, 337) 'is_resizing' (line 467) [352, 353) 'is_active' (line 468) [368, 376) 'ref.tmp' (line 469) [400, 416) 'gtest_ar' (line 476) [432, 440) 'ref.tmp18' (line 476) [464, 472) 'ref.tmp20' (line 476) [496, 504) 'temp.lvalue' [528, 544) 'gtest_ar24' (line 480) [560, 564) 'ref.tmp25' (line 480) [576, 580) 'ref.tmp29' (line 480) [592, 600) 'ref.tmp34' (line 480) [624, 632) 'temp.lvalue35' [656, 672) 'gtest_ar38' (line 481) [688, 692) 'ref.tmp39' (line 481) [704, 712) 'ref.tmp43' (line 481) [736, 744) 'temp.lvalue44' [768, 784) 'gtest_ar51' (line 487) [800, 824) 'ref.tmp52' (line 487) [864, 888) 'ref.tmp56' (line 487) [928, 936) 'ref.tmp60' (line 487) [960, 968) 'temp.lvalue61' [992, 1008) 'gtest_ar64' (line 489) [1024, 1028) 'ref.tmp65' (line 489) [1040, 1048) 'ref.tmp69' (line 489) [1072, 1080) 'temp.lvalue70' [1104, 1112) 'buffer_size' (line 493) [1136, 1144) 'agg.tmp' [1168, 1184) 'gtest_ar_' (line 500) [1200, 1208) 'ref.tmp86' (line 500) [1232, 1240) 'temp.lvalue87' [1264, 1288) 'ref.tmp88' (line 500) [1328, 1344) 'gtest_ar_96' (line 503) [1360, 1368) 'ref.tmp101' (line 503) [1392, 1400) 'temp.lvalue102' [1424, 1448) 'ref.tmp103' (line 503) [1488, 1504) 'gtest_ar_106' (line 505) [1520, 1528) 'ref.tmp114' (line 505) [1552, 1560) 'temp.lvalue115' [1584, 1608) 'ref.tmp116' (line 505) [1648, 1664) 'gtest_ar_121' (line 508) [1680, 1688) 'ref.tmp125' (line 508) [1712, 1720) 'temp.lvalue126' [1744, 1768) 'ref.tmp127' (line 508) It looks like ~Surface / ~ShellSurface run a callback from their dtor that then goes and references already-destroyed things. (not sure which component exo stuff goes in)
,
Jun 22 2018
TEST_F(ShellSurfaceTest, ConfigureCallback) in components/exo/shell_surface_unittest.cc needs the same type of fix. I'll make a CL.
,
Jun 22 2018
,
Jun 22 2018
,
Jun 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d156786a350fbc93b184acf56211fad5c146472f commit d156786a350fbc93b184acf56211fad5c146472f Author: Nico Weber <thakis@chromium.org> Date: Fri Jun 22 16:41:48 2018 Fix stack-use-after-free bugs in exo_unittests. Bug: 855573 Change-Id: Ia62bf4d471589b6eb3916277746af4fd7e388ec1 Reviewed-on: https://chromium-review.googlesource.com/1111614 Reviewed-by: David Reveman <reveman@chromium.org> Commit-Queue: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#569656} [modify] https://crrev.com/d156786a350fbc93b184acf56211fad5c146472f/components/exo/shell_surface_unittest.cc [modify] https://crrev.com/d156786a350fbc93b184acf56211fad5c146472f/components/exo/surface_unittest.cc
,
Jun 22 2018
Hopefully better now.
,
Jun 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/65338c09a5e9cbf8eae281d52eefc14182e58a8a commit 65338c09a5e9cbf8eae281d52eefc14182e58a8a Author: Nico Weber <thakis@chromium.org> Date: Fri Jun 22 19:13:40 2018 Enable more tests on the cros memory bots. The motivation is to remove chromium_memory_chromiumos_asan_gtests in favor of linux_chromeos_rel_gtests, but when I tried doing that directly [1], I got many asan/lsan and msan reports for these tests. Some of them pass however, so I'd like to lock in the working ones while I work on fixing the broken ones. 1: https://chromium-review.googlesource.com/c/chromium/src/+/1110703 Adds to "Linux ChromiumOS MSan Tests" and "Linux Chromium OS ASan LSan Tests (1)" these tests: - chromevox_tests - gl_unittests_ozone (to asan/lsan only) - ozone_gl_unittests - ozone_x11_unittests - select_to_speak_extension_tests - views_mus_interactive_ui_tests Bug: 843511 Bug: 855573 , 855580 , 855588 Bug: 855584 , 855585 , 855583 Change-Id: I9402cd43d1d0ef95432eba8231ae79c936b4848b Reviewed-on: https://chromium-review.googlesource.com/1112037 Reviewed-by: Kenneth Russell <kbr@chromium.org> Reviewed-by: John Budorick <jbudorick@chromium.org> Cr-Commit-Position: refs/heads/master@{#569725} [modify] https://crrev.com/65338c09a5e9cbf8eae281d52eefc14182e58a8a/testing/buildbot/chromium.memory.json [modify] https://crrev.com/65338c09a5e9cbf8eae281d52eefc14182e58a8a/testing/buildbot/test_suite_exceptions.pyl [modify] https://crrev.com/65338c09a5e9cbf8eae281d52eefc14182e58a8a/testing/buildbot/test_suites.pyl
,
Jun 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a6d3592a52a71c7ac1ecefa1412891f31592da91 commit a6d3592a52a71c7ac1ecefa1412891f31592da91 Author: Nico Weber <thakis@chromium.org> Date: Sat Jun 23 15:37:33 2018 Run exo_unittests on the cros memory bots, it should pass now. Bug: 855573 Change-Id: If84f612e1500d9189f2206a4ff106fccfdc308d5 Reviewed-on: https://chromium-review.googlesource.com/1112468 Reviewed-by: Kenneth Russell <kbr@chromium.org> Commit-Queue: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#569877} [modify] https://crrev.com/a6d3592a52a71c7ac1ecefa1412891f31592da91/testing/buildbot/chromium.memory.json [modify] https://crrev.com/a6d3592a52a71c7ac1ecefa1412891f31592da91/testing/buildbot/test_suites.pyl |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by thakis@chromium.org
, Jun 22 2018I think the first one has an easy fix: In components/exo/surface_unittest.cc, TEST_P(SurfaceTest, RequestFrameCallback) { std::unique_ptr<Surface> surface(new Surface); base::TimeTicks frame_time; surface->RequestFrameCallback( base::Bind(&SetFrameTime, base::Unretained(&frame_time))); surface->Commit(); // Callback should not run synchronously. EXPECT_TRUE(frame_time.is_null()); } should start with TEST_P(SurfaceTest, RequestFrameCallback) { base::TimeTicks frame_time; std::unique_ptr<Surface> surface(new Surface); instead so that frame_time outlives surface.