Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(71)

Issue 545148: Import google-glog's Symbolize() and use it in debug_util_posix.cc. (Closed)

Created:
10 years, 11 months ago by satorux1
Modified:
9 years, 7 months ago
Reviewers:
hamaji, Evan Martin, awong
CC:
chromium-reviews_googlegroups.com, brettw+cc_chromium.org, Paweł Hajdan Jr., pam+watch_chromium.org, John Grabowski, Yusuke Sato
Visibility:
Public.

Description

Import google-glog's Symbolize() and use it in debug_util_posix.cc. Unlike glibc's backtrace_symbols(), google-glog's Symbolize() can resolve symbols in binaries built with -fvisibility=hidden. This is because Symbolize() uses both dynamic and regular symbols, while backtrace_symbols() only uses dynamic symbols. As shown below, the new backtrace is slightly less informative as it does not have the binary name like out/Debug/base_unittests and the distance information like +0x20, but it should be ok. BUG=32762 TEST=out/Debug/base_unittests --gtest_filter='StackTrace*' --gtest_also_run_disabled_tests; and manually: BEFORE out/Debug/base_unittests(StackTrace::StackTrace()+0x20) [0x81d38f6] out/Debug/base_unittests(StackTrace_DISABLED_DebugOutputToStream_Test::TestBody()+0x17) [0x806633d] out/Debug/base_unittests(testing::Test::Run()+0x7a) [0x8242f96] out/Debug/base_unittests(testing::internal::TestInfoImpl::Run()+0xb9) [0x824347b] out/Debug/base_unittests(testing::TestCase::Run()+0xb7) [0x8243a7f] out/Debug/base_unittests(testing::internal::UnitTestImpl::RunAllTests()+0x256) [0x824777e] out/Debug/base_unittests(testing::UnitTest::Run()+0x14) [0x824680a] out/Debug/base_unittests [0x8048cc5] out/Debug/base_unittests(main+0x50) [0x80482a4] /lib/tls/i686/cmov/libc.so.6(__libc_start_main+0xe6) [0xb6b41b56] out/Debug/base_unittests [0x80481c1] AFTER % out/Debug/base_unittests --gtest_filter='*DebugOutputToStream' --gtest_also_run_disabled_tests StackTrace::StackTrace() [0x81d3910] StackTrace_DISABLED_DebugOutputToStream_Test::TestBody() [0x806633d] testing::Test::Run() [0x8242f66] testing::internal::TestInfoImpl::Run() [0x824344b] testing::TestCase::Run() [0x8243a4f] testing::internal::UnitTestImpl::RunAllTests() [0x824774e] testing::UnitTest::Run() [0x82467da] TestSuite::Run() [0x8048cc5] main [0x80482a4] 0xb6c0db56 0x80481c1 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=36997 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=37090

Patch Set 1 #

Patch Set 2 : add README.chromium #

Patch Set 3 : cosmetic change #

Patch Set 4 : Delete use of backtrace_symbols_fd() #

Total comments: 6

Patch Set 5 : address comments #

Patch Set 6 : subtract one before symbolizing #

Patch Set 7 : fix the build #

Total comments: 2

Patch Set 8 : Fix checkdeps.py error #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+2256 lines, -16 lines) Patch
M base/base.gypi View 1 2 3 4 5 6 7 2 chunks +19 lines, -0 lines 0 comments Download
M base/debug_util_posix.cc View 1 2 3 4 5 6 5 chunks +61 lines, -16 lines 2 comments Download
M base/debug_util_unittest.cc View 1 2 3 4 1 chunk +15 lines, -0 lines 0 comments Download
A base/third_party/symbolize/DEPS View 1 chunk +3 lines, -0 lines 0 comments Download
A base/third_party/symbolize/README.chromium View 2 3 4 1 chunk +17 lines, -0 lines 0 comments Download
A base/third_party/symbolize/config.h View 1 2 3 4 1 chunk +7 lines, -0 lines 0 comments Download
A base/third_party/symbolize/demangle.h View 1 chunk +84 lines, -0 lines 0 comments Download
A base/third_party/symbolize/demangle.cc View 1 chunk +1231 lines, -0 lines 0 comments Download
A base/third_party/symbolize/glog/logging.h View 1 2 3 4 1 chunk +5 lines, -0 lines 0 comments Download
A base/third_party/symbolize/glog/raw_logging.h View 1 2 3 4 1 chunk +6 lines, -0 lines 0 comments Download
A base/third_party/symbolize/symbolize.h View 1 chunk +116 lines, -0 lines 0 comments Download
A base/third_party/symbolize/symbolize.cc View 1 chunk +681 lines, -0 lines 0 comments Download
A base/third_party/symbolize/utilities.h View 1 2 3 4 1 chunk +11 lines, -0 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
satorux1
Evan and Albert, I just gave it a shot. Hope you'll like it.
10 years, 11 months ago (2010-01-21 09:56:33 UTC) #1
satorux1
Oops, I accidentally uploaded *.o files. I'll remove these. At this moment, I don't have ...
10 years, 11 months ago (2010-01-21 14:35:36 UTC) #2
Evan Martin
http://codereview.chromium.org/545148/diff/4001/4004 File base/debug_util_unittest.cc (right): http://codereview.chromium.org/545148/diff/4001/4004#newcode96 base/debug_util_unittest.cc:96: // --gtest_filter='*DebugOutputToStream' --gtest_also_run_disabled_tests Why is it for manual testing ...
10 years, 11 months ago (2010-01-21 19:54:01 UTC) #3
satorux1
Evan, Thank you for the review. http://codereview.chromium.org/545148/diff/4001/4004 File base/debug_util_unittest.cc (right): http://codereview.chromium.org/545148/diff/4001/4004#newcode96 base/debug_util_unittest.cc:96: // --gtest_filter='*DebugOutputToStream' --gtest_also_run_disabled_tests ...
10 years, 11 months ago (2010-01-22 02:28:37 UTC) #4
Evan Martin
LGTM
10 years, 11 months ago (2010-01-22 02:32:25 UTC) #5
satorux1
Slightly changed the code to deal with a "return address is in the next function ...
10 years, 11 months ago (2010-01-22 03:56:00 UTC) #6
hamaji
LGTM
10 years, 11 months ago (2010-01-22 04:08:51 UTC) #7
awong
LGTM I only looked through the debug_util changes. http://codereview.chromium.org/545148/diff/5004/5007 File base/debug_util_unittest.cc (right): http://codereview.chromium.org/545148/diff/5004/5007#newcode70 base/debug_util_unittest.cc:70: #elif ...
10 years, 11 months ago (2010-01-22 20:03:58 UTC) #8
satorux1
http://codereview.chromium.org/545148/diff/5004/5007 File base/debug_util_unittest.cc (right): http://codereview.chromium.org/545148/diff/5004/5007#newcode70 base/debug_util_unittest.cc:70: #elif 0 On 2010/01/22 20:03:58, awong wrote: > Are ...
10 years, 11 months ago (2010-01-25 10:47:02 UTC) #9
satorux1
Fixed the checkdeps.py error by adding base/third_party/symbolize/DEPS, that caused the tree to be closed. Please ...
10 years, 11 months ago (2010-01-25 13:20:40 UTC) #10
Evan Martin
http://codereview.chromium.org/545148/diff/8004/7004 File base/debug_util_posix.cc (right): http://codereview.chromium.org/545148/diff/8004/7004#newcode37 base/debug_util_posix.cc:37: #include "base/third_party/symbolize/symbolize.h" It seems this included file then then ...
10 years, 11 months ago (2010-01-25 18:10:40 UTC) #11
satorux1
http://codereview.chromium.org/545148/diff/8004/7004 File base/debug_util_posix.cc (right): http://codereview.chromium.org/545148/diff/8004/7004#newcode37 base/debug_util_posix.cc:37: #include "base/third_party/symbolize/symbolize.h" On 2010/01/25 18:10:40, Evan Martin wrote: > ...
10 years, 11 months ago (2010-01-25 21:15:50 UTC) #12
Evan Martin
10 years, 11 months ago (2010-01-25 23:49:14 UTC) #13
re-LGTM in case it wasn't clear

Powered by Google App Engine
This is Rietveld 408576698