Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 2840193003: [Android] Fix stack symbolization when packed relocations are on. (Closed)

Created:
6 months, 3 weeks ago by jbudorick
Modified:
6 months, 3 weeks ago
Reviewers:
Ted C, Dirk Pranke, agrieve
CC:
chromium-reviews, kalyank, mikecase+watch_chromium.org, sadrul, jbudorick+watch_chromium.org, agrieve+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[Android] Fix stack symbolization when packed relocations are on. We had been looking in lib.stripped for the library when symbolizing debuggerd traces. The libraries in lib.stripped are not the ones with packed relocations, though, so the resulting symbolization was wrong. This CL generates a wrapper script around the stack symbolization script that points to the correct library to use for symbolization. It also contains a few tests that intentionally crash in various ways. These are all disabled by default and should only ever be run manually for testing out instrumentation test crash tooling. BUG=712265 Review-Url: https://codereview.chromium.org/2840193003 Cr-Commit-Position: refs/heads/master@{#468206} Committed: https://chromium.googlesource.com/chromium/src/+/276cc56b7c04cc3a28ec6315feb00f056ea8327b

Patch Set 1 #

Patch Set 2 : omitted build/secondary/third_party/android_platform/ #

Total comments: 5

Patch Set 3 : agrieve comments #

Total comments: 2

Patch Set 4 : tedchoc comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+291 lines, -33 lines) Patch
M PRESUBMIT.py View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
A build/android/gyp/create_stack_script.py View 1 chunk +84 lines, -0 lines 0 comments Download
M build/config/android/internal_rules.gni View 1 2 3 1 chunk +55 lines, -0 lines 0 comments Download
M build/config/android/rules.gni View 1 2 3 1 chunk +11 lines, -0 lines 0 comments Download
M build/print_python_deps.py View 1 2 1 chunk +2 lines, -1 line 0 comments Download
A build/secondary/third_party/android_platform/development/scripts/BUILD.gn View 1 1 chunk +13 lines, -0 lines 0 comments Download
A build/secondary/third_party/android_platform/development/scripts/stack.pydeps View 1 1 chunk +19 lines, -0 lines 0 comments Download
M chrome/android/java_sources.gni View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
A chrome/android/javatests/src/org/chromium/chrome/test/crash/IntentionalCrashTest.java View 1 2 3 1 chunk +69 lines, -0 lines 0 comments Download
M third_party/android_platform/development/scripts/stack View 5 chunks +18 lines, -23 lines 0 comments Download
M third_party/android_platform/development/scripts/stack_libs.py View 4 chunks +17 lines, -9 lines 0 comments Download

Messages

Total messages: 21 (10 generated)
jbudorick
Noticed this while working on multidex tests. I think there's room for further work here ...
6 months, 3 weeks ago (2017-04-26 17:18:20 UTC) #2
agrieve
lgtm. Nice to have this actually work. I know there have been the odd complaint ...
6 months, 3 weeks ago (2017-04-26 18:19:44 UTC) #3
jbudorick
I do want the bots to run this in some capacity. There are a few ...
6 months, 3 weeks ago (2017-04-26 19:34:22 UTC) #4
agrieve
lgtm https://codereview.chromium.org/2840193003/diff/20001/build/print_python_deps.py File build/print_python_deps.py (right): https://codereview.chromium.org/2840193003/diff/20001/build/print_python_deps.py#newcode41 build/print_python_deps.py:41: if path.endswith('c') and os.path.exists(path[:-1]): On 2017/04/26 19:34:22, jbudorick ...
6 months, 3 weeks ago (2017-04-27 00:30:18 UTC) #9
jbudorick
On 2017/04/27 00:30:18, agrieve wrote: > lgtm > > https://codereview.chromium.org/2840193003/diff/20001/build/print_python_deps.py > File build/print_python_deps.py (right): > ...
6 months, 3 weeks ago (2017-04-27 00:40:06 UTC) #10
jbudorick
+dpranke for PRESUBMIT.py +tedchoc for IntentionalCrashTest
6 months, 3 weeks ago (2017-04-27 00:45:54 UTC) #12
Dirk Pranke
lgtm
6 months, 3 weeks ago (2017-04-27 00:49:40 UTC) #13
Ted C
lgtm https://codereview.chromium.org/2840193003/diff/40001/chrome/android/javatests/src/org/chromium/chrome/test/crash/IntentionalCrashTest.java File chrome/android/javatests/src/org/chromium/chrome/test/crash/IntentionalCrashTest.java (right): https://codereview.chromium.org/2840193003/diff/40001/chrome/android/javatests/src/org/chromium/chrome/test/crash/IntentionalCrashTest.java#newcode23 chrome/android/javatests/src/org/chromium/chrome/test/crash/IntentionalCrashTest.java:23: @CommandLineFlags.Add("disable-fre") ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE
6 months, 3 weeks ago (2017-04-27 16:09:45 UTC) #14
jbudorick
https://codereview.chromium.org/2840193003/diff/40001/chrome/android/javatests/src/org/chromium/chrome/test/crash/IntentionalCrashTest.java File chrome/android/javatests/src/org/chromium/chrome/test/crash/IntentionalCrashTest.java (right): https://codereview.chromium.org/2840193003/diff/40001/chrome/android/javatests/src/org/chromium/chrome/test/crash/IntentionalCrashTest.java#newcode23 chrome/android/javatests/src/org/chromium/chrome/test/crash/IntentionalCrashTest.java:23: @CommandLineFlags.Add("disable-fre") On 2017/04/27 16:09:45, Ted C wrote: > ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE ...
6 months, 3 weeks ago (2017-04-28 23:00:14 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2840193003/60001
6 months, 3 weeks ago (2017-04-28 23:01:04 UTC) #18
commit-bot: I haz the power
6 months, 3 weeks ago (2017-04-29 01:35:18 UTC) #21
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/276cc56b7c04cc3a28ec6315feb0...

Powered by Google App Engine
This is Rietveld efc10ee0f