New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 616268 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

ProcessMemoryDumpTest.TakeAllDumpsFrom started failing on ClangToTLinuxUBSanVptr tester

Project Member Reported by thakis@chromium.org, May 31 2016

Issue description

started here: https://build.chromium.org/p/chromium.fyi/builders/ClangToTLinuxUBSanVptr%20tester/builds/520

https://build.chromium.org/p/chromium.fyi/builders/ClangToTLinuxUBSanVptr%20tester/builds/520/steps/base_unittests%20on%20Ubuntu-12.04/logs/ProcessMemoryDumpTest.TakeAllDumpsFrom

ProcessMemoryDumpTest.TakeAllDumpsFrom (run #1):
[ RUN      ] ProcessMemoryDumpTest.TakeAllDumpsFrom
Received signal 11 SEGV_MAPERR 000000000000
#0 0x0000018c37e8 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#1 0x7f25e9b15cb0 <unknown>
#2 0x000001a3f9ea base::trace_event::internal::HeapDumpWriter::AddEntryForBucket()
#3 0x000001a3f912 base::trace_event::internal::HeapDumpWriter::Summarize()
#4 0x000001a40859 base::trace_event::ExportHeapDump()
#5 0x000001a576c1 base::trace_event::ProcessMemoryDump::DumpHeapUsage()
#6 0x0000016fb644 base::trace_event::ProcessMemoryDumpTest_TakeAllDumpsFrom_Test::TestBody()
#7 0x000001b9de1f testing::Test::Run()
#8 0x000001ba04fe testing::TestInfo::Run()
#9 0x000001ba22a3 testing::TestCase::Run()
#10 0x000001bbbb58 testing::internal::UnitTestImpl::RunAllTests()
#11 0x000001bbad87 testing::internal::HandleExceptionsInMethodIfSupported<>()
#12 0x000001bba858 testing::UnitTest::Run()
#13 0x000001b274ad base::TestSuite::Run()
#14 0x000001b09139 base::(anonymous namespace)::LaunchUnitTestsInternal()
#15 0x000001b08f4e base::LaunchUnitTests()
#16 0x000001b02c56 main
#17 0x7f25e95537ed __libc_start_main
#18 0x00000055923d <unknown>
  r8: 0000000008000000  r9: 00002320f47fd0b0 r10: 0000000000000009 r11: 000000008f1bbcdc
 r12: 00007fff9422d340 r13: 0000000000000000 r14: 00007fff9422d3c8 r15: 00002320f47eaa80
  di: 9ddfea08eb382d69  si: 00002320f47eaa80  bp: 00007fff9422d310  bx: 00007fff9422d3c8
  dx: 0000000001a3fb20  ax: 00002320f485ca20  cx: 0000000001a3fb20  sp: 00007fff9422d2c0
  ip: 0000000001a3f9ea efl: 0000000000010202 cgf: 0000000000000033 erf: 0000000000000004
 trp: 000000000000000e msk: 0000000000000000 cr2: 0000000000000000
[end of stack trace]


No obvious culprits on the chrome side.
 

Comment 1 by krasin@chromium.org, May 31 2016

Cc: p...@chromium.org
Owner: krasin@chromium.org
Status: Assigned (was: Untriaged)
Taking this one. Will try to reproduce with better diagnostics (http://reviews.llvm.org/D19750)

Comment 2 by krasin@chromium.org, May 31 2016

I reproduced this locally, but no meaningful diagnostics so far.
It seems the be a genuine regression in LLVM, but unfortunately ClangToTLinuxUBSanVptr didn't run for 3 days, so the commit range is very large.

Bisecting...

Comment 3 by krasin@chromium.org, May 31 2016

Huh. I build the test r271040 (succeeded here: https://build.chromium.org/p/chromium.fyi/builders/ClangToTLinuxUBSanVptr%20tester/builds/519/), and the test is broken. Which means that regression is on Chromium side.

Starting bisection on Chromium side...
Cc: bashi@chromium.org
The culprit is identified:

commit fa5b57e219d183c0e53a2c84f1642c3ca07a8902
Author: bashi <bashi@chromium.org>
Date:   Sun May 29 18:51:57 2016 -0700

    Remove ProcessMemoryDump::AddHeapDump()
    
    BUG= 605822 
    
    Review-Url: https://codereview.chromium.org/2016613002
    Cr-Commit-Position: refs/heads/master@{#396666}

Taking a closer look at the CL.
It also does not require a ToT toolchain to reproduce. Had I made UBSanVptr Linux green, we would spot it quicker:
https://build.chromium.org/p/chromium.fyi/builders/UBSanVptr%20Linux/builds/379

So, the steps to reproduce:

1. Sync to master
2. gn gen //out/gn-vptr '--args=is_ubsan_vptr=true is_ubsan_no_recover=true is_debug=false is_component_build=false symbol_level=1 dcheck_always_on=true' --check
3. ninja -C out/gn-vptr base_unittests
4. $ ./out/gn-vptr/base_unittests --gtest_filter=ProcessMemoryDumpTest.TakeAllDumpsFrom                                                                                                    
IMPORTANT DEBUGGING NOTE: batches of tests are run inside their
own process. For debugging a test inside a debugger, use the
--gtest_filter=<your_test_name> flag along with
--single-process-tests.
Using sharding settings from environment. This is shard 0/1
Using 1 parallel jobs.
Note: Google Test filter = ProcessMemoryDumpTest.TakeAllDumpsFrom
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ProcessMemoryDumpTest
[ RUN      ] ProcessMemoryDumpTest.TakeAllDumpsFrom
Received signal 11 SEGV_MAPERR 000000000000
#0 0x0000012f3416 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#1 0x7eff159c4330 <unknown>
#2 0x0000013cb4a1 base::trace_event::internal::HeapDumpWriter::AddEntryForBucket()
#3 0x0000013cb350 base::trace_event::internal::HeapDumpWriter::Summarize()
#4 0x0000013cbc64 base::trace_event::ExportHeapDump()
#5 0x0000013d9353 base::trace_event::ProcessMemoryDump::DumpHeapUsage()
#6 0x000000d78ab9 base::trace_event::ProcessMemoryDumpTest_TakeAllDumpsFrom_Test::TestBody()
#7 0x00000143c617 testing::Test::Run()
#8 0x00000143d9ee testing::TestInfo::Run()
#9 0x00000143e932 testing::TestCase::Run()
#10 0x00000144ba38 testing::internal::UnitTestImpl::RunAllTests()
#11 0x00000144ae09 testing::internal::HandleExceptionsInMethodIfSupported<>()
#12 0x00000144abfd testing::UnitTest::Run()
#13 0x000000f3fe20 base::TestSuite::Run()
#14 0x000000f5a5a5 base::(anonymous namespace)::LaunchUnitTestsInternal()
#15 0x000000f5a459 base::LaunchUnitTests()
#16 0x000000f2f113 main
#17 0x7eff153faf45 __libc_start_main
#18 0x0000004d815d <unknown>
  r8: 6cbd84a04f7ea230  r9: 00001103e4e4af80 r10: 00000000d2e160ef r11: 00000000338ed214
 r12: 00007ffd2d3d3a30 r13: 0000000000000000 r14: 9ddfea08eb382d69 r15: 00007ffd2d3d3ab0
  di: 00007ffd2d3d3ab0  si: 00001103e4e22a80  bp: 00007ffd2d3d36f0  bx: 00001103e4e22a80
  dx: 0000000000000000  ax: 0000000000000000  cx: 0000000000000003  sp: 00007ffd2d3d3390
  ip: 00000000013cb4a1 efl: 0000000000010202 cgf: 0000000000000033 erf: 0000000000000004
 trp: 000000000000000e msk: 0000000000000000 cr2: 0000000000000000
[end of stack trace]
[0531/173600:ERROR:kill_posix.cc(82)] Unable to terminate process group 96907: No such process
[1/1] ProcessMemoryDumpTest.TakeAllDumpsFrom (CRASHED)
1 test crashed:
    ProcessMemoryDumpTest.TakeAllDumpsFrom (../../base/trace_event/process_memory_dump_unittest.cc:81)
Tests took 0 seconds.
Symbolized stack trace with line numbers:

#0  0x00000000013cb4a1 in AddEntryForBucket () at ../../base/trace_event/heap_profiler_heap_dump_writer.cc:205
#1  0x00000000013cb350 in Summarize () at ../../base/trace_event/heap_profiler_heap_dump_writer.cc:259
#2  0x00000000013cbc64 in ExportHeapDump () at ../../base/trace_event/heap_profiler_heap_dump_writer.cc:319
#3  0x00000000013d9353 in DumpHeapUsage () at ../../base/trace_event/process_memory_dump.cc:229
#4  0x0000000000d78ab9 in TestBody () at ../../base/trace_event/process_memory_dump_unittest.cc:92
#5  0x000000000143c617 in testing::Test::Run() () at ../../testing/gtest/src/gtest.cc:2474
#6  0x000000000143d9ee in Run () at ../../testing/gtest/src/gtest.cc:2656
#7  0x000000000143e932 in Run () at ../../testing/gtest/src/gtest.cc:2774
#8  0x000000000144ba38 in RunAllTests () at ../../testing/gtest/src/gtest.cc:4647
#9  0x000000000144ae09 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> () at ../../testing/gtest/src/gtest.cc:2458
#10 0x000000000144abfd in Run () at ../../testing/gtest/src/gtest.cc:4255
#11 0x0000000000f3fe20 in RUN_ALL_TESTS () at ../../testing/gtest/include/gtest/gtest.h:2237
#12 Run () at ../../base/test/test_suite.cc:230
#13 0x0000000000f5a5a5 in Run () at ../../base/callback.h:397
#14 LaunchUnitTestsInternal () at ../../base/test/launcher/unit_test_launcher.cc:206
#15 0x0000000000f5a459 in LaunchUnitTests () at ../../base/test/launcher/unit_test_launcher.cc:445
#16 0x0000000000f2f113 in main () at ../../base/test/run_all_unittests.cc:21

which points to
https://code.google.com/p/chromium/codesearch#chromium/src/base/trace_event/process_memory_dump_unittest.cc&q=process_memory_dump_unittest.cc:92&sq=package:chromium&l=92

pmd1->DumpHeapUsage(metrics_by_context, overhead, "pmd1/heap_dump1");

on the high level, and to
https://code.google.com/p/chromium/codesearch#chromium/src/base/trace_event/heap_profiler_heap_dump_writer.cc&sq=package:chromium&l=205&rcl=1464722289

entry.stack_frame_id = stack_frame_deduplicator_->Insert(backtrace_begin, backtrace_end);

on the low level. I've inserted a debug printf right before the line:

fprintf(stderr, "stack_frame_deduplicator: %p\n", stack_frame_deduplicator_);

Not surprisingly, it gives us: "stack_frame_deduplicator: (nil)".

Kenichi, can you please take a look at what happens there and possibly fix it?
Note that in the regular mode the test passes despite the pointer being null:

gn gen //out/gn '--args=is_debug=false' --check
ninja -C out/gn base_unittests
./out/gn/base_unittests  --gtest_filter=ProcessMemoryDumpTest.TakeAllDumpsFrom                                                                                                         
IMPORTANT DEBUGGING NOTE: batches of tests are run inside their
own process. For debugging a test inside a debugger, use the
--gtest_filter=<your_test_name> flag along with
--single-process-tests.
Using sharding settings from environment. This is shard 0/1
Using 1 parallel jobs.
Note: Google Test filter = ProcessMemoryDumpTest.TakeAllDumpsFrom
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ProcessMemoryDumpTest
[ RUN      ] ProcessMemoryDumpTest.TakeAllDumpsFrom
stack_frame_deduplicator: (nil)
stack_frame_deduplicator: (nil)
stack_frame_deduplicator: (nil)
stack_frame_deduplicator: (nil)
[       OK ] ProcessMemoryDumpTest.TakeAllDumpsFrom (0 ms)
[----------] 1 test from ProcessMemoryDumpTest (0 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (0 ms total)
[  PASSED  ] 1 test.
[1/1] ProcessMemoryDumpTest.TakeAllDumpsFrom (0 ms)
SUCCESS: all tests passed.

So the issue is not specific to UBSan. It's just UBSan buildbot being more strict than others.
Labels: -Pri-2 Pri-1
Owner: bashi@chromium.org
Kenichi,

reassigning the bug to you, as the fix would likely require your knowledge of the code.

feel free to reassign back to me, if that does not work for you.
Kenichi,

friendly ping. This bug keeps UBSanVptr ToT bot red:
https://build.chromium.org/p/chromium.fyi/builders/ClangToTLinuxUBSanVptr%20tester

And the bot being red limits our ability to roll a new Clang toolchain.
Uploaded a CL to fix this. Thanks for finding the bug!
https://codereview.chromium.org/2032803002

Project Member

Comment 11 by bugdroid1@chromium.org, Jun 2 2016

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

commit 425e3fe4d120e37f6c45e1b38f62cb2f1048a4ff
Author: bashi <bashi@chromium.org>
Date: Thu Jun 02 20:51:59 2016

Set Deduplicators in ProcessMemoryDumpTest.TakeAllDumpsFrom

We need to set them to avoid null pointer dereference.

BUG= 616268 

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

[modify] https://crrev.com/425e3fe4d120e37f6c45e1b38f62cb2f1048a4ff/base/trace_event/process_memory_dump_unittest.cc

Status: Fixed (was: Assigned)
Thank you for fixing it!
Project Member

Comment 14 by bugdroid1@chromium.org, Jun 2 2016

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

commit 2ec1834c98f0fd1caf86983f31d0d1a122e0ad1e
Author: krasin <krasin@google.com>
Date: Thu Jun 02 23:10:28 2016

Add GN configs for -fsanitizer=null.

This allows to sanitize null pointer accesses and
could increase readability of the messages from UBSan Vptr bot
(once enabled there).

BUG= 616268 , 616870 

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

[modify] https://crrev.com/2ec1834c98f0fd1caf86983f31d0d1a122e0ad1e/build/config/BUILD.gn
[modify] https://crrev.com/2ec1834c98f0fd1caf86983f31d0d1a122e0ad1e/build/config/sanitizers/BUILD.gn
[modify] https://crrev.com/2ec1834c98f0fd1caf86983f31d0d1a122e0ad1e/build/config/sanitizers/sanitizers.gni
[modify] https://crrev.com/2ec1834c98f0fd1caf86983f31d0d1a122e0ad1e/build/toolchain/gcc_toolchain.gni

I've committed a change that should make the reason for the test above to fail much easier to understand. Instead of a segfault, like we got in the original report, it would tell something like:

../../base/trace_event/heap_profiler_heap_dump_writer.cc:205:53: runtime error: member call on null pointer of type 'base::trace_event::StackFrameDeduplicator'
    #0 0x14273bd in base::trace_event::internal::HeapDumpWriter::AddEntryForBucket(base::trace_event::internal::(anonymous namespace)::Bucket const&) out/gn-vptr/../../base/trace_event/heap_profiler_heap_dump_writer.cc:205:53
    #1 0x14270d4 in base::trace_event::internal::HeapDumpWriter::Summarize(std::unordered_map<base::trace_event::AllocationContext, base::trace_event::AllocationMetrics, base_hash::hash<base::trace_event::AllocationContext>, std::equal_to<base::trace_event::AllocationContext>, std::allocator<std::pair<base::trace_event::AllocationContext const, base::trace_event::AllocationMetrics> > > const&) out/gn-vptr/../../base/trace_event/heap_profiler_heap_dump_writer.cc:259:3
    #2 0x1427be3 in base::trace_event::ExportHeapDump(std::unordered_map<base::trace_event::AllocationContext, base::trace_event::AllocationMetrics, base_hash::hash<base::trace_event::AllocationContext>, std::equal_to<base::trace_event::AllocationContext>, std::allocator<std::pair<base::trace_event::AllocationContext const, base::trace_event::AllocationMetrics> > > const&, base::trace_event::MemoryDumpSessionState const&) out/gn-vptr/../../base/trace_event/heap_profiler_heap_dump_writer.cc:319:27
    #3 0x1436383 in base::trace_event::ProcessMemoryDump::DumpHeapUsage(std::unordered_map<base::trace_event::AllocationContext, base::trace_event::AllocationMetrics, base_hash::hash<base::trace_event::AllocationContext>, std::equal_to<base::trace_event::AllocationContext>, std::allocator<std::pair<base::trace_event::AllocationContext const, base::trace_event::AllocationMetrics> > > const&, base::trace_event::TraceEventMemoryOverhead&, char const*) out/gn-vptr/../../base/trace_event/process_memory_dump.cc:229:46
    #4 0xd97cbc in base::trace_event::ProcessMemoryDumpTest_TakeAllDumpsFrom_Test::TestBody() out/gn-vptr/../../base/trace_event/process_memory_dump_unittest.cc:92:9

Sign in to add a comment