Implement native allocation tracing |
|||
Issue descriptionThis is tracking issue for implementing "Native stack traces in memory-infra heap profiler" as outlined in https://goo.gl/DFoqfi
,
Apr 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/57a5db32caf032ebc01b6a1b215f038d6b74aedb commit 57a5db32caf032ebc01b6a1b215f038d6b74aedb Author: catapult-deps-roller <catapult-deps-roller@chromium.org> Date: Wed Apr 20 19:42:09 2016 Roll src/third_party/catapult/ d3578ff8c..5a702cdeb (2 commits). https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/d3578ff8c479..5a702cdeb6e3 $ git log d3578ff8c..5a702cdeb --date=short --no-merges --format='%ad %ae %s' BUG= 602701 TBR=catapult-sheriff@chromium.org Review URL: https://codereview.chromium.org/1902103004 Cr-Commit-Position: refs/heads/master@{#388557} [modify] https://crrev.com/57a5db32caf032ebc01b6a1b215f038d6b74aedb/DEPS
,
Apr 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7e308e6f283b287ad0e2229e8d8adb865a7542b6 commit 7e308e6f283b287ad0e2229e8d8adb865a7542b6 Author: dskiba <dskiba@google.com> Date: Wed Apr 20 19:59:59 2016 Revert of [tracing] Turn StackFrame into struct. (patchset #5 id:80001 of https://codereview.chromium.org/1891543003/ ) Reason for revert: Broke build on Windows x64 (warning was treated as error): heap_profiler_allocation_context.cc(58): warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss of data Original issue's description: > [tracing] Turn StackFrame into struct. > > This change turns StackFrame (aka const char*) into a struct and > introduces 'type' field which controls how stack frame is formatted > when it's written to trace file. As an example, thread name, which > previously was just a string like any other function name, is now > formatted as '[Thread: %s]'. > > More stack frame types will be added in the future, for example > native allocation tracing will add 'program counter' type. > > BUG= 602701 TBR=primiano@chromium.org,ssid@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 602701 Review URL: https://codereview.chromium.org/1907593002 Cr-Commit-Position: refs/heads/master@{#388558} [modify] https://crrev.com/7e308e6f283b287ad0e2229e8d8adb865a7542b6/base/trace_event/heap_profiler_allocation_context.cc [modify] https://crrev.com/7e308e6f283b287ad0e2229e8d8adb865a7542b6/base/trace_event/heap_profiler_allocation_context.h [modify] https://crrev.com/7e308e6f283b287ad0e2229e8d8adb865a7542b6/base/trace_event/heap_profiler_allocation_context_tracker.cc [modify] https://crrev.com/7e308e6f283b287ad0e2229e8d8adb865a7542b6/base/trace_event/heap_profiler_allocation_context_tracker.h [modify] https://crrev.com/7e308e6f283b287ad0e2229e8d8adb865a7542b6/base/trace_event/heap_profiler_allocation_context_tracker_unittest.cc [modify] https://crrev.com/7e308e6f283b287ad0e2229e8d8adb865a7542b6/base/trace_event/heap_profiler_allocation_register_unittest.cc [modify] https://crrev.com/7e308e6f283b287ad0e2229e8d8adb865a7542b6/base/trace_event/heap_profiler_heap_dump_writer.cc [modify] https://crrev.com/7e308e6f283b287ad0e2229e8d8adb865a7542b6/base/trace_event/heap_profiler_heap_dump_writer_unittest.cc [modify] https://crrev.com/7e308e6f283b287ad0e2229e8d8adb865a7542b6/base/trace_event/heap_profiler_stack_frame_deduplicator.cc [modify] https://crrev.com/7e308e6f283b287ad0e2229e8d8adb865a7542b6/base/trace_event/heap_profiler_stack_frame_deduplicator_unittest.cc
,
Apr 21 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c4b751e78f505adc6316bae2d0ab111ba0218dce commit c4b751e78f505adc6316bae2d0ab111ba0218dce Author: dskiba <dskiba@google.com> Date: Thu Apr 21 00:09:59 2016 Reland of [tracing] Turn StackFrame into struct. The original CL (1891543003) was reverted because it broke Windows x64 build: heap_profiler_allocation_context.cc(58): warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss of data This CL adds necessary cast to avoid the warning. Original issue's description: > [tracing] Turn StackFrame into struct. > > This change turns StackFrame (aka const char*) into a struct and > introduces 'type' field which controls how stack frame is formatted > when it's written to trace file. As an example, thread name, which > previously was just a string like any other function name, is now > formatted as '[Thread: %s]'. > > More stack frame types will be added in the future, for example > native allocation tracing will add 'program counter' type. > BUG= 602701 TBR=primiano@chromium.org Review URL: https://codereview.chromium.org/1904823002 Cr-Commit-Position: refs/heads/master@{#388615} [modify] https://crrev.com/c4b751e78f505adc6316bae2d0ab111ba0218dce/base/trace_event/heap_profiler_allocation_context.cc [modify] https://crrev.com/c4b751e78f505adc6316bae2d0ab111ba0218dce/base/trace_event/heap_profiler_allocation_context.h [modify] https://crrev.com/c4b751e78f505adc6316bae2d0ab111ba0218dce/base/trace_event/heap_profiler_allocation_context_tracker.cc [modify] https://crrev.com/c4b751e78f505adc6316bae2d0ab111ba0218dce/base/trace_event/heap_profiler_allocation_context_tracker.h [modify] https://crrev.com/c4b751e78f505adc6316bae2d0ab111ba0218dce/base/trace_event/heap_profiler_allocation_context_tracker_unittest.cc [modify] https://crrev.com/c4b751e78f505adc6316bae2d0ab111ba0218dce/base/trace_event/heap_profiler_allocation_register_unittest.cc [modify] https://crrev.com/c4b751e78f505adc6316bae2d0ab111ba0218dce/base/trace_event/heap_profiler_heap_dump_writer.cc [modify] https://crrev.com/c4b751e78f505adc6316bae2d0ab111ba0218dce/base/trace_event/heap_profiler_heap_dump_writer_unittest.cc [modify] https://crrev.com/c4b751e78f505adc6316bae2d0ab111ba0218dce/base/trace_event/heap_profiler_stack_frame_deduplicator.cc [modify] https://crrev.com/c4b751e78f505adc6316bae2d0ab111ba0218dce/base/trace_event/heap_profiler_stack_frame_deduplicator_unittest.cc
,
Apr 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ead0969fe721b3a71a18abb9c1203c15e1276aa4 commit ead0969fe721b3a71a18abb9c1203c15e1276aa4 Author: dskiba <dskiba@google.com> Date: Fri Apr 22 15:56:11 2016 Add function to trace stack using frame pointers. For memory-infra we need fast stack traces to implement allocation tracing (see https://goo.gl/DFoqfi). StackTrace class uses unwinding and is too slow. This change adds a function that uses frame pointers to walk the stack. The function supports x86, x64 and arm (but not thumb) architectures on POSIX platforms. BUG= 602701 Review URL: https://codereview.chromium.org/1879073002 Cr-Commit-Position: refs/heads/master@{#389123} [modify] https://crrev.com/ead0969fe721b3a71a18abb9c1203c15e1276aa4/base/debug/stack_trace.cc [modify] https://crrev.com/ead0969fe721b3a71a18abb9c1203c15e1276aa4/base/debug/stack_trace.h [modify] https://crrev.com/ead0969fe721b3a71a18abb9c1203c15e1276aa4/base/debug/stack_trace_unittest.cc
,
Apr 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9cf919380c57cb39a2d5b7a24120d575fbe7258a commit 9cf919380c57cb39a2d5b7a24120d575fbe7258a Author: apacible <apacible@chromium.org> Date: Fri Apr 22 17:17:58 2016 Revert of Add function to trace stack using frame pointers. (patchset #8 id:140001 of https://codereview.chromium.org/1879073002/ ) Reason for revert: Failure on Linux ChromeOS MSan Tests: https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20ChromeOS%20MSan%20Tests/builds/9050 See: https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20ChromeOS%20MSan%20Tests/builds/9050/steps/base_unittests%20on%20Ubuntu-12.04/logs/stdio Original issue's description: > Add function to trace stack using frame pointers. > > For memory-infra we need fast stack traces to implement allocation > tracing (see https://goo.gl/DFoqfi). StackTrace class uses unwinding > and is too slow. This change adds a function that uses frame pointers > to walk the stack. > > The function supports x86, x64 and arm (but not thumb) architectures > on POSIX platforms. > > BUG= 602701 TBR=thakis@chromium.org,bcwhite@chromium.org,primiano@chromium.org,dskiba@google.com # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 602701 Review URL: https://codereview.chromium.org/1907373002 Cr-Commit-Position: refs/heads/master@{#389149} [modify] https://crrev.com/9cf919380c57cb39a2d5b7a24120d575fbe7258a/base/debug/stack_trace.cc [modify] https://crrev.com/9cf919380c57cb39a2d5b7a24120d575fbe7258a/base/debug/stack_trace.h [modify] https://crrev.com/9cf919380c57cb39a2d5b7a24120d575fbe7258a/base/debug/stack_trace_unittest.cc
,
Apr 22 2016
MSan is sad on my stack unwinding function, because we are walking the stack, reading stuff from it. So we are both right :)
I'm going to disable test in the presence of MEMORY_SANITIZER symbol.
StackTraceTest.TraceStackFramePointers (run #1):
[ RUN ] StackTraceTest.TraceStackFramePointers
==10701==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x1e7c55e in ?? base/debug/stack_trace.cc:74:11
#1 0x80e623 in ExpectStackFramePointers\u003C1> base/debug/stack_trace_unittest.cc:266:18
#2 0x8157a6 in ExpectStackFramePointers\u003C2> base/debug/stack_trace_unittest.cc:250:3
#3 0x8144d6 in ExpectStackFramePointers\u003C3> base/debug/stack_trace_unittest.cc:250:3
#4 0x813206 in ExpectStackFramePointers\u003C4> base/debug/stack_trace_unittest.cc:250:3
#5 0x80ffe6 in ExpectStackFramePointers\u003C5> base/debug/stack_trace_unittest.cc:250:3
#6 0x80f926 in ?? base/debug/stack_trace_unittest.cc:278:3
#7 0x22b0fa2 in HandleExceptionsInMethodIfSupported\u003Ctesting::Test, void> testing/gtest/src/gtest.cc:2458:12
#8 0x22b0fa2 in Run testing/gtest/src/gtest.cc:2474:0
#9 0x22b3fc9 in Run testing/gtest/src/gtest.cc:2656:5
#10 0x22b57fb in Run testing/gtest/src/gtest.cc:2774:5
#11 0x22d2c01 in RunAllTests testing/gtest/src/gtest.cc:4647:11
#12 0x22d1c01 in HandleExceptionsInMethodIfSupported\u003Ctesting::internal::UnitTestImpl, bool> testing/gtest/src/gtest.cc:2458:12
#13 0x22d1c01 in Run testing/gtest/src/gtest.cc:4255:0
#14 0x21f4374 in RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2237:10
#15 0x21f4374 in Run base/test/test_suite.cc:230:0
#16 0x21c702d in Run base/callback.h:397:12
#17 0x21c702d in LaunchUnitTestsInternal base/test/launcher/unit_test_launcher.cc:206:0
#18 0x21c6897 in LaunchUnitTests base/test/launcher/unit_test_launcher.cc:445:10
#19 0x21be8b6 in main base/test/run_all_unittests.cc:21:10
#20 0x7ff15738176c in __libc_start_main /build/eglibc-rrybNj/eglibc-2.15/csu/libc-start.c:226:0
#21 0x55195c in _start ??:0
Uninitialized value was created by an allocation of '__p.i.i' in the stack frame of function '_ZNKSt3__16locale9use_facetERNS0_2idE'
#0 0x7ff157cd82b0 in use_facet buildtools/third_party/libc++/trunk/src/locale.cpp:593:0
SUMMARY: MemorySanitizer: use-of-uninitialized-value (/tmp/runW4UoY3/out/Release/base_unittests+0x1e7c55e)
Exiting
StackTraceTest.TraceStackFramePointers (run #2):
[ RUN ] StackTraceTest.TraceStackFramePointers
==12287==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x1e7c55e in ?? base/debug/stack_trace.cc:74:11
#1 0x80e623 in ExpectStackFramePointers\u003C1> base/debug/stack_trace_unittest.cc:266:18
#2 0x8157a6 in ExpectStackFramePointers\u003C2> base/debug/stack_trace_unittest.cc:250:3
#3 0x8144d6 in ExpectStackFramePointers\u003C3> base/debug/stack_trace_unittest.cc:250:3
#4 0x813206 in ExpectStackFramePointers\u003C4> base/debug/stack_trace_unittest.cc:250:3
#5 0x80ffe6 in ExpectStackFramePointers\u003C5> base/debug/stack_trace_unittest.cc:250:3
#6 0x80f926 in ?? base/debug/stack_trace_unittest.cc:278:3
#7 0x22b0fa2 in HandleExceptionsInMethodIfSupported\u003Ctesting::Test, void> testing/gtest/src/gtest.cc:2458:12
#8 0x22b0fa2 in Run testing/gtest/src/gtest.cc:2474:0
#9 0x22b3fc9 in Run testing/gtest/src/gtest.cc:2656:5
#10 0x22b57fb in Run testing/gtest/src/gtest.cc:2774:5
#11 0x22d2c01 in RunAllTests testing/gtest/src/gtest.cc:4647:11
#12 0x22d1c01 in HandleExceptionsInMethodIfSupported\u003Ctesting::internal::UnitTestImpl, bool> testing/gtest/src/gtest.cc:2458:12
#13 0x22d1c01 in Run testing/gtest/src/gtest.cc:4255:0
#14 0x21f4374 in RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2237:10
#15 0x21f4374 in Run base/test/test_suite.cc:230:0
#16 0x21c702d in Run base/callback.h:397:12
#17 0x21c702d in LaunchUnitTestsInternal base/test/launcher/unit_test_launcher.cc:206:0
#18 0x21c6897 in LaunchUnitTests base/test/launcher/unit_test_launcher.cc:445:10
#19 0x21be8b6 in main base/test/run_all_unittests.cc:21:10
#20 0x7fc10339e76c in __libc_start_main /build/eglibc-rrybNj/eglibc-2.15/csu/libc-start.c:226:0
#21 0x55195c in _start ??:0
Uninitialized value was created by an allocation of 'temp.lvalue' in the stack frame of function '_ZN4base5debug13ParseProcMapsERKNSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEEPNS1_6vectorINS0_18MappedMemoryRegionENS5_ISB_EEEE'
#0 0x1e7a460 in ParseProcMaps base/debug/proc_maps_linux.cc:95:0
SUMMARY: MemorySanitizer: use-of-uninitialized-value (/tmp/runW4UoY3/out/Release/base_unittests+0x1e7c55e)
Exiting
StackTraceTest.TraceStackFramePointers (run #3):
[ RUN ] StackTraceTest.TraceStackFramePointers
==12288==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x1e7c55e in ?? base/debug/stack_trace.cc:74:11
#1 0x80e623 in ExpectStackFramePointers\u003C1> base/debug/stack_trace_unittest.cc:266:18
#2 0x8157a6 in ExpectStackFramePointers\u003C2> base/debug/stack_trace_unittest.cc:250:3
#3 0x8144d6 in ExpectStackFramePointers\u003C3> base/debug/stack_trace_unittest.cc:250:3
#4 0x813206 in ExpectStackFramePointers\u003C4> base/debug/stack_trace_unittest.cc:250:3
#5 0x80ffe6 in ExpectStackFramePointers\u003C5> base/debug/stack_trace_unittest.cc:250:3
#6 0x80f926 in ?? base/debug/stack_trace_unittest.cc:278:3
#7 0x22b0fa2 in HandleExceptionsInMethodIfSupported\u003Ctesting::Test, void> testing/gtest/src/gtest.cc:2458:12
#8 0x22b0fa2 in Run testing/gtest/src/gtest.cc:2474:0
#9 0x22b3fc9 in Run testing/gtest/src/gtest.cc:2656:5
#10 0x22b57fb in Run testing/gtest/src/gtest.cc:2774:5
#11 0x22d2c01 in RunAllTests testing/gtest/src/gtest.cc:4647:11
#12 0x22d1c01 in HandleExceptionsInMethodIfSupported\u003Ctesting::internal::UnitTestImpl, bool> testing/gtest/src/gtest.cc:2458:12
#13 0x22d1c01 in Run testing/gtest/src/gtest.cc:4255:0
#14 0x21f4374 in RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2237:10
#15 0x21f4374 in Run base/test/test_suite.cc:230:0
#16 0x21c702d in Run base/callback.h:397:12
#17 0x21c702d in LaunchUnitTestsInternal base/test/launcher/unit_test_launcher.cc:206:0
#18 0x21c6897 in LaunchUnitTests base/test/launcher/unit_test_launcher.cc:445:10
#19 0x21be8b6 in main base/test/run_all_unittests.cc:21:10
#20 0x7fa998ddb76c in __libc_start_main /build/eglibc-rrybNj/eglibc-2.15/csu/libc-start.c:226:0
#21 0x55195c in _start ??:0
Uninitialized value was created by an allocation of 'temp.lvalue' in the stack frame of function '_ZN4base5debug13ParseProcMapsERKNSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEEPNS1_6vectorINS0_18MappedMemoryRegionENS5_ISB_EEEE'
#0 0x1e7a460 in ParseProcMaps base/debug/proc_maps_linux.cc:95:0
SUMMARY: MemorySanitizer: use-of-uninitialized-value (/tmp/runW4UoY3/out/Release/base_unittests+0x1e7c55e)
Exiting
StackTraceTest.TraceStackFramePointers (run #4):
[ RUN ] StackTraceTest.TraceStackFramePointers
==12289==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x1e7c55e in ?? base/debug/stack_trace.cc:74:11
#1 0x80e623 in ExpectStackFramePointers\u003C1> base/debug/stack_trace_unittest.cc:266:18
#2 0x8157a6 in ExpectStackFramePointers\u003C2> base/debug/stack_trace_unittest.cc:250:3
#3 0x8144d6 in ExpectStackFramePointers\u003C3> base/debug/stack_trace_unittest.cc:250:3
#4 0x813206 in ExpectStackFramePointers\u003C4> base/debug/stack_trace_unittest.cc:250:3
#5 0x80ffe6 in ExpectStackFramePointers\u003C5> base/debug/stack_trace_unittest.cc:250:3
#6 0x80f926 in ?? base/debug/stack_trace_unittest.cc:278:3
#7 0x22b0fa2 in HandleExceptionsInMethodIfSupported\u003Ctesting::Test, void> testing/gtest/src/gtest.cc:2458:12
#8 0x22b0fa2 in Run testing/gtest/src/gtest.cc:2474:0
#9 0x22b3fc9 in Run testing/gtest/src/gtest.cc:2656:5
#10 0x22b57fb in Run testing/gtest/src/gtest.cc:2774:5
#11 0x22d2c01 in RunAllTests testing/gtest/src/gtest.cc:4647:11
#12 0x22d1c01 in HandleExceptionsInMethodIfSupported\u003Ctesting::internal::UnitTestImpl, bool> testing/gtest/src/gtest.cc:2458:12
#13 0x22d1c01 in Run testing/gtest/src/gtest.cc:4255:0
#14 0x21f4374 in RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2237:10
#15 0x21f4374 in Run base/test/test_suite.cc:230:0
#16 0x21c702d in Run base/callback.h:397:12
#17 0x21c702d in LaunchUnitTestsInternal base/test/launcher/unit_test_launcher.cc:206:0
#18 0x21c6897 in LaunchUnitTests base/test/launcher/unit_test_launcher.cc:445:10
#19 0x21be8b6 in main base/test/run_all_unittests.cc:21:10
#20 0x7f9f3147276c in __libc_start_main /build/eglibc-rrybNj/eglibc-2.15/csu/libc-start.c:226:0
#21 0x55195c in _start ??:0
Uninitialized value was created by an allocation of 'temp.lvalue' in the stack frame of function '_ZN4base5debug13ParseProcMapsERKNSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEEPNS1_6vectorINS0_18MappedMemoryRegionENS5_ISB_EEEE'
#0 0x1e7a460 in ParseProcMaps base/debug/proc_maps_linux.cc:95:0
SUMMARY: MemorySanitizer: use-of-uninitialized-value (/tmp/runW4UoY3/out/Release/base_unittests+0x1e7c55e)
Exiting
,
Apr 23 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/79080da0203790b2e12ab36ab50f8a374aea8455 commit 79080da0203790b2e12ab36ab50f8a374aea8455 Author: dskiba <dskiba@google.com> Date: Sat Apr 23 00:10:27 2016 Reland of Add function to trace stack using frame pointers. Original CL (1879073002) was reverted because it triggered several use-of-uninitialized-value errors on Linux ChromeOS MSan bot. See crbug.com/602701#c6 for details. Those errors are expected because reading values from the stack outside of the function's frame is essential to the process of unwinding. This CL adds change to disable the unit test if MEMORY_SANITIZER is defined. Additionally, this CL fixes unwinding on ARM by moving sp adjustment several lines up. Original issue's description: > For memory-infra we need fast stack traces to implement allocation > tracing (see https://goo.gl/DFoqfi). StackTrace class uses unwinding > and is too slow. This change adds a function that uses frame pointers > to walk the stack. > > The function supports x86, x64 and arm (but not thumb) architectures > on POSIX platforms. BUG= 602701 Review URL: https://codereview.chromium.org/1915593003 Cr-Commit-Position: refs/heads/master@{#389320} [modify] https://crrev.com/79080da0203790b2e12ab36ab50f8a374aea8455/base/debug/stack_trace.cc [modify] https://crrev.com/79080da0203790b2e12ab36ab50f8a374aea8455/base/debug/stack_trace.h [modify] https://crrev.com/79080da0203790b2e12ab36ab50f8a374aea8455/base/debug/stack_trace_unittest.cc
,
Apr 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5a05938adaabf413a40f1e0dfec3d1e4c683693a commit 5a05938adaabf413a40f1e0dfec3d1e4c683693a Author: dskiba <dskiba@google.com> Date: Mon Apr 25 16:47:49 2016 [tracing] Add native allocation tracing mode. This change adds 'native' mode to '--enable-heap-profiling' option. When started with '--enable-heap-profiling=native' Chrome will keep native (real) stack traces for all live allocations. These native stack traces are then dumped to a trace file if 'memory-infra' category is enabled. Stack traces are dumped unsymbolized (for performance reasons), and there is a script (third_party/catapult/tracing/bin/symbolize_trace) that symbolizes all stack traces in a given trace file. Design document: https://goo.gl/DFoqfi Note: native stack tracing relies on frame pointers. You'll need to use either debug or a profiling release build (enable_profiling = true). Note: for now this all works on Linux @ x86/x64 only. BUG= 602701 Review URL: https://codereview.chromium.org/1839503002 Cr-Commit-Position: refs/heads/master@{#389500} [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/base_switches.cc [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/base_switches.h [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/trace_event/heap_profiler.h [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/trace_event/heap_profiler_allocation_context.h [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/trace_event/heap_profiler_allocation_context_tracker.cc [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/trace_event/heap_profiler_allocation_context_tracker.h [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/trace_event/heap_profiler_allocation_context_tracker_unittest.cc [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/trace_event/heap_profiler_stack_frame_deduplicator.cc [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/trace_event/memory_dump_manager.cc [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/trace_event/trace_event.h [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/trace_event/trace_log.cc
,
Apr 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e commit 2a03e86fcfb6f831ef8658cbd360f54bb5ace62e Author: creis <creis@chromium.org> Date: Mon Apr 25 17:13:05 2016 Revert of [tracing] Add native allocation tracing mode. (patchset #15 id:270001 of https://codereview.chromium.org/1839503002/ ) Reason for revert: Appears to have caused a compile error on https://build.chromium.org/p/chromium/builders/Win%20x64/builds/92. Original issue's description: > [tracing] Add native allocation tracing mode. > > This change adds 'native' mode to '--enable-heap-profiling' option. When > started with '--enable-heap-profiling=native' Chrome will keep native > (real) stack traces for all live allocations. These native stack traces > are then dumped to a trace file if 'memory-infra' category is enabled. > Stack traces are dumped unsymbolized (for performance reasons), and there > is a script (third_party/catapult/tracing/bin/symbolize_trace) that > symbolizes all stack traces in a given trace file. > > Design document: https://goo.gl/DFoqfi > > Note: native stack tracing relies on frame pointers. You'll need to use > either debug or a profiling release build (enable_profiling = true). > > Note: for now this all works on Linux @ x86/x64 only. > > BUG= 602701 > > Committed: https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a > Cr-Commit-Position: refs/heads/master@{#389500} TBR=primiano@chromium.org,perezju@chromium.org,ssid@chromium.org,mark@chromium.org,dskiba@google.com # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 602701 Review URL: https://codereview.chromium.org/1916033002 Cr-Commit-Position: refs/heads/master@{#389507} [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/base_switches.cc [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/base_switches.h [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/trace_event/heap_profiler.h [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/trace_event/heap_profiler_allocation_context.h [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/trace_event/heap_profiler_allocation_context_tracker.cc [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/trace_event/heap_profiler_allocation_context_tracker.h [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/trace_event/heap_profiler_allocation_context_tracker_unittest.cc [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/trace_event/heap_profiler_stack_frame_deduplicator.cc [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/trace_event/memory_dump_manager.cc [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/trace_event/trace_event.h [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/trace_event/trace_log.cc
,
Apr 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/79080da0203790b2e12ab36ab50f8a374aea8455 commit 79080da0203790b2e12ab36ab50f8a374aea8455 Author: dskiba <dskiba@google.com> Date: Sat Apr 23 00:10:27 2016 Reland of Add function to trace stack using frame pointers. Original CL (1879073002) was reverted because it triggered several use-of-uninitialized-value errors on Linux ChromeOS MSan bot. See crbug.com/602701#c6 for details. Those errors are expected because reading values from the stack outside of the function's frame is essential to the process of unwinding. This CL adds change to disable the unit test if MEMORY_SANITIZER is defined. Additionally, this CL fixes unwinding on ARM by moving sp adjustment several lines up. Original issue's description: > For memory-infra we need fast stack traces to implement allocation > tracing (see https://goo.gl/DFoqfi). StackTrace class uses unwinding > and is too slow. This change adds a function that uses frame pointers > to walk the stack. > > The function supports x86, x64 and arm (but not thumb) architectures > on POSIX platforms. BUG= 602701 Review URL: https://codereview.chromium.org/1915593003 Cr-Commit-Position: refs/heads/master@{#389320} [modify] https://crrev.com/79080da0203790b2e12ab36ab50f8a374aea8455/base/debug/stack_trace.cc [modify] https://crrev.com/79080da0203790b2e12ab36ab50f8a374aea8455/base/debug/stack_trace.h [modify] https://crrev.com/79080da0203790b2e12ab36ab50f8a374aea8455/base/debug/stack_trace_unittest.cc
,
Apr 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5a05938adaabf413a40f1e0dfec3d1e4c683693a commit 5a05938adaabf413a40f1e0dfec3d1e4c683693a Author: dskiba <dskiba@google.com> Date: Mon Apr 25 16:47:49 2016 [tracing] Add native allocation tracing mode. This change adds 'native' mode to '--enable-heap-profiling' option. When started with '--enable-heap-profiling=native' Chrome will keep native (real) stack traces for all live allocations. These native stack traces are then dumped to a trace file if 'memory-infra' category is enabled. Stack traces are dumped unsymbolized (for performance reasons), and there is a script (third_party/catapult/tracing/bin/symbolize_trace) that symbolizes all stack traces in a given trace file. Design document: https://goo.gl/DFoqfi Note: native stack tracing relies on frame pointers. You'll need to use either debug or a profiling release build (enable_profiling = true). Note: for now this all works on Linux @ x86/x64 only. BUG= 602701 Review URL: https://codereview.chromium.org/1839503002 Cr-Commit-Position: refs/heads/master@{#389500} [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/base_switches.cc [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/base_switches.h [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/trace_event/heap_profiler.h [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/trace_event/heap_profiler_allocation_context.h [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/trace_event/heap_profiler_allocation_context_tracker.cc [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/trace_event/heap_profiler_allocation_context_tracker.h [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/trace_event/heap_profiler_allocation_context_tracker_unittest.cc [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/trace_event/heap_profiler_stack_frame_deduplicator.cc [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/trace_event/memory_dump_manager.cc [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/trace_event/trace_event.h [modify] https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a/base/trace_event/trace_log.cc
,
Apr 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e commit 2a03e86fcfb6f831ef8658cbd360f54bb5ace62e Author: creis <creis@chromium.org> Date: Mon Apr 25 17:13:05 2016 Revert of [tracing] Add native allocation tracing mode. (patchset #15 id:270001 of https://codereview.chromium.org/1839503002/ ) Reason for revert: Appears to have caused a compile error on https://build.chromium.org/p/chromium/builders/Win%20x64/builds/92. Original issue's description: > [tracing] Add native allocation tracing mode. > > This change adds 'native' mode to '--enable-heap-profiling' option. When > started with '--enable-heap-profiling=native' Chrome will keep native > (real) stack traces for all live allocations. These native stack traces > are then dumped to a trace file if 'memory-infra' category is enabled. > Stack traces are dumped unsymbolized (for performance reasons), and there > is a script (third_party/catapult/tracing/bin/symbolize_trace) that > symbolizes all stack traces in a given trace file. > > Design document: https://goo.gl/DFoqfi > > Note: native stack tracing relies on frame pointers. You'll need to use > either debug or a profiling release build (enable_profiling = true). > > Note: for now this all works on Linux @ x86/x64 only. > > BUG= 602701 > > Committed: https://crrev.com/5a05938adaabf413a40f1e0dfec3d1e4c683693a > Cr-Commit-Position: refs/heads/master@{#389500} TBR=primiano@chromium.org,perezju@chromium.org,ssid@chromium.org,mark@chromium.org,dskiba@google.com # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 602701 Review URL: https://codereview.chromium.org/1916033002 Cr-Commit-Position: refs/heads/master@{#389507} [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/base_switches.cc [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/base_switches.h [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/trace_event/heap_profiler.h [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/trace_event/heap_profiler_allocation_context.h [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/trace_event/heap_profiler_allocation_context_tracker.cc [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/trace_event/heap_profiler_allocation_context_tracker.h [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/trace_event/heap_profiler_allocation_context_tracker_unittest.cc [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/trace_event/heap_profiler_stack_frame_deduplicator.cc [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/trace_event/memory_dump_manager.cc [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/trace_event/trace_event.h [modify] https://crrev.com/2a03e86fcfb6f831ef8658cbd360f54bb5ace62e/base/trace_event/trace_log.cc
,
Apr 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3de5ae3b200a3b86c9e7b48adbf2a4ed2c113216 commit 3de5ae3b200a3b86c9e7b48adbf2a4ed2c113216 Author: dskiba <dskiba@google.com> Date: Thu Apr 28 19:40:29 2016 Reland of [tracing] Add native allocation tracing mode. Original CL (1839503002) was reverted because it broke GYP builds. This CL removes dependency on debugging_flags.h from a header and moves it into a source file, avoiding the issue. Original issue's description: > [tracing] Add native allocation tracing mode. > > This change adds 'native' mode to '--enable-heap-profiling' option. When > started with '--enable-heap-profiling=native' Chrome will keep native > (real) stack traces for all live allocations. These native stack traces > are then dumped to a trace file if 'memory-infra' category is enabled. > Stack traces are dumped unsymbolized (for performance reasons), and there > is a script (third_party/catapult/tracing/bin/symbolize_trace) that > symbolizes all stack traces in a given trace file. > > Design document: https://goo.gl/DFoqfi > > Note: native stack tracing relies on frame pointers. You'll need to use > either debug or a profiling release build (enable_profiling = true). > > Note: for now this all works on Linux @ x86/x64 only. BUG= 602701 TBR=mark@chromium.org Review-Url: https://codereview.chromium.org/1921773003 Cr-Commit-Position: refs/heads/master@{#390449} [modify] https://crrev.com/3de5ae3b200a3b86c9e7b48adbf2a4ed2c113216/base/base_switches.cc [modify] https://crrev.com/3de5ae3b200a3b86c9e7b48adbf2a4ed2c113216/base/base_switches.h [modify] https://crrev.com/3de5ae3b200a3b86c9e7b48adbf2a4ed2c113216/base/trace_event/heap_profiler.h [modify] https://crrev.com/3de5ae3b200a3b86c9e7b48adbf2a4ed2c113216/base/trace_event/heap_profiler_allocation_context.h [modify] https://crrev.com/3de5ae3b200a3b86c9e7b48adbf2a4ed2c113216/base/trace_event/heap_profiler_allocation_context_tracker.cc [modify] https://crrev.com/3de5ae3b200a3b86c9e7b48adbf2a4ed2c113216/base/trace_event/heap_profiler_allocation_context_tracker.h [modify] https://crrev.com/3de5ae3b200a3b86c9e7b48adbf2a4ed2c113216/base/trace_event/heap_profiler_allocation_context_tracker_unittest.cc [modify] https://crrev.com/3de5ae3b200a3b86c9e7b48adbf2a4ed2c113216/base/trace_event/heap_profiler_stack_frame_deduplicator.cc [modify] https://crrev.com/3de5ae3b200a3b86c9e7b48adbf2a4ed2c113216/base/trace_event/memory_dump_manager.cc [modify] https://crrev.com/3de5ae3b200a3b86c9e7b48adbf2a4ed2c113216/base/trace_event/trace_event.h [modify] https://crrev.com/3de5ae3b200a3b86c9e7b48adbf2a4ed2c113216/base/trace_event/trace_log.cc
,
May 11 2016
Android notes: 1. We need to build in ARM mode (as opposed to THUMB mode), because stack frames are not properly generated in THUMB. There is currently no way of forcing ARM mode in GN builds. In case of GYP ARM mode was forced in profiling builds (with -marm). 2. Even with enable_profiling=true, .ninja files specify -fomit-frame-pointer in GN builds. What is interesting is that -fno-omit-frame-pointer is also specified, but it comes before -fomit-frame-pointer and thus has no effect. Fix is pending. 3. Relocation packing needs to be turned off. This is achieved with enable_profiling = true.
,
May 12 2016
> There is currently no way of forcing ARM mode in GN builds. Is it something we are going to fix? Not sure if this is unrelated with 2? Is it just a matter of -marm that got lost in the GYP->GN transition? Or is it a deliberate decision?
,
May 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/14430a4de781aca8e67f6ec7b8893647d7ee26d9 commit 14430a4de781aca8e67f6ec7b8893647d7ee26d9 Author: dskiba <dskiba@google.com> Date: Thu May 12 19:00:45 2016 Don't omit frame pointers in profiling Android GN builds. This CL makes sure that frame pointers (used for fast stack unwinding) are not omitted when enable_profiling is true. There are two changes: 1. Don't add -fomit-frame-pointer flag (Android-specific change). 2. Add -fno-omit-frame-pointer flag even in debug builds. This is needed for Android where debug builds are optimized too (-Os). BUG= 602701 Review-Url: https://codereview.chromium.org/1965143003 Cr-Commit-Position: refs/heads/master@{#393320} [modify] https://crrev.com/14430a4de781aca8e67f6ec7b8893647d7ee26d9/build/config/compiler/BUILD.gn
,
May 13 2016
The only protection we have in TraceStackFramePointers() against reading values beyond the stack is checking that next frame pointer is closer than 100000. That seems to work on Linux, but on Android we sometimes crash when trying to read value from 0xfff2f2ee.
Here is the stack trace at the time of the crash:
#0 base::debug::TraceStackFramePointers (out_trace=out_trace@entry=0xfff1bbd8, max_depth=max_depth@entry=128,
skip_initial=skip_initial@entry=0) at ../../base/debug/stack_trace.cc:70
#1 0xe01d52cc in base::trace_event::AllocationContextTracker::GetContextSnapshot (this=0xd946e980)
at ../../base/trace_event/heap_profiler_allocation_context_tracker.cc:178
#2 0xe01f49bc in base::trace_event::MallocDumpProvider::InsertAllocation (this=0xd92cff28, address=0xc6a38be0, size=224)
at ../../base/trace_event/malloc_dump_provider.cc:235
#3 0xe01f4c58 in base::trace_event::(anonymous namespace)::HookAlloc (self=<optimized out>, size=224)
at ../../base/trace_event/malloc_dump_provider.cc:38
#4 0xde0359e8 in ShimCppNew (size=224) at ../../base/allocator/allocator_shim.cc:150
#5 0xdd6e4ce0 in allocate (this=<optimized out>, __n=112)
at ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:1634
#6 allocate (__a=..., __n=112) at ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:1439
#7 std::__1::basic_string<unsigned short, base::string16_char_traits, std::__1::allocator<unsigned short> >::__init<char const*> (
this=this@entry=0xfff1c02c,
__first=0xebbc7be0 "https://www.google.com/search?q=fdfcgg&oq=fdfcgg&aqs=chrome..69i57j0l3.1968j0j4&sourceid=chrome-mobile&ie=UTF-8",
__last=0xebbc7c4f "") at ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:2259
#8 0xdd6e47b4 in basic_string<char const*> (__last=<optimized out>, __first=<optimized out>, this=0xfff1c02c)
at ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:2274
#9 base::UTF8ToUTF16 (utf8=...) at ../../base/strings/utf_string_conversions.cc:161
#10 0xdd6e46d0 in base::android::ConvertUTF8ToJavaString (env=env@entry=0xf5596b40, str=...) at ../../base/android/jni_string.cc:74
#11 0xdd820420 in content::WebContentsAndroid::GetURL (this=<optimized out>, env=0xf5596b40, obj=...)
at ../../content/browser/web_contents/web_contents_android.cc:360
#12 0xdd8203b8 in content::Java_org_chromium_content_browser_webcontents_WebContentsImpl_nativeGetURL (env=<optimized out>,
jcaller=<optimized out>, nativeWebContentsAndroid=<optimized out>)
at gen/content/public/android/content_jni_headers/content/jni/WebContentsImpl_jni.h:414
#13 0xe2049e6a in ?? ()
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
Note frame #12 - it's JNI function that was called by ART. Crash happens only on main thread, and only when we're called from Java.
We crashed because we tried to do reinterpret_cast<const void**>(sp)[1] with sp being 0xfff2f2ee. According to proc/maps stand ends 70382 bytes eariler:
ff71f000-fff1e000 rw-p 00000000 00:00 0 [stack]
At the point of the crash we collected 12 frames (depth is 12):
(gdb) x/12x out_trace
0xfff1bbd8: 0xe01d52cc 0xe01f49bc 0xe01f4c58 0xde0359e8
0xfff1bbe8: 0xdd6e4ce0 0xdd6e47b4 0xdd6e46d0 0xdd820420
0xfff1bbf8: 0xdd8203b8 0xe2049e6b 0x00000002 0xf55b658c
Let's see what gdb thinks:
#11 0xdd820420 in content::WebContentsAndroid::GetURL (this=<optimized out>, env=0xf5596b40, obj=...)
at ../../content/browser/web_contents/web_contents_android.cc:360
(gdb) p/x $lr
$23 = 0xdd820420
#12 0xdd8203b8 in content::Java_org_chromium_content_browser_webcontents_WebContentsImpl_nativeGetURL (env=<optimized out>,
jcaller=<optimized out>, nativeWebContentsAndroid=<optimized out>)
at gen/content/public/android/content_jni_headers/content/jni/WebContentsImpl_jni.h:414
(gdb) p/x $lr
$24 = 0xdd8203b8
#13 0xe2049e6a in ?? ()
(gdb) p/x $lr
$25 = 0xe2049e6b
So frame #13 corresponds to out_trace[9]. At this point gdb stops, but we continue our journey into the depths of ART, and eventually we find bad sp there.
There are two ways to avoid the crash:
1. Check that sp inside stack. This involves getting stack address and size via pthread_getattr_np / pthread_attr_getstack.
Problem: On Linux (Android included) main thread is special, and while usually pthread_getattr_np() just dereferences passed in pthread_t, for main thread it goes and reads proc/maps every time. Reading proc/maps involves malloc'ing things (both glibc and bionic use fopen), and that's a problem on Linux where malloc is weak (but not on Android where it's not).
2. Check that PC is inside our binary. I.e. on both Linux and Android system libs don't have frame pointers, so we'll stop on system libraries anyway, just instead of sloppy checking of sp being within 100000, we'll reliably check PC.
Problem: Hard to do in component build. In non-component build we can just find proc/maps entry that contains &TraceStackFramePointers, cache it's address/size in statics and voila. Checking is also super simple. Component build complicates that significantly - we'll have to somehow find all our libraries in proc/maps, have array of addresses, binary search it, etc.
I think I'm going to implement both #1 and #2 and we'll see which is better.
,
May 13 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2a91d261e9858e3914153f0fc3d0f57a4b126558 commit 2a91d261e9858e3914153f0fc3d0f57a4b126558 Author: dskiba <dskiba@google.com> Date: Fri May 13 20:33:59 2016 Allow opting out of thumb mode for ARM builds. Heap profiler relies on frame pointers to do stack unwinding. ARM builds are using thumb mode by default, but since frame pointers are broken in thumb mode for both GCC and LLVM (https://llvm.org/bugs/show_bug.cgi?id=18505), we need a way to build in arm mode. This change exposes arm_use_thumb GN variable so that it can be set to false. BUG= 602701 Review-Url: https://codereview.chromium.org/1977533002 Cr-Commit-Position: refs/heads/master@{#393615} [modify] https://crrev.com/2a91d261e9858e3914153f0fc3d0f57a4b126558/build/config/arm.gni
,
May 19 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/330cd00d2ce9cc10d34ba1264d4dcc66c40916b1 commit 330cd00d2ce9cc10d34ba1264d4dcc66c40916b1 Author: catapult-deps-roller <catapult-deps-roller@chromium.org> Date: Thu May 19 06:37:01 2016 Roll src/third_party/catapult/ 73dc5f4ee..9085ddba7 (15 commits). https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/73dc5f4ee8e4..9085ddba7182 $ git log 73dc5f4ee..9085ddba7 --date=short --no-merges --format='%ad %ae %s' BUG= 352807 , 352807 ,612140, 581716 , 602701 TBR=catapult-sheriff@chromium.org Review-Url: https://codereview.chromium.org/1994023002 Cr-Commit-Position: refs/heads/master@{#394699} [modify] https://crrev.com/330cd00d2ce9cc10d34ba1264d4dcc66c40916b1/DEPS
,
May 20 2016
Implemented both methods from #18: 1. https://codereview.chromium.org/1975393002 uses 'stack end' to stop unwinding once we get bad sp. 2. https://codereview.chromium.org/1996243002 works in non-component build only and uses 'is PC from Chrome' check to stop unwinding as soon as we cross Chrome boundary. Additionally it adds a hack to unwinding Clang thumb builds.
,
May 20 2016
I'm not sure that 2 will give you any protection from crashes. Knowing if a PC is inside the chrome so can be useful to tell whether is bogus or not. However the problem here is that you need to know if an alleged FP is within the stack, that is causing you to segfault. In other words 2. is not going to fix the problem where you have a valid PC but the FP is pointing to garbage. I'd just go for 1, my theory is that if the FP chain is valid the PC which sits just next to that should be correct almost all the times. Also an invalid PC doesn't cause you to crash, just to produce garbage data. It's not the case instead for an invalid FP.
,
May 20 2016
So in that specific case FP was bad precisely because we went outside of Chrome. All Chrome is built with frame pointers, so as long as we stay within, FP should always be good. And actually we shouldn't unwind into JNI and system libraries at all because they don't have FPs.
,
Jun 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0bed5150f19acdd4897dd12ea8e4d4802c51d75f commit 0bed5150f19acdd4897dd12ea8e4d4802c51d75f Author: dskiba <dskiba@google.com> Date: Mon Jun 06 21:37:57 2016 Check stack pointer to be inside stack when unwinding. TraceStackFramePointers() function, which unwinds stack using frame pointers, sometimes crashes on Android when it dives deep into JVM internals and finds bad stack pointer there. See details here: crbug.com/602701#c18 This CL adds checks to make sure only valid stack pointers are dereferenced. BUG= 602701 Review-Url: https://codereview.chromium.org/1975393002 Cr-Commit-Position: refs/heads/master@{#398134} [modify] https://crrev.com/0bed5150f19acdd4897dd12ea8e4d4802c51d75f/base/debug/stack_trace.cc
,
Jun 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2914afa182f14140fb21bc9a68e058452474a0e3 commit 2914afa182f14140fb21bc9a68e058452474a0e3 Author: dskiba <dskiba@google.com> Date: Thu Jun 30 20:27:59 2016 [Docs] Add native heap profiling instructions. Native heap profiling captures C/C++ backtraces for allocations, but requires a special build (and a symbolization step). BUG= 602701 Review-Url: https://codereview.chromium.org/2084973002 Cr-Commit-Position: refs/heads/master@{#403269} [modify] https://crrev.com/2914afa182f14140fb21bc9a68e058452474a0e3/components/tracing/docs/heap_profiler.md
,
Aug 1 2016
|
|||
►
Sign in to add a comment |
|||
Comment 1 by bugdroid1@chromium.org
, Apr 20 2016