LeakSanitizer detected memory leaks in NGInlineLayoutAlgorithmTest.VerticalAlignBottomReplaced and LayoutTableCellTest.AbsoluteColumnIndex |
||||||||
Issue descriptionFirst observed in https://build.chromium.org/p/chromium.fyi/builders/ClangToTLinuxASan%20tester/builds/2058 Logs: https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.fyi%2FClangToTLinuxASan_tester%2F2058%2F%2B%2Frecipes%2Fsteps%2Fwebkit_unit_tests%2F0%2Fstdout [ RUN ] NGInlineLayoutAlgorithmTest.VerticalAlignBottomReplaced [ OK ] NGInlineLayoutAlgorithmTest.VerticalAlignBottomReplaced (99 ms) [----------] 1 test from NGInlineLayoutAlgorithmTest (99 ms total) [----------] Global test environment tear-down [==========] 1 test from 1 test case ran. (100 ms total) [ PASSED ] 1 test. ================================================================= ==19090==ERROR: LeakSanitizer: detected memory leaks Direct leak of 64 byte(s) in 1 object(s) allocated from: #0 0x698096 in __interceptor_malloc /b/c/builder/ClangToTLinuxASan/src/third_party/llvm/compiler-rt/lib/asan/asan_malloc_linux.cc:66 #1 0xae6c20a in PartitionAlloc base/allocator/partition_allocator/partition_alloc.h:703:18 #2 0xae6c20a in blink::InlineBox::operator new(unsigned long) third_party/WebKit/Source/core/layout/line/InlineBox.cpp:81 #3 0xabd7c26 in blink::LayoutBox::CreateInlineBox() third_party/WebKit/Source/core/layout/LayoutBox.cpp:2125:10 #4 0xab9485a in CreateInlineBox third_party/WebKit/Source/core/layout/api/LineLayoutBox.h:95:50 #5 0xab9485a in CreateInlineBoxForLayoutObject third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp:134 #6 0xab9485a in blink::LayoutBlockFlow::ConstructLine(blink::BidiRunList<blink::BidiRun>&, blink::LineInfo const&) third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp:297 #7 0xaee7087 in blink::NGInlineNode::CopyFragmentDataToLayoutBox(blink::NGConstraintSpace const&, blink::NGLayoutResult*) third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node.cc:291:21 #8 0xaee5d5a in blink::NGInlineNode::Layout(blink::NGConstraintSpace*, blink::NGBreakToken*) third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node.cc:203:3 #9 0x1983093 in blink::(anonymous namespace)::NGInlineLayoutAlgorithmTest_VerticalAlignBottomReplaced_Test::TestBody() third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm_test.cc:97:55 #10 0x48da9f3 in HandleExceptionsInMethodIfSupported<testing::Test, void> third_party/googletest/src/googletest/src/gtest.cc:2458:12 #11 0x48da9f3 in testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc:2474 #12 0x48dbe29 in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2656:11 #13 0x48dcdfe in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2774:28 #14 0x48ea339 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:4647:43 #15 0x48e993a in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> third_party/googletest/src/googletest/src/gtest.cc:2458:12 #16 0x48e993a in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4255 #17 0x67f35a3 in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2237:46 #18 0x67f35a3 in base::TestSuite::Run() base/test/test_suite.cc:271 #19 0x8e037a in (anonymous namespace)::runHelper(base::TestSuite*) third_party/WebKit/Source/web/tests/RunAllTests.cpp:48:27 #20 0x67f6eb9 in Run base/callback.h:80:12 #21 0x67f6eb9 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&, int, int, bool, base::Callback<void (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) base/test/launcher/unit_test_launcher.cc:211 #22 0x67f6ad4 in base::LaunchUnitTests(int, char**, base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) base/test/launcher/unit_test_launcher.cc:453:10 #23 0x8e01ca in main third_party/WebKit/Source/web/tests/RunAllTests.cpp:70:10 #24 0x7f6b7fa9df44 in __libc_start_main /build/eglibc-MjiXCM/eglibc-2.19/csu/libc-start.c:287 ----------------------------------------------------- Suppressions used: count bytes template 2 288 libfontconfig ----------------------------------------------------- SUMMARY: AddressSanitizer: 64 byte(s) leaked in 1 allocation(s). [ RUN ] LayoutTableCellTest.AbsoluteColumnIndex [ OK ] LayoutTableCellTest.AbsoluteColumnIndex (24 ms) [----------] 1 test from LayoutTableCellTest (24 ms total) [----------] Global test environment tear-down [==========] 1 test from 1 test case ran. (25 ms total) [ PASSED ] 1 test. ================================================================= ==32255==ERROR: LeakSanitizer: detected memory leaks Direct leak of 272 byte(s) in 1 object(s) allocated from: #0 0x698096 in __interceptor_malloc /b/c/builder/ClangToTLinuxASan/src/third_party/llvm/compiler-rt/lib/asan/asan_malloc_linux.cc:66 #1 0xacfdcc4 in PartitionAlloc base/allocator/partition_allocator/partition_alloc.h:703:18 #2 0xacfdcc4 in blink::LayoutObject::operator new(unsigned long) third_party/WebKit/Source/core/layout/LayoutObject.cpp:140 #3 0xad7309e in blink::LayoutTableCell::CreateAnonymous(blink::Document*) third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:1414:36 #4 0x17dd6da in blink::LayoutTableCellTest_AbsoluteColumnIndex_Test::TestBody() third_party/WebKit/Source/core/layout/LayoutTableCellTest.cpp:92:16 #5 0x48da9f3 in HandleExceptionsInMethodIfSupported<testing::Test, void> third_party/googletest/src/googletest/src/gtest.cc:2458:12 #6 0x48da9f3 in testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc:2474 #7 0x48dbe29 in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2656:11 #8 0x48dcdfe in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2774:28 #9 0x48ea339 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:4647:43 #10 0x48e993a in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> third_party/googletest/src/googletest/src/gtest.cc:2458:12 #11 0x48e993a in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4255 #12 0x67f35a3 in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2237:46 #13 0x67f35a3 in base::TestSuite::Run() base/test/test_suite.cc:271 #14 0x8e037a in (anonymous namespace)::runHelper(base::TestSuite*) third_party/WebKit/Source/web/tests/RunAllTests.cpp:48:27 #15 0x67f6eb9 in Run base/callback.h:80:12 #16 0x67f6eb9 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&, int, int, bool, base::Callback<void (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) base/test/launcher/unit_test_launcher.cc:211 #17 0x67f6ad4 in base::LaunchUnitTests(int, char**, base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) base/test/launcher/unit_test_launcher.cc:453:10 #18 0x8e01ca in main third_party/WebKit/Source/web/tests/RunAllTests.cpp:70:10 #19 0x7f1d7d97ef44 in __libc_start_main /build/eglibc-MjiXCM/eglibc-2.19/csu/libc-start.c:287 SUMMARY: AddressSanitizer: 272 byte(s) leaked in 1 allocation(s).
,
May 15 2017
The other test is also pretty new, +kojii Can you each take a look and check if the leak in your test looks real? This is a bot using the latest version of asan, so maybe it's a bug in asan, or the newer asan finds an issue the current one doesn't. (https://chromium.googlesource.com/chromium/src/+/c731464f86c6a5c5c3f2b64890e04b38750896fc and https://chromium.googlesource.com/chromium/src/+/c8a96c63d6eb03dbc39afc03fd22e7f19f36569a)
,
May 15 2017
I can reproduce this (I only tried the first test) with pinned Clang too, so it's not anything that changed in ASan.
,
May 15 2017
Bisect shows that https://codereview.chromium.org/2846563002 is the first revision where this leak occurs. wangxianzhu, I'll assign the bug to you as you are the author of that change.
,
May 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8ba6e31c50feaa168f08fc02fd2e467d3f64836d commit 8ba6e31c50feaa168f08fc02fd2e467d3f64836d Author: wangxianzhu <wangxianzhu@chromium.org> Date: Tue May 16 01:22:10 2017 Remove a duplicate and leaking test LayoutTableCellTest.AbsoluteColumnIndex The test leaks the cell. Remove it instead of fixing it because there have been other tests (LayoutTableCellDeathTest.*) testing the same thing. BUG= 721932 Review-Url: https://codereview.chromium.org/2884733003 Cr-Commit-Position: refs/heads/master@{#471970} [modify] https://crrev.com/8ba6e31c50feaa168f08fc02fd2e467d3f64836d/third_party/WebKit/Source/core/layout/LayoutTableCellTest.cpp
,
May 16 2017
The leaking test LayoutTableCellTest.AbsoluteColumnIndex has been removed because it was duplicate with other tests (which don't leak). kojii@ can you take a look at NGInlineLayoutAlgorithmTest.VerticalAlignBottomReplaced?
,
May 16 2017
Will do. Note that the suspected code is temporary, we expect to remove in not too far future, so I'm not putting priority. Probably related with issue 706614 .
,
May 19 2017
Leaving the test broken like this is not acceptable. It is keeping buildbots red. Please fix the test or revert the change that introduced the breakage. I would revert myself, but it seems like other changes have landed on top.
,
May 22 2017
kojii: Ping?
,
May 22 2017
Ok, I'll disable until it's ready.
,
May 22 2017
Can we revert instead? Landing code that leaks and then disabling tests is not healthy for the codebase.
,
May 22 2017
#8: Thank you for the heads up, I wasn't clear how much trouble it was. #11: This is LayoutNG only code, and needed only until we turn on painting for LayoutNG so that we can bootstrap the new layout code without waiting for the new paint code. The stack #7 CopyFragmentDataToLayoutBox will be removed when we turn painting on. I hope your understanding. Also, I'm quite sure the suspected CL is not the root cause, as we've been seeing issue 706614 . The reverting isn't likely to help. It turning on painting takes more than we expect, I'll look into the root cause. WIP: https://codereview.chromium.org/2896873002
,
May 22 2017
I tried reverting, but since the code has been in so long, and in an area that's changing, it seems very hard. https://codereview.chromium.org/2846563002 is where the test start failing. Reverting that causes conflicts with (at least) https://codereview.chromium.org/2861373003 which causes conflicts with (at least) https://codereview.chromium.org/2872423002, and so on.. These are touching layout code and updating layout test expectations and for each dependence the conflicts just get more difficult. As unfortunate as it seems, I think the pragmatic option is to disable this test under asan.
,
May 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cabeb0d9ab1b39fc25a1647d38afade5bea2e70e commit cabeb0d9ab1b39fc25a1647d38afade5bea2e70e Author: Hans Wennborg <hans@chromium.org> Date: Mon May 22 18:47:58 2017 Disable NGInlineLayoutAlgorithmTest.VerticalAlignBottomReplaced under ASan It leaks memory. Disabling until the code is fixed. BUG= 721932 R=kojii@chromium.org, thakis@chromium.org TBR=kojii CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng Review-Url: https://codereview.chromium.org/2896743003 . Cr-Commit-Position: refs/heads/master@{#473639} [modify] https://crrev.com/cabeb0d9ab1b39fc25a1647d38afade5bea2e70e/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm_test.cc
,
May 23 2017
Thank you again, hans@. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by thakis@chromium.org
, May 15 2017