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

Issue 819888 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Implement stack unwinding on Android user builds

Project Member Reported by ssid@chromium.org, Mar 8 2018

Issue description

The idea is to extract the unwind table from unstripped binary and add it to the apk as an asset file. This is only done on dev and canary channels so that the beta and stable channels still benefit from the unwind table optimizations.

Doc: https://docs.google.com/document/d/1TLuUZ1HaMO6Rv0Q9Y1-w4a-9wcyia1VygQbs4Osb7Oo
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 13 2018

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

commit 764226bdbd10e46076fbc691bcaa27f050075c7a
Author: Siddhartha <ssid@chromium.org>
Date: Tue Mar 13 02:32:55 2018

Add a script to extract and add unwind table to apk

The unwind tables are stripped from release builds. This CL adds a
script to extract the unwind table sections from unstripped binary and
adds it as an asset file in the apk.
This script is first used in the test apk, when add_unwind_tables_in_apk
is specified. Will be used later added to chrome apk.

BUG=819888

Change-Id: I68d2ea843ed0c7a7a3ee60c48f683e01809ff217
Reviewed-on: https://chromium-review.googlesource.com/954421
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542699}
[add] https://crrev.com/764226bdbd10e46076fbc691bcaa27f050075c7a/build/android/gyp/extract_unwind_tables.py
[add] https://crrev.com/764226bdbd10e46076fbc691bcaa27f050075c7a/build/config/android/extract_unwind_tables.gni
[modify] https://crrev.com/764226bdbd10e46076fbc691bcaa27f050075c7a/build/config/android/rules.gni
[modify] https://crrev.com/764226bdbd10e46076fbc691bcaa27f050075c7a/testing/test.gni

Project Member

Comment 2 by bugdroid1@chromium.org, Mar 13 2018

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

commit 7819ba6a2af0d732806aaea80db94361cd626d6e
Author: Siddhartha <ssid@chromium.org>
Date: Tue Mar 13 05:10:58 2018

Extract CFI table info from breakpad symbol file for unwinding

The extract_unwind_tables script dumps the breakpad symbol file and
extracts cfi unwind table from it.
The output format is discussed in this doc:
https://docs.google.com/document/d/1TLuUZ1HaMO6Rv0Q9Y1-w4a-9wcyia1VygQbs4Osb7Oo
This is just a basic format and will be changed in subsequent cl for
size efficiency and performance.

BUG=819888

Change-Id: I987aa2e4ef37f86b6553410d7ee73c15f655495b
Reviewed-on: https://chromium-review.googlesource.com/956971
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542736}
[modify] https://crrev.com/7819ba6a2af0d732806aaea80db94361cd626d6e/build/android/gyp/extract_unwind_tables.py
[add] https://crrev.com/7819ba6a2af0d732806aaea80db94361cd626d6e/build/android/gyp/extract_unwind_tables_tests.py
[modify] https://crrev.com/7819ba6a2af0d732806aaea80db94361cd626d6e/build/android/pylib/constants/__init__.py
[modify] https://crrev.com/7819ba6a2af0d732806aaea80db94361cd626d6e/build/config/android/extract_unwind_tables.gni

Comment 3 by ssid@chromium.org, Mar 26 2018

Cc: erikc...@chromium.org
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 28 2018

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

commit 1d8b0c00961571a16a25633be92ed196fbe51271
Author: Siddhartha <ssid@chromium.org>
Date: Wed Mar 28 00:54:20 2018

Android: Add code to parse the CFI info added to the apk

The previous CL added support for adding unwind tables in apk:
https://chromium-review.googlesource.com/c/chromium/src/+/956971
This CL parses and adds test for unwinding on base_unittests_apk.
The non-official builds do not contain debug info since all bots set
strip_debug_info=true. So, make sure base_unittests do not take this
flag.

BUG=819888

Change-Id: Ib69909e14f9f8623ac07154ef60be248558b08a4
Reviewed-on: https://chromium-review.googlesource.com/958326
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546343}
[modify] https://crrev.com/1d8b0c00961571a16a25633be92ed196fbe51271/base/BUILD.gn
[add] https://crrev.com/1d8b0c00961571a16a25633be92ed196fbe51271/base/trace_event/cfi_backtrace_android.cc
[add] https://crrev.com/1d8b0c00961571a16a25633be92ed196fbe51271/base/trace_event/cfi_backtrace_android.h
[add] https://crrev.com/1d8b0c00961571a16a25633be92ed196fbe51271/base/trace_event/cfi_backtrace_android_unittest.cc
[modify] https://crrev.com/1d8b0c00961571a16a25633be92ed196fbe51271/build/config/compiler/BUILD.gn
[modify] https://crrev.com/1d8b0c00961571a16a25633be92ed196fbe51271/build/config/compiler/compiler.gni

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 28 2018

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

commit eb07cea1254cc280dfec0c8ca0e6411dfcceecca
Author: Siddhartha <ssid@chromium.org>
Date: Wed Mar 28 06:37:44 2018

Enable extracting unwind table on official builds without channel

This CL enables extraction of unwind tables for chrome when
is_official_build is set to true and when android_channel is not set to
a release channel. This affects build time only on official builds.
Also, Set legacy heap profiler to use unwinder for profiling.

TBR=dpranke@chromium.org
BUG=819888

Change-Id: If9732b5ac1f9dff49c58e88e2ffb6cb387715244
Reviewed-on: https://chromium-review.googlesource.com/976944
Reviewed-by: Siddhartha S <ssid@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546425}
[modify] https://crrev.com/eb07cea1254cc280dfec0c8ca0e6411dfcceecca/base/BUILD.gn
[modify] https://crrev.com/eb07cea1254cc280dfec0c8ca0e6411dfcceecca/base/trace_event/heap_profiler_allocation_context_tracker.cc
[modify] https://crrev.com/eb07cea1254cc280dfec0c8ca0e6411dfcceecca/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/eb07cea1254cc280dfec0c8ca0e6411dfcceecca/build/config/android/extract_unwind_tables.gni
[modify] https://crrev.com/eb07cea1254cc280dfec0c8ca0e6411dfcceecca/chrome/android/BUILD.gn
[modify] https://crrev.com/eb07cea1254cc280dfec0c8ca0e6411dfcceecca/testing/test.gni

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 28 2018

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

commit 88328b6800f987ba9f532842862021cb48875618
Author: François Doray <fdoray@chromium.org>
Date: Wed Mar 28 13:51:21 2018

Revert "Android: Add code to parse the CFI info added to the apk"

This reverts commit 1d8b0c00961571a16a25633be92ed196fbe51271.

Reason for revert:  https://crbug.com/826718 

CL causes failures on Android CFI builder.

Original change's description:
> Android: Add code to parse the CFI info added to the apk
> 
> The previous CL added support for adding unwind tables in apk:
> https://chromium-review.googlesource.com/c/chromium/src/+/956971
> This CL parses and adds test for unwinding on base_unittests_apk.
> The non-official builds do not contain debug info since all bots set
> strip_debug_info=true. So, make sure base_unittests do not take this
> flag.
> 
> BUG=819888
> 
> Change-Id: Ib69909e14f9f8623ac07154ef60be248558b08a4
> Reviewed-on: https://chromium-review.googlesource.com/958326
> Commit-Queue: Siddhartha S <ssid@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Primiano Tucci <primiano@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#546343}

TBR=dcheng@chromium.org,primiano@chromium.org,dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org,dskiba@chromium.org

Change-Id: Ib9bcb81ffa30861ea91107c1a1f3ccb8f29a7f7d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 819888
Reviewed-on: https://chromium-review.googlesource.com/983972
Reviewed-by: François Doray <fdoray@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546472}
[modify] https://crrev.com/88328b6800f987ba9f532842862021cb48875618/base/BUILD.gn
[delete] https://crrev.com/2eab8d34fcdf23fb06fbd336f8ca38043d56d62f/base/trace_event/cfi_backtrace_android.cc
[delete] https://crrev.com/2eab8d34fcdf23fb06fbd336f8ca38043d56d62f/base/trace_event/cfi_backtrace_android.h
[delete] https://crrev.com/2eab8d34fcdf23fb06fbd336f8ca38043d56d62f/base/trace_event/cfi_backtrace_android_unittest.cc
[modify] https://crrev.com/88328b6800f987ba9f532842862021cb48875618/build/config/compiler/BUILD.gn
[modify] https://crrev.com/88328b6800f987ba9f532842862021cb48875618/build/config/compiler/compiler.gni

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 28 2018

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

commit a2474b6490ea9cfda76528059fbf23369e7d281f
Author: agrieve <agrieve@chromium.org>
Date: Wed Mar 28 14:10:37 2018

Revert "Enable extracting unwind table on official builds without channel"

This reverts commit eb07cea1254cc280dfec0c8ca0e6411dfcceecca.

Reason for revert: Windows tryjobs are failing.
https://logs.chromium.org/v/?s=chromium%2Fbb%2Ftryserver.chromium.win%2Fwin-msvc-rel%2F81420%2F%2B%2Frecipes%2Fsteps%2Fanalyze%2F0%2Fstdout for an example.

ERROR at //base/BUILD.gn:1802:33: Undefined identifier in string expansion.
    "CAN_UNWIND_WITH_CFI_TABLE=$can_unwind_with_cfi_table",
                                ^------------------------
"can_unwind_with_cfi_table" is not currently in scope.
See //BUILD.gn:65:5: which caused the file to be included.
    "//base:base_perftests",
    ^----------------------
GN gen failed: 1

Original change's description:
> Enable extracting unwind table on official builds without channel
> 
> This CL enables extraction of unwind tables for chrome when
> is_official_build is set to true and when android_channel is not set to
> a release channel. This affects build time only on official builds.
> Also, Set legacy heap profiler to use unwinder for profiling.
> 
> TBR=dpranke@chromium.org
> BUG=819888
> 
> Change-Id: If9732b5ac1f9dff49c58e88e2ffb6cb387715244
> Reviewed-on: https://chromium-review.googlesource.com/976944
> Reviewed-by: Siddhartha S <ssid@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Commit-Queue: Siddhartha S <ssid@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#546425}

TBR=dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org

Change-Id: Id033094871dfcf74248f631390f15f379d9cd3c7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 819888
Reviewed-on: https://chromium-review.googlesource.com/983893
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546478}
[modify] https://crrev.com/a2474b6490ea9cfda76528059fbf23369e7d281f/base/BUILD.gn
[modify] https://crrev.com/a2474b6490ea9cfda76528059fbf23369e7d281f/base/trace_event/heap_profiler_allocation_context_tracker.cc
[modify] https://crrev.com/a2474b6490ea9cfda76528059fbf23369e7d281f/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/a2474b6490ea9cfda76528059fbf23369e7d281f/build/config/android/extract_unwind_tables.gni
[modify] https://crrev.com/a2474b6490ea9cfda76528059fbf23369e7d281f/chrome/android/BUILD.gn
[modify] https://crrev.com/a2474b6490ea9cfda76528059fbf23369e7d281f/testing/test.gni

Project Member

Comment 8 by bugdroid1@chromium.org, Mar 28 2018

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

commit 839ddb373b08557fbfce1a836ea45906d5650990
Author: Henrik Andreasson <henrika@chromium.org>
Date: Wed Mar 28 14:16:32 2018

Reland "Android: Add code to parse the CFI info added to the apk"

This reverts commit 88328b6800f987ba9f532842862021cb48875618.

Reason for revert: seems to break "generate_build_files" step on several of our build bots, see e.g.:

https://build.chromium.org/deprecated/chromium.webrtc/builders/Linux%20Builder/builds/126662

Original change's description:
> Revert "Android: Add code to parse the CFI info added to the apk"
> 
> This reverts commit 1d8b0c00961571a16a25633be92ed196fbe51271.
> 
> Reason for revert:  https://crbug.com/826718 
> 
> CL causes failures on Android CFI builder.
> 
> Original change's description:
> > Android: Add code to parse the CFI info added to the apk
> > 
> > The previous CL added support for adding unwind tables in apk:
> > https://chromium-review.googlesource.com/c/chromium/src/+/956971
> > This CL parses and adds test for unwinding on base_unittests_apk.
> > The non-official builds do not contain debug info since all bots set
> > strip_debug_info=true. So, make sure base_unittests do not take this
> > flag.
> > 
> > BUG=819888
> > 
> > Change-Id: Ib69909e14f9f8623ac07154ef60be248558b08a4
> > Reviewed-on: https://chromium-review.googlesource.com/958326
> > Commit-Queue: Siddhartha S <ssid@chromium.org>
> > Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> > Reviewed-by: Primiano Tucci <primiano@chromium.org>
> > Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> > Reviewed-by: agrieve <agrieve@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#546343}
> 
> TBR=dcheng@chromium.org,primiano@chromium.org,dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org,dskiba@chromium.org
> 
> Change-Id: Ib9bcb81ffa30861ea91107c1a1f3ccb8f29a7f7d
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 819888
> Reviewed-on: https://chromium-review.googlesource.com/983972
> Reviewed-by: François Doray <fdoray@chromium.org>
> Commit-Queue: François Doray <fdoray@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#546472}

TBR=dcheng@chromium.org,primiano@chromium.org,fdoray@chromium.org,dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org,dskiba@chromium.org

Change-Id: Ib39e4563c82264389d7a740a877d27edef343101
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 819888
Reviewed-on: https://chromium-review.googlesource.com/984092
Reviewed-by: Henrik Andreasson <henrika@chromium.org>
Commit-Queue: Henrik Andreasson <henrika@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546479}
[modify] https://crrev.com/839ddb373b08557fbfce1a836ea45906d5650990/base/BUILD.gn
[add] https://crrev.com/839ddb373b08557fbfce1a836ea45906d5650990/base/trace_event/cfi_backtrace_android.cc
[add] https://crrev.com/839ddb373b08557fbfce1a836ea45906d5650990/base/trace_event/cfi_backtrace_android.h
[add] https://crrev.com/839ddb373b08557fbfce1a836ea45906d5650990/base/trace_event/cfi_backtrace_android_unittest.cc
[modify] https://crrev.com/839ddb373b08557fbfce1a836ea45906d5650990/build/config/compiler/BUILD.gn
[modify] https://crrev.com/839ddb373b08557fbfce1a836ea45906d5650990/build/config/compiler/compiler.gni

Project Member

Comment 9 by bugdroid1@chromium.org, Mar 28 2018

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

commit 19a53de61b455f332b946a76da03faa716b9b11e
Author: Peter Mayo <petermayo@chromium.org>
Date: Wed Mar 28 14:35:56 2018

Revert "Reland "Android: Add code to parse the CFI info added to the apk""

This reverts commit 839ddb373b08557fbfce1a836ea45906d5650990.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> Reland "Android: Add code to parse the CFI info added to the apk"
> 
> This reverts commit 88328b6800f987ba9f532842862021cb48875618.
> 
> Reason for revert: seems to break "generate_build_files" step on several of our build bots, see e.g.:
> 
> https://build.chromium.org/deprecated/chromium.webrtc/builders/Linux%20Builder/builds/126662
> 
> Original change's description:
> > Revert "Android: Add code to parse the CFI info added to the apk"
> > 
> > This reverts commit 1d8b0c00961571a16a25633be92ed196fbe51271.
> > 
> > Reason for revert:  https://crbug.com/826718 
> > 
> > CL causes failures on Android CFI builder.
> > 
> > Original change's description:
> > > Android: Add code to parse the CFI info added to the apk
> > > 
> > > The previous CL added support for adding unwind tables in apk:
> > > https://chromium-review.googlesource.com/c/chromium/src/+/956971
> > > This CL parses and adds test for unwinding on base_unittests_apk.
> > > The non-official builds do not contain debug info since all bots set
> > > strip_debug_info=true. So, make sure base_unittests do not take this
> > > flag.
> > > 
> > > BUG=819888
> > > 
> > > Change-Id: Ib69909e14f9f8623ac07154ef60be248558b08a4
> > > Reviewed-on: https://chromium-review.googlesource.com/958326
> > > Commit-Queue: Siddhartha S <ssid@chromium.org>
> > > Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> > > Reviewed-by: Primiano Tucci <primiano@chromium.org>
> > > Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> > > Reviewed-by: agrieve <agrieve@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#546343}
> > 
> > TBR=dcheng@chromium.org,primiano@chromium.org,dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org,dskiba@chromium.org
> > 
> > Change-Id: Ib9bcb81ffa30861ea91107c1a1f3ccb8f29a7f7d
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug: 819888
> > Reviewed-on: https://chromium-review.googlesource.com/983972
> > Reviewed-by: François Doray <fdoray@chromium.org>
> > Commit-Queue: François Doray <fdoray@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#546472}
> 
> TBR=dcheng@chromium.org,primiano@chromium.org,fdoray@chromium.org,dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org,dskiba@chromium.org
> 
> Change-Id: Ib39e4563c82264389d7a740a877d27edef343101
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 819888
> Reviewed-on: https://chromium-review.googlesource.com/984092
> Reviewed-by: Henrik Andreasson <henrika@chromium.org>
> Commit-Queue: Henrik Andreasson <henrika@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#546479}

TBR=dcheng@chromium.org,primiano@chromium.org,fdoray@chromium.org,dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org,dskiba@chromium.org,henrika@chromium.org

Change-Id: I1bbb46629fed0b9467482d66f88a548e4d949a24
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 819888
Reviewed-on: https://chromium-review.googlesource.com/984013
Reviewed-by: Peter Mayo <petermayo@chromium.org>
Commit-Queue: Peter Mayo <petermayo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546481}
[modify] https://crrev.com/19a53de61b455f332b946a76da03faa716b9b11e/base/BUILD.gn
[delete] https://crrev.com/5bd2969b2eb60b096b3ab2f7d423c9d1ac8bac8a/base/trace_event/cfi_backtrace_android.cc
[delete] https://crrev.com/5bd2969b2eb60b096b3ab2f7d423c9d1ac8bac8a/base/trace_event/cfi_backtrace_android.h
[delete] https://crrev.com/5bd2969b2eb60b096b3ab2f7d423c9d1ac8bac8a/base/trace_event/cfi_backtrace_android_unittest.cc
[modify] https://crrev.com/19a53de61b455f332b946a76da03faa716b9b11e/build/config/compiler/BUILD.gn
[modify] https://crrev.com/19a53de61b455f332b946a76da03faa716b9b11e/build/config/compiler/compiler.gni

Project Member

Comment 10 by bugdroid1@chromium.org, Mar 28 2018

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

commit a06ad5617853f1370a09145189cd830598ac9d46
Author: Siddhartha <ssid@chromium.org>
Date: Wed Mar 28 21:06:18 2018

Reland "Android: Add code to parse the CFI info added to the apk"

This reverts commit 88328b6800f987ba9f532842862021cb48875618.

Reason for revert: Attempt fixing cfi bot by removing unwind tables in cfi
builds.

Original change's description:
> Revert "Android: Add code to parse the CFI info added to the apk"
>
> This reverts commit 1d8b0c00961571a16a25633be92ed196fbe51271.
>
> Reason for revert:  https://crbug.com/826718 
>
> CL causes failures on Android CFI builder.
>
> Original change's description:
> > Android: Add code to parse the CFI info added to the apk
> >
> > The previous CL added support for adding unwind tables in apk:
> > https://chromium-review.googlesource.com/c/chromium/src/+/956971
> > This CL parses and adds test for unwinding on base_unittests_apk.
> > The non-official builds do not contain debug info since all bots set
> > strip_debug_info=true. So, make sure base_unittests do not take this
> > flag.
> >
> > BUG=819888
> >
> > Change-Id: Ib69909e14f9f8623ac07154ef60be248558b08a4
> > Reviewed-on: https://chromium-review.googlesource.com/958326
> > Commit-Queue: Siddhartha S <ssid@chromium.org>
> > Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> > Reviewed-by: Primiano Tucci <primiano@chromium.org>
> > Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> > Reviewed-by: agrieve <agrieve@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#546343}
>
> TBR=dcheng@chromium.org,primiano@chromium.org,dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org,dskiba@chromium.org
>
> Change-Id: Ib9bcb81ffa30861ea91107c1a1f3ccb8f29a7f7d
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 819888
> Reviewed-on: https://chromium-review.googlesource.com/983972
> Reviewed-by: François Doray <fdoray@chromium.org>
> Commit-Queue: François Doray <fdoray@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#546472}

TBR=dcheng@chromium.org,primiano@chromium.org,fdoray@chromium.org,dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org,dskiba@chromium.org

Bug: 819888
Change-Id: Iadb8825988a311c6576c219b9a67e35b45838d7b
Reviewed-on: https://chromium-review.googlesource.com/984452
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Commit-Queue: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546597}
[modify] https://crrev.com/a06ad5617853f1370a09145189cd830598ac9d46/base/BUILD.gn
[add] https://crrev.com/a06ad5617853f1370a09145189cd830598ac9d46/base/trace_event/cfi_backtrace_android.cc
[add] https://crrev.com/a06ad5617853f1370a09145189cd830598ac9d46/base/trace_event/cfi_backtrace_android.h
[add] https://crrev.com/a06ad5617853f1370a09145189cd830598ac9d46/base/trace_event/cfi_backtrace_android_unittest.cc
[modify] https://crrev.com/a06ad5617853f1370a09145189cd830598ac9d46/build/config/compiler/BUILD.gn
[modify] https://crrev.com/a06ad5617853f1370a09145189cd830598ac9d46/build/config/compiler/compiler.gni

Project Member

Comment 11 by bugdroid1@chromium.org, Mar 29 2018

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

commit 328b9d9e8b0160904575b6a843992c14572a4f06
Author: Tobias Sargeant <tobiasjs@chromium.org>
Date: Thu Mar 29 15:09:07 2018

Revert "Reland "Android: Add code to parse the CFI info added to the apk""

This reverts commit a06ad5617853f1370a09145189cd830598ac9d46.

Reason for revert: crbug.com/827116

Original change's description:
> Reland "Android: Add code to parse the CFI info added to the apk"
> 
> This reverts commit 88328b6800f987ba9f532842862021cb48875618.
> 
> Reason for revert: Attempt fixing cfi bot by removing unwind tables in cfi
> builds.
> 
> Original change's description:
> > Revert "Android: Add code to parse the CFI info added to the apk"
> >
> > This reverts commit 1d8b0c00961571a16a25633be92ed196fbe51271.
> >
> > Reason for revert:  https://crbug.com/826718 
> >
> > CL causes failures on Android CFI builder.
> >
> > Original change's description:
> > > Android: Add code to parse the CFI info added to the apk
> > >
> > > The previous CL added support for adding unwind tables in apk:
> > > https://chromium-review.googlesource.com/c/chromium/src/+/956971
> > > This CL parses and adds test for unwinding on base_unittests_apk.
> > > The non-official builds do not contain debug info since all bots set
> > > strip_debug_info=true. So, make sure base_unittests do not take this
> > > flag.
> > >
> > > BUG=819888
> > >
> > > Change-Id: Ib69909e14f9f8623ac07154ef60be248558b08a4
> > > Reviewed-on: https://chromium-review.googlesource.com/958326
> > > Commit-Queue: Siddhartha S <ssid@chromium.org>
> > > Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> > > Reviewed-by: Primiano Tucci <primiano@chromium.org>
> > > Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> > > Reviewed-by: agrieve <agrieve@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#546343}
> >
> > TBR=dcheng@chromium.org,primiano@chromium.org,dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org,dskiba@chromium.org
> >
> > Change-Id: Ib9bcb81ffa30861ea91107c1a1f3ccb8f29a7f7d
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug: 819888
> > Reviewed-on: https://chromium-review.googlesource.com/983972
> > Reviewed-by: François Doray <fdoray@chromium.org>
> > Commit-Queue: François Doray <fdoray@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#546472}
> 
> TBR=dcheng@chromium.org,primiano@chromium.org,fdoray@chromium.org,dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org,dskiba@chromium.org
> 
> Bug: 819888
> Change-Id: Iadb8825988a311c6576c219b9a67e35b45838d7b
> Reviewed-on: https://chromium-review.googlesource.com/984452
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Reviewed-by: Siddhartha S <ssid@chromium.org>
> Commit-Queue: Siddhartha S <ssid@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#546597}

TBR=dcheng@chromium.org,primiano@chromium.org,fdoray@chromium.org,dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org,dskiba@chromium.org

Change-Id: Ib9dab098899b40b8814fb94c3d239d08eecc714f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 819888
Reviewed-on: https://chromium-review.googlesource.com/986572
Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
Commit-Queue: Tobias Sargeant <tobiasjs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546836}
[modify] https://crrev.com/328b9d9e8b0160904575b6a843992c14572a4f06/base/BUILD.gn
[delete] https://crrev.com/073518139031f312a783850e77d7847f749b2242/base/trace_event/cfi_backtrace_android.cc
[delete] https://crrev.com/073518139031f312a783850e77d7847f749b2242/base/trace_event/cfi_backtrace_android.h
[delete] https://crrev.com/073518139031f312a783850e77d7847f749b2242/base/trace_event/cfi_backtrace_android_unittest.cc
[modify] https://crrev.com/328b9d9e8b0160904575b6a843992c14572a4f06/build/config/compiler/BUILD.gn
[modify] https://crrev.com/328b9d9e8b0160904575b6a843992c14572a4f06/build/config/compiler/compiler.gni

Project Member

Comment 12 by bugdroid1@chromium.org, Mar 29 2018

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

commit 945901d43d375517fe79311b1940589b57addb7d
Author: Siddhartha <ssid@chromium.org>
Date: Thu Mar 29 21:31:30 2018

Reland "Reland "Android: Add code to parse the CFI info added to the apk""

This reverts commit 328b9d9e8b0160904575b6a843992c14572a4f06.

Reason for revert: Test ran on bots for half day. Started failing after more changes in base/.
I guess the extract didn't run because input arg was missing.

Original change's description:
> Revert "Reland "Android: Add code to parse the CFI info added to the apk""
>
> This reverts commit a06ad5617853f1370a09145189cd830598ac9d46.
>
> Reason for revert: crbug.com/827116
>
> Original change's description:
> > Reland "Android: Add code to parse the CFI info added to the apk"
> >
> > This reverts commit 88328b6800f987ba9f532842862021cb48875618.
> >
> > Reason for revert: Attempt fixing cfi bot by removing unwind tables in cfi
> > builds.
> >
> > Original change's description:
> > > Revert "Android: Add code to parse the CFI info added to the apk"
> > >
> > > This reverts commit 1d8b0c00961571a16a25633be92ed196fbe51271.
> > >
> > > Reason for revert:  https://crbug.com/826718 
> > >
> > > CL causes failures on Android CFI builder.
> > >
> > > Original change's description:
> > > > Android: Add code to parse the CFI info added to the apk
> > > >
> > > > The previous CL added support for adding unwind tables in apk:
> > > > https://chromium-review.googlesource.com/c/chromium/src/+/956971
> > > > This CL parses and adds test for unwinding on base_unittests_apk.
> > > > The non-official builds do not contain debug info since all bots set
> > > > strip_debug_info=true. So, make sure base_unittests do not take this
> > > > flag.
> > > >
> > > > BUG=819888
> > > >
> > > > Change-Id: Ib69909e14f9f8623ac07154ef60be248558b08a4
> > > > Reviewed-on: https://chromium-review.googlesource.com/958326
> > > > Commit-Queue: Siddhartha S <ssid@chromium.org>
> > > > Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> > > > Reviewed-by: Primiano Tucci <primiano@chromium.org>
> > > > Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> > > > Reviewed-by: agrieve <agrieve@chromium.org>
> > > > Cr-Commit-Position: refs/heads/master@{#546343}
> > >
> > > TBR=dcheng@chromium.org,primiano@chromium.org,dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org,dskiba@chromium.org
> > >
> > > Change-Id: Ib9bcb81ffa30861ea91107c1a1f3ccb8f29a7f7d
> > > No-Presubmit: true
> > > No-Tree-Checks: true
> > > No-Try: true
> > > Bug: 819888
> > > Reviewed-on: https://chromium-review.googlesource.com/983972
> > > Reviewed-by: François Doray <fdoray@chromium.org>
> > > Commit-Queue: François Doray <fdoray@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#546472}
> >
> > TBR=dcheng@chromium.org,primiano@chromium.org,fdoray@chromium.org,dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org,dskiba@chromium.org
> >
> > Bug: 819888
> > Change-Id: Iadb8825988a311c6576c219b9a67e35b45838d7b
> > Reviewed-on: https://chromium-review.googlesource.com/984452
> > Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> > Reviewed-by: agrieve <agrieve@chromium.org>
> > Reviewed-by: Siddhartha S <ssid@chromium.org>
> > Commit-Queue: Siddhartha S <ssid@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#546597}
>
> TBR=dcheng@chromium.org,primiano@chromium.org,fdoray@chromium.org,dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org,dskiba@chromium.org
>
> Change-Id: Ib9dab098899b40b8814fb94c3d239d08eecc714f
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 819888
> Reviewed-on: https://chromium-review.googlesource.com/986572
> Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
> Commit-Queue: Tobias Sargeant <tobiasjs@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#546836}

TBR=dcheng@chromium.org,primiano@chromium.org,fdoray@chromium.org,dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org,dskiba@chromium.org,tobiasjs@chromium.org

Change-Id: Ieef28dee93a9270e8de3e1f9e1ca7212178c2465
No-Presubmit: true
No-Tree-Checks: true
Bug: 819888
Reviewed-on: https://chromium-review.googlesource.com/985960
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546959}
[modify] https://crrev.com/945901d43d375517fe79311b1940589b57addb7d/base/BUILD.gn
[add] https://crrev.com/945901d43d375517fe79311b1940589b57addb7d/base/trace_event/cfi_backtrace_android.cc
[add] https://crrev.com/945901d43d375517fe79311b1940589b57addb7d/base/trace_event/cfi_backtrace_android.h
[add] https://crrev.com/945901d43d375517fe79311b1940589b57addb7d/base/trace_event/cfi_backtrace_android_unittest.cc
[modify] https://crrev.com/945901d43d375517fe79311b1940589b57addb7d/build/config/android/extract_unwind_tables.gni
[modify] https://crrev.com/945901d43d375517fe79311b1940589b57addb7d/build/config/compiler/BUILD.gn
[modify] https://crrev.com/945901d43d375517fe79311b1940589b57addb7d/build/config/compiler/compiler.gni

Project Member

Comment 13 by bugdroid1@chromium.org, Mar 30 2018

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

commit 0db71a0dbc0636c7281b72d1ab7b59a269c5043a
Author: Siddhartha S <ssid@chromium.org>
Date: Fri Mar 30 17:15:30 2018

Reland "Enable extracting unwind table on official builds without channel"

This reverts commit a2474b6490ea9cfda76528059fbf23369e7d281f.

Reason for revert: The failure was due to parent CL getting reverted
Relanded parent in: https://chromium-review.googlesource.com/c/chromium/src/+/984452

Original change's description:
> Revert "Enable extracting unwind table on official builds without channel"
>
> This reverts commit eb07cea1254cc280dfec0c8ca0e6411dfcceecca.
>
> Reason for revert: Windows tryjobs are failing.
> https://logs.chromium.org/v/?s=chromium%2Fbb%2Ftryserver.chromium.win%2Fwin-msvc-rel%2F81420%2F%2B%2Frecipes%2Fsteps%2Fanalyze%2F0%2Fstdout for an example.
>
> ERROR at //base/BUILD.gn:1802:33: Undefined identifier in string expansion.
>     "CAN_UNWIND_WITH_CFI_TABLE=$can_unwind_with_cfi_table",
>                                 ^------------------------
> "can_unwind_with_cfi_table" is not currently in scope.
> See //BUILD.gn:65:5: which caused the file to be included.
>     "//base:base_perftests",
>     ^----------------------
> GN gen failed: 1
>
> Original change's description:
> > Enable extracting unwind table on official builds without channel
> >
> > This CL enables extraction of unwind tables for chrome when
> > is_official_build is set to true and when android_channel is not set to
> > a release channel. This affects build time only on official builds.
> > Also, Set legacy heap profiler to use unwinder for profiling.
> >
> > TBR=dpranke@chromium.org
> > BUG=819888
> >
> > Change-Id: If9732b5ac1f9dff49c58e88e2ffb6cb387715244
> > Reviewed-on: https://chromium-review.googlesource.com/976944
> > Reviewed-by: Siddhartha S <ssid@chromium.org>
> > Reviewed-by: agrieve <agrieve@chromium.org>
> > Commit-Queue: Siddhartha S <ssid@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#546425}
>
> TBR=dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org
>
> Change-Id: Id033094871dfcf74248f631390f15f379d9cd3c7
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 819888
> Reviewed-on: https://chromium-review.googlesource.com/983893
> Reviewed-by: agrieve <agrieve@chromium.org>
> Commit-Queue: agrieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#546478}

TBR=dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org

Change-Id: I100f540c8d5bc9f59b9631ea207c7f290dc42e96
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 819888
Reviewed-on: https://chromium-review.googlesource.com/985112
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547196}
[modify] https://crrev.com/0db71a0dbc0636c7281b72d1ab7b59a269c5043a/base/BUILD.gn
[modify] https://crrev.com/0db71a0dbc0636c7281b72d1ab7b59a269c5043a/base/trace_event/heap_profiler_allocation_context_tracker.cc
[modify] https://crrev.com/0db71a0dbc0636c7281b72d1ab7b59a269c5043a/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/0db71a0dbc0636c7281b72d1ab7b59a269c5043a/build/config/android/extract_unwind_tables.gni
[modify] https://crrev.com/0db71a0dbc0636c7281b72d1ab7b59a269c5043a/chrome/android/BUILD.gn
[modify] https://crrev.com/0db71a0dbc0636c7281b72d1ab7b59a269c5043a/testing/test.gni

Project Member

Comment 14 by bugdroid1@chromium.org, Mar 30 2018

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

commit 6698460ac408f106815c0f4b43be6b034812c616
Author: Siddhartha <ssid@chromium.org>
Date: Fri Mar 30 17:57:04 2018

Represent Android unwind tables in more compact format

The unwind table format is changed to make it more compact, but still
binary searchable using the file memory map.
The common data for functions are stored in separate table and the index
table only stored index into the second table.
The length of the function is no longer stored, and dummy entries are
added to specify end of functions. Turns out it is more efficient since
most function addresses are continuous.

BUG=819888

Change-Id: I6a43ba10de9be705c7dfb7404e07b3856520d200
Reviewed-on: https://chromium-review.googlesource.com/965441
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547203}
[modify] https://crrev.com/6698460ac408f106815c0f4b43be6b034812c616/base/trace_event/cfi_backtrace_android.cc
[modify] https://crrev.com/6698460ac408f106815c0f4b43be6b034812c616/base/trace_event/cfi_backtrace_android.h
[modify] https://crrev.com/6698460ac408f106815c0f4b43be6b034812c616/base/trace_event/cfi_backtrace_android_unittest.cc
[modify] https://crrev.com/6698460ac408f106815c0f4b43be6b034812c616/build/android/gyp/extract_unwind_tables.py
[modify] https://crrev.com/6698460ac408f106815c0f4b43be6b034812c616/build/android/gyp/extract_unwind_tables_tests.py

Project Member

Comment 15 by bugdroid1@chromium.org, Mar 31 2018

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

commit 2298aca2e030f8d3214c43c06b9cfd03dc18701b
Author: Siddhartha S <ssid@chromium.org>
Date: Sat Mar 31 00:19:13 2018

Revert "Reland "Enable extracting unwind table on official builds without channel""

This reverts commit 0db71a0dbc0636c7281b72d1ab7b59a269c5043a.

Reason for revert: Monochrome apk merge is failing because the unwind
file for both apk has the same name.

Original change's description:
> Reland "Enable extracting unwind table on official builds without channel"
> 
> This reverts commit a2474b6490ea9cfda76528059fbf23369e7d281f.
> 
> Reason for revert: The failure was due to parent CL getting reverted
> Relanded parent in: https://chromium-review.googlesource.com/c/chromium/src/+/984452
> 
> Original change's description:
> > Revert "Enable extracting unwind table on official builds without channel"
> >
> > This reverts commit eb07cea1254cc280dfec0c8ca0e6411dfcceecca.
> >
> > Reason for revert: Windows tryjobs are failing.
> > https://logs.chromium.org/v/?s=chromium%2Fbb%2Ftryserver.chromium.win%2Fwin-msvc-rel%2F81420%2F%2B%2Frecipes%2Fsteps%2Fanalyze%2F0%2Fstdout for an example.
> >
> > ERROR at //base/BUILD.gn:1802:33: Undefined identifier in string expansion.
> >     "CAN_UNWIND_WITH_CFI_TABLE=$can_unwind_with_cfi_table",
> >                                 ^------------------------
> > "can_unwind_with_cfi_table" is not currently in scope.
> > See //BUILD.gn:65:5: which caused the file to be included.
> >     "//base:base_perftests",
> >     ^----------------------
> > GN gen failed: 1
> >
> > Original change's description:
> > > Enable extracting unwind table on official builds without channel
> > >
> > > This CL enables extraction of unwind tables for chrome when
> > > is_official_build is set to true and when android_channel is not set to
> > > a release channel. This affects build time only on official builds.
> > > Also, Set legacy heap profiler to use unwinder for profiling.
> > >
> > > TBR=dpranke@chromium.org
> > > BUG=819888
> > >
> > > Change-Id: If9732b5ac1f9dff49c58e88e2ffb6cb387715244
> > > Reviewed-on: https://chromium-review.googlesource.com/976944
> > > Reviewed-by: Siddhartha S <ssid@chromium.org>
> > > Reviewed-by: agrieve <agrieve@chromium.org>
> > > Commit-Queue: Siddhartha S <ssid@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#546425}
> >
> > TBR=dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org
> >
> > Change-Id: Id033094871dfcf74248f631390f15f379d9cd3c7
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug: 819888
> > Reviewed-on: https://chromium-review.googlesource.com/983893
> > Reviewed-by: agrieve <agrieve@chromium.org>
> > Commit-Queue: agrieve <agrieve@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#546478}
> 
> TBR=dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org
> 
> Change-Id: I100f540c8d5bc9f59b9631ea207c7f290dc42e96
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 819888
> Reviewed-on: https://chromium-review.googlesource.com/985112
> Commit-Queue: Siddhartha S <ssid@chromium.org>
> Reviewed-by: Siddhartha S <ssid@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#547196}

TBR=dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org

Change-Id: Ifa59a5882b18c2634bcf96c2c1fc58b09d7f303f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 819888
Reviewed-on: https://chromium-review.googlesource.com/989153
Reviewed-by: Siddhartha S <ssid@chromium.org>
Commit-Queue: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547332}
[modify] https://crrev.com/2298aca2e030f8d3214c43c06b9cfd03dc18701b/base/BUILD.gn
[modify] https://crrev.com/2298aca2e030f8d3214c43c06b9cfd03dc18701b/base/trace_event/heap_profiler_allocation_context_tracker.cc
[modify] https://crrev.com/2298aca2e030f8d3214c43c06b9cfd03dc18701b/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/2298aca2e030f8d3214c43c06b9cfd03dc18701b/build/config/android/extract_unwind_tables.gni
[modify] https://crrev.com/2298aca2e030f8d3214c43c06b9cfd03dc18701b/chrome/android/BUILD.gn
[modify] https://crrev.com/2298aca2e030f8d3214c43c06b9cfd03dc18701b/testing/test.gni

Project Member

Comment 16 by bugdroid1@chromium.org, Mar 31 2018

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

commit 579e583e477e33ad4eb47aa225ff115c9d022f4d
Author: Siddhartha <ssid@chromium.org>
Date: Sat Mar 31 02:19:25 2018

Reland "Reland "Enable extracting unwind table on official builds without channel""

This reverts commit 2298aca2e030f8d3214c43c06b9cfd03dc18701b.

Reason for revert: Remove changes made in monochrome since apk merge script needs to
be updated first.

Original change's description:
> Revert "Reland "Enable extracting unwind table on official builds without channel""
>
> This reverts commit 0db71a0dbc0636c7281b72d1ab7b59a269c5043a.
>
> Reason for revert: Monochrome apk merge is failing because the unwind
> file for both apk has the same name.
>
> Original change's description:
> > Reland "Enable extracting unwind table on official builds without channel"
> >
> > This reverts commit a2474b6490ea9cfda76528059fbf23369e7d281f.
> >
> > Reason for revert: The failure was due to parent CL getting reverted
> > Relanded parent in: https://chromium-review.googlesource.com/c/chromium/src/+/984452
> >
> > Original change's description:
> > > Revert "Enable extracting unwind table on official builds without channel"
> > >
> > > This reverts commit eb07cea1254cc280dfec0c8ca0e6411dfcceecca.
> > >
> > > Reason for revert: Windows tryjobs are failing.
> > > https://logs.chromium.org/v/?s=chromium%2Fbb%2Ftryserver.chromium.win%2Fwin-msvc-rel%2F81420%2F%2B%2Frecipes%2Fsteps%2Fanalyze%2F0%2Fstdout for an example.
> > >
> > > ERROR at //base/BUILD.gn:1802:33: Undefined identifier in string expansion.
> > >     "CAN_UNWIND_WITH_CFI_TABLE=$can_unwind_with_cfi_table",
> > >                                 ^------------------------
> > > "can_unwind_with_cfi_table" is not currently in scope.
> > > See //BUILD.gn:65:5: which caused the file to be included.
> > >     "//base:base_perftests",
> > >     ^----------------------
> > > GN gen failed: 1
> > >
> > > Original change's description:
> > > > Enable extracting unwind table on official builds without channel
> > > >
> > > > This CL enables extraction of unwind tables for chrome when
> > > > is_official_build is set to true and when android_channel is not set to
> > > > a release channel. This affects build time only on official builds.
> > > > Also, Set legacy heap profiler to use unwinder for profiling.
> > > >
> > > > TBR=dpranke@chromium.org
> > > > BUG=819888
> > > >
> > > > Change-Id: If9732b5ac1f9dff49c58e88e2ffb6cb387715244
> > > > Reviewed-on: https://chromium-review.googlesource.com/976944
> > > > Reviewed-by: Siddhartha S <ssid@chromium.org>
> > > > Reviewed-by: agrieve <agrieve@chromium.org>
> > > > Commit-Queue: Siddhartha S <ssid@chromium.org>
> > > > Cr-Commit-Position: refs/heads/master@{#546425}
> > >
> > > TBR=dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org
> > >
> > > Change-Id: Id033094871dfcf74248f631390f15f379d9cd3c7
> > > No-Presubmit: true
> > > No-Tree-Checks: true
> > > No-Try: true
> > > Bug: 819888
> > > Reviewed-on: https://chromium-review.googlesource.com/983893
> > > Reviewed-by: agrieve <agrieve@chromium.org>
> > > Commit-Queue: agrieve <agrieve@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#546478}
> >
> > TBR=dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org
> >
> > Change-Id: I100f540c8d5bc9f59b9631ea207c7f290dc42e96
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug: 819888
> > Reviewed-on: https://chromium-review.googlesource.com/985112
> > Commit-Queue: Siddhartha S <ssid@chromium.org>
> > Reviewed-by: Siddhartha S <ssid@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#547196}
>
> TBR=dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org
>
> Change-Id: Ifa59a5882b18c2634bcf96c2c1fc58b09d7f303f
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 819888
> Reviewed-on: https://chromium-review.googlesource.com/989153
> Reviewed-by: Siddhartha S <ssid@chromium.org>
> Commit-Queue: Siddhartha S <ssid@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#547332}

TBR=dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org,ctzsm@chromium.org

Change-Id: I7750c70e52b55ccfbf1e2457a19fee4852575e59
Bug: 819888
Reviewed-on: https://chromium-review.googlesource.com/989192
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547361}
[modify] https://crrev.com/579e583e477e33ad4eb47aa225ff115c9d022f4d/base/BUILD.gn
[modify] https://crrev.com/579e583e477e33ad4eb47aa225ff115c9d022f4d/base/trace_event/heap_profiler_allocation_context_tracker.cc
[modify] https://crrev.com/579e583e477e33ad4eb47aa225ff115c9d022f4d/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/579e583e477e33ad4eb47aa225ff115c9d022f4d/build/config/android/extract_unwind_tables.gni
[modify] https://crrev.com/579e583e477e33ad4eb47aa225ff115c9d022f4d/chrome/android/BUILD.gn
[modify] https://crrev.com/579e583e477e33ad4eb47aa225ff115c9d022f4d/testing/test.gni

Project Member

Comment 17 by bugdroid1@chromium.org, Apr 2 2018

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

commit c36e45cb3cde0a857a96aa7e921868f7c51589f3
Author: Siddhartha S <ssid@chromium.org>
Date: Mon Apr 02 19:48:25 2018

Revert "Reland "Reland "Enable extracting unwind table on official builds without channel"""

This reverts commit 579e583e477e33ad4eb47aa225ff115c9d022f4d.

Reason for revert: monochrome_apk_checker.py needs to be updated.
crbug/827937

Original change's description:
> Reland "Reland "Enable extracting unwind table on official builds without channel""
> 
> This reverts commit 2298aca2e030f8d3214c43c06b9cfd03dc18701b.
> 
> Reason for revert: Remove changes made in monochrome since apk merge script needs to
> be updated first.
> 
> Original change's description:
> > Revert "Reland "Enable extracting unwind table on official builds without channel""
> >
> > This reverts commit 0db71a0dbc0636c7281b72d1ab7b59a269c5043a.
> >
> > Reason for revert: Monochrome apk merge is failing because the unwind
> > file for both apk has the same name.
> >
> > Original change's description:
> > > Reland "Enable extracting unwind table on official builds without channel"
> > >
> > > This reverts commit a2474b6490ea9cfda76528059fbf23369e7d281f.
> > >
> > > Reason for revert: The failure was due to parent CL getting reverted
> > > Relanded parent in: https://chromium-review.googlesource.com/c/chromium/src/+/984452
> > >
> > > Original change's description:
> > > > Revert "Enable extracting unwind table on official builds without channel"
> > > >
> > > > This reverts commit eb07cea1254cc280dfec0c8ca0e6411dfcceecca.
> > > >
> > > > Reason for revert: Windows tryjobs are failing.
> > > > https://logs.chromium.org/v/?s=chromium%2Fbb%2Ftryserver.chromium.win%2Fwin-msvc-rel%2F81420%2F%2B%2Frecipes%2Fsteps%2Fanalyze%2F0%2Fstdout for an example.
> > > >
> > > > ERROR at //base/BUILD.gn:1802:33: Undefined identifier in string expansion.
> > > >     "CAN_UNWIND_WITH_CFI_TABLE=$can_unwind_with_cfi_table",
> > > >                                 ^------------------------
> > > > "can_unwind_with_cfi_table" is not currently in scope.
> > > > See //BUILD.gn:65:5: which caused the file to be included.
> > > >     "//base:base_perftests",
> > > >     ^----------------------
> > > > GN gen failed: 1
> > > >
> > > > Original change's description:
> > > > > Enable extracting unwind table on official builds without channel
> > > > >
> > > > > This CL enables extraction of unwind tables for chrome when
> > > > > is_official_build is set to true and when android_channel is not set to
> > > > > a release channel. This affects build time only on official builds.
> > > > > Also, Set legacy heap profiler to use unwinder for profiling.
> > > > >
> > > > > TBR=dpranke@chromium.org
> > > > > BUG=819888
> > > > >
> > > > > Change-Id: If9732b5ac1f9dff49c58e88e2ffb6cb387715244
> > > > > Reviewed-on: https://chromium-review.googlesource.com/976944
> > > > > Reviewed-by: Siddhartha S <ssid@chromium.org>
> > > > > Reviewed-by: agrieve <agrieve@chromium.org>
> > > > > Commit-Queue: Siddhartha S <ssid@chromium.org>
> > > > > Cr-Commit-Position: refs/heads/master@{#546425}
> > > >
> > > > TBR=dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org
> > > >
> > > > Change-Id: Id033094871dfcf74248f631390f15f379d9cd3c7
> > > > No-Presubmit: true
> > > > No-Tree-Checks: true
> > > > No-Try: true
> > > > Bug: 819888
> > > > Reviewed-on: https://chromium-review.googlesource.com/983893
> > > > Reviewed-by: agrieve <agrieve@chromium.org>
> > > > Commit-Queue: agrieve <agrieve@chromium.org>
> > > > Cr-Commit-Position: refs/heads/master@{#546478}
> > >
> > > TBR=dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org
> > >
> > > Change-Id: I100f540c8d5bc9f59b9631ea207c7f290dc42e96
> > > No-Presubmit: true
> > > No-Tree-Checks: true
> > > No-Try: true
> > > Bug: 819888
> > > Reviewed-on: https://chromium-review.googlesource.com/985112
> > > Commit-Queue: Siddhartha S <ssid@chromium.org>
> > > Reviewed-by: Siddhartha S <ssid@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#547196}
> >
> > TBR=dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org
> >
> > Change-Id: Ifa59a5882b18c2634bcf96c2c1fc58b09d7f303f
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug: 819888
> > Reviewed-on: https://chromium-review.googlesource.com/989153
> > Reviewed-by: Siddhartha S <ssid@chromium.org>
> > Commit-Queue: Siddhartha S <ssid@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#547332}
> 
> TBR=dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org,ctzsm@chromium.org
> 
> Change-Id: I7750c70e52b55ccfbf1e2457a19fee4852575e59
> Bug: 819888
> Reviewed-on: https://chromium-review.googlesource.com/989192
> Commit-Queue: Siddhartha S <ssid@chromium.org>
> Reviewed-by: Siddhartha S <ssid@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#547361}

TBR=dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org,ctzsm@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 819888
Change-Id: I064a5bdae54fa1552eee14ee72d08f15cf081a63
Reviewed-on: https://chromium-review.googlesource.com/990092
Reviewed-by: Siddhartha S <ssid@chromium.org>
Commit-Queue: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547490}
[modify] https://crrev.com/c36e45cb3cde0a857a96aa7e921868f7c51589f3/base/BUILD.gn
[modify] https://crrev.com/c36e45cb3cde0a857a96aa7e921868f7c51589f3/base/trace_event/heap_profiler_allocation_context_tracker.cc
[modify] https://crrev.com/c36e45cb3cde0a857a96aa7e921868f7c51589f3/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/c36e45cb3cde0a857a96aa7e921868f7c51589f3/build/config/android/extract_unwind_tables.gni
[modify] https://crrev.com/c36e45cb3cde0a857a96aa7e921868f7c51589f3/chrome/android/BUILD.gn
[modify] https://crrev.com/c36e45cb3cde0a857a96aa7e921868f7c51589f3/testing/test.gni

Project Member

Comment 18 by bugdroid1@chromium.org, Apr 3 2018

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

commit 56c8ecc0d6f80a9e641cb691f9afe232d4fc7c9f
Author: Siddhartha <ssid@chromium.org>
Date: Tue Apr 03 22:44:20 2018

Represent UNW_INDEX table column-wise for better search locality

The UNW_INDEX table has 2 columns: address and index. This table is
represented column wise so that binary search uses lesser number of
pages and it is easier to represent the memory map as arrays of
primitive types.

BUG=819888

Change-Id: I7f3245cfc1526425aed414fe89ee01661b914a40
Reviewed-on: https://chromium-review.googlesource.com/985151
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547855}
[modify] https://crrev.com/56c8ecc0d6f80a9e641cb691f9afe232d4fc7c9f/base/trace_event/cfi_backtrace_android.cc
[modify] https://crrev.com/56c8ecc0d6f80a9e641cb691f9afe232d4fc7c9f/base/trace_event/cfi_backtrace_android.h
[modify] https://crrev.com/56c8ecc0d6f80a9e641cb691f9afe232d4fc7c9f/base/trace_event/cfi_backtrace_android_unittest.cc
[modify] https://crrev.com/56c8ecc0d6f80a9e641cb691f9afe232d4fc7c9f/build/android/gyp/extract_unwind_tables.py
[modify] https://crrev.com/56c8ecc0d6f80a9e641cb691f9afe232d4fc7c9f/build/android/gyp/extract_unwind_tables_tests.py

Project Member

Comment 19 by bugdroid1@chromium.org, Apr 4 2018

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

commit 16e808d789d05fcc2bfcc8b00e002d11405adb1a
Author: Siddhartha <ssid@chromium.org>
Date: Wed Apr 04 06:00:46 2018

Enable extracting unwind table on official builds without channel

The original cl was here:
https://chromium-review.googlesource.com/c/chromium/src/+/990092
This CL fixes the following problems with the original CL:
1. The apk_merger script fails because the unwind tables were only added
   in 32-bit apk. The merger script expects all the files to be same and
   the ones different should be checked.
 1a. The resources.arsc is non-hermetic and ordering is affected by
     adding file to only one apk. As a workaround for crbug/828528,
     add an empty (valid) unwind table file to the 64 bit monochrome
     apk to make the resource.arsc consistent.
 1b. The merger script simply adds all the files in apk which are not
     same. To keep the script simple and functional, the unwind resource
     is renamed to unwind_cfi_32 and unwind_cfi_empty in respective
     builds and the app_merger is updated to specify this file is
     expected to be different and included. This causes an extra file
     (4 byte) in the merged apk.

2. The unwind tables were always generated for "libchrome.so" for all
   chrome apks. The different chrome_apk(s) have different shared
   libraries like libchromefortest, etc.. So, update the unwind asset to
   get unwind table for the right library for each apk. Only adds assets
   to *_public_apk(s).

3. The monochrome_apk_checker was failing because the unwind file
   included was different in chrome_apk and monochrome_apk. This CL adds
   the asset to all apk at the same time and adds exception for this
   file.

BUG=819888
TBR=dpranke@chromium.org

Change-Id: Ibceeeacc19fa424d519891b8c17e349ee6c2dfd6
Reviewed-on: https://chromium-review.googlesource.com/991236
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547993}
[modify] https://crrev.com/16e808d789d05fcc2bfcc8b00e002d11405adb1a/android_webview/tools/apk_merger.py
[modify] https://crrev.com/16e808d789d05fcc2bfcc8b00e002d11405adb1a/base/BUILD.gn
[modify] https://crrev.com/16e808d789d05fcc2bfcc8b00e002d11405adb1a/base/trace_event/cfi_backtrace_android.cc
[modify] https://crrev.com/16e808d789d05fcc2bfcc8b00e002d11405adb1a/base/trace_event/heap_profiler_allocation_context_tracker.cc
[modify] https://crrev.com/16e808d789d05fcc2bfcc8b00e002d11405adb1a/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/16e808d789d05fcc2bfcc8b00e002d11405adb1a/build/android/gyp/extract_unwind_tables.py
[modify] https://crrev.com/16e808d789d05fcc2bfcc8b00e002d11405adb1a/build/config/android/extract_unwind_tables.gni
[modify] https://crrev.com/16e808d789d05fcc2bfcc8b00e002d11405adb1a/chrome/android/BUILD.gn
[modify] https://crrev.com/16e808d789d05fcc2bfcc8b00e002d11405adb1a/chrome/android/chrome_public_apk_tmpl.gni
[modify] https://crrev.com/16e808d789d05fcc2bfcc8b00e002d11405adb1a/chrome/android/monochrome/scripts/monochrome_apk_checker.py
[modify] https://crrev.com/16e808d789d05fcc2bfcc8b00e002d11405adb1a/testing/test.gni

Project Member

Comment 20 by bugdroid1@chromium.org, Apr 4 2018

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

commit ef18e86a1e65f29a7415964aec310d9b40619b39
Author: Anthony Berent <aberent@chromium.org>
Date: Wed Apr 04 15:25:36 2018

Revert "Enable extracting unwind table on official builds without channel"

This reverts commit 16e808d789d05fcc2bfcc8b00e002d11405adb1a.

Reason for revert: Merge step still failing. 

Original change's description:
> Enable extracting unwind table on official builds without channel
> 
> The original cl was here:
> https://chromium-review.googlesource.com/c/chromium/src/+/990092
> This CL fixes the following problems with the original CL:
> 1. The apk_merger script fails because the unwind tables were only added
>    in 32-bit apk. The merger script expects all the files to be same and
>    the ones different should be checked.
>  1a. The resources.arsc is non-hermetic and ordering is affected by
>      adding file to only one apk. As a workaround for crbug/828528,
>      add an empty (valid) unwind table file to the 64 bit monochrome
>      apk to make the resource.arsc consistent.
>  1b. The merger script simply adds all the files in apk which are not
>      same. To keep the script simple and functional, the unwind resource
>      is renamed to unwind_cfi_32 and unwind_cfi_empty in respective
>      builds and the app_merger is updated to specify this file is
>      expected to be different and included. This causes an extra file
>      (4 byte) in the merged apk.
> 
> 2. The unwind tables were always generated for "libchrome.so" for all
>    chrome apks. The different chrome_apk(s) have different shared
>    libraries like libchromefortest, etc.. So, update the unwind asset to
>    get unwind table for the right library for each apk. Only adds assets
>    to *_public_apk(s).
> 
> 3. The monochrome_apk_checker was failing because the unwind file
>    included was different in chrome_apk and monochrome_apk. This CL adds
>    the asset to all apk at the same time and adds exception for this
>    file.
> 
> BUG=819888
> TBR=dpranke@chromium.org
> 
> Change-Id: Ibceeeacc19fa424d519891b8c17e349ee6c2dfd6
> Reviewed-on: https://chromium-review.googlesource.com/991236
> Commit-Queue: Siddhartha S <ssid@chromium.org>
> Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
> Reviewed-by: Bo <boliu@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#547993}

TBR=boliu@chromium.org,dpranke@chromium.org,mariakhomenko@chromium.org,changwan@chromium.org,ssid@chromium.org,agrieve@chromium.org

Change-Id: I0a96e213133b6cb21c36db365b7c72f0f4642c8e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 819888
Bug: 828879
Reviewed-on: https://chromium-review.googlesource.com/995697
Reviewed-by: Anthony Berent <aberent@chromium.org>
Commit-Queue: Anthony Berent <aberent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548066}
[modify] https://crrev.com/ef18e86a1e65f29a7415964aec310d9b40619b39/android_webview/tools/apk_merger.py
[modify] https://crrev.com/ef18e86a1e65f29a7415964aec310d9b40619b39/base/BUILD.gn
[modify] https://crrev.com/ef18e86a1e65f29a7415964aec310d9b40619b39/base/trace_event/cfi_backtrace_android.cc
[modify] https://crrev.com/ef18e86a1e65f29a7415964aec310d9b40619b39/base/trace_event/heap_profiler_allocation_context_tracker.cc
[modify] https://crrev.com/ef18e86a1e65f29a7415964aec310d9b40619b39/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/ef18e86a1e65f29a7415964aec310d9b40619b39/build/android/gyp/extract_unwind_tables.py
[modify] https://crrev.com/ef18e86a1e65f29a7415964aec310d9b40619b39/build/config/android/extract_unwind_tables.gni
[modify] https://crrev.com/ef18e86a1e65f29a7415964aec310d9b40619b39/chrome/android/BUILD.gn
[modify] https://crrev.com/ef18e86a1e65f29a7415964aec310d9b40619b39/chrome/android/chrome_public_apk_tmpl.gni
[modify] https://crrev.com/ef18e86a1e65f29a7415964aec310d9b40619b39/chrome/android/monochrome/scripts/monochrome_apk_checker.py
[modify] https://crrev.com/ef18e86a1e65f29a7415964aec310d9b40619b39/testing/test.gni

Project Member

Comment 21 by bugdroid1@chromium.org, Apr 4 2018

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

commit 9b8952c8bb4a010f858bdb7efff762d41bad99bc
Author: Andrew Grieve <agrieve@chromium.org>
Date: Wed Apr 04 16:38:31 2018

Teach resource_sizes about unwind_cfi

Bug: 819888
Change-Id: I648353f7f4fa6790f63d4d4eca711c1d48d2092e
Reviewed-on: https://chromium-review.googlesource.com/995585
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: Peter Wen <wnwen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548092}
[modify] https://crrev.com/9b8952c8bb4a010f858bdb7efff762d41bad99bc/build/android/resource_sizes.py

Project Member

Comment 22 by bugdroid1@chromium.org, Apr 4 2018

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

commit 6c58c0c66be9b01d330d6bd3661ca3effbd25df1
Author: Siddhartha <ssid@chromium.org>
Date: Wed Apr 04 23:37:03 2018

Reland "Enable extracting unwind table on official builds without channel"

This reverts commit ef18e86a1e65f29a7415964aec310d9b40619b39.

Reason for revert: Make sure apk_merger.py works with and without the
unwind file, till downstream cl lands. Also rebase on:
https://chromium-review.googlesource.com/c/chromium/src/+/994545
and remove workaround for crbug/828528.

Original change's description:
> Revert "Enable extracting unwind table on official builds without channel"
>
> This reverts commit 16e808d789d05fcc2bfcc8b00e002d11405adb1a.
>
> Reason for revert: Merge step still failing.
>
> Original change's description:
> > Enable extracting unwind table on official builds without channel
> >
> > The original cl was here:
> > https://chromium-review.googlesource.com/c/chromium/src/+/990092
> > This CL fixes the following problems with the original CL:
> > 1. The apk_merger script fails because the unwind tables were only added
> >    in 32-bit apk. The merger script expects all the files to be same and
> >    the ones different should be checked.
> >  1a. The resources.arsc is non-hermetic and ordering is affected by
> >      adding file to only one apk. As a workaround for crbug/828528,
> >      add an empty (valid) unwind table file to the 64 bit monochrome
> >      apk to make the resource.arsc consistent.
> >  1b. The merger script simply adds all the files in apk which are not
> >      same. To keep the script simple and functional, the unwind resource
> >      is renamed to unwind_cfi_32 and unwind_cfi_empty in respective
> >      builds and the app_merger is updated to specify this file is
> >      expected to be different and included. This causes an extra file
> >      (4 byte) in the merged apk.
> >
> > 2. The unwind tables were always generated for "libchrome.so" for all
> >    chrome apks. The different chrome_apk(s) have different shared
> >    libraries like libchromefortest, etc.. So, update the unwind asset to
> >    get unwind table for the right library for each apk. Only adds assets
> >    to *_public_apk(s).
> >
> > 3. The monochrome_apk_checker was failing because the unwind file
> >    included was different in chrome_apk and monochrome_apk. This CL adds
> >    the asset to all apk at the same time and adds exception for this
> >    file.
> >
> > BUG=819888
> > TBR=dpranke@chromium.org
> >
> > Change-Id: Ibceeeacc19fa424d519891b8c17e349ee6c2dfd6
> > Reviewed-on: https://chromium-review.googlesource.com/991236
> > Commit-Queue: Siddhartha S <ssid@chromium.org>
> > Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
> > Reviewed-by: Bo <boliu@chromium.org>
> > Reviewed-by: agrieve <agrieve@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#547993}
>
> TBR=boliu@chromium.org,dpranke@chromium.org,mariakhomenko@chromium.org,changwan@chromium.org,ssid@chromium.org,agrieve@chromium.org
>
> Change-Id: I0a96e213133b6cb21c36db365b7c72f0f4642c8e
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 819888
> Bug: 828879
> Reviewed-on: https://chromium-review.googlesource.com/995697
> Reviewed-by: Anthony Berent <aberent@chromium.org>
> Commit-Queue: Anthony Berent <aberent@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#548066}

TBR=dpranke@chromium.org,mariakhomenko@chromium.org

Bug: 819888, 828879
Change-Id: I2eba81de32632bea90171ece4cba1a4144c55d25
Reviewed-on: https://chromium-review.googlesource.com/996272
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548249}
[modify] https://crrev.com/6c58c0c66be9b01d330d6bd3661ca3effbd25df1/android_webview/tools/apk_merger.py
[modify] https://crrev.com/6c58c0c66be9b01d330d6bd3661ca3effbd25df1/base/BUILD.gn
[modify] https://crrev.com/6c58c0c66be9b01d330d6bd3661ca3effbd25df1/base/trace_event/heap_profiler_allocation_context_tracker.cc
[modify] https://crrev.com/6c58c0c66be9b01d330d6bd3661ca3effbd25df1/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/6c58c0c66be9b01d330d6bd3661ca3effbd25df1/build/config/android/extract_unwind_tables.gni
[modify] https://crrev.com/6c58c0c66be9b01d330d6bd3661ca3effbd25df1/chrome/android/BUILD.gn
[modify] https://crrev.com/6c58c0c66be9b01d330d6bd3661ca3effbd25df1/chrome/android/chrome_public_apk_tmpl.gni
[modify] https://crrev.com/6c58c0c66be9b01d330d6bd3661ca3effbd25df1/chrome/android/monochrome/scripts/monochrome_apk_checker.py
[modify] https://crrev.com/6c58c0c66be9b01d330d6bd3661ca3effbd25df1/testing/test.gni

Project Member

Comment 23 by bugdroid1@chromium.org, Apr 5 2018

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

commit 6b20e46523014c755704f827190538203cb03bb5
Author: Siddhartha <ssid@chromium.org>
Date: Thu Apr 05 21:51:05 2018

Add a thread local cache for CFI table unwinder

Adds a simple cache with prime modulo hashing. This cache gets 95% hit
rate and gives us 30% performance improvement compared to without cache
for running heap profiler.

BUG=819888

Change-Id: I4c3dd6cf34ee1db21469e56886eaa3f62dd1881d
Reviewed-on: https://chromium-review.googlesource.com/985234
Reviewed-by: Dmitry Skiba <dskiba@chromium.org>
Commit-Queue: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548570}
[modify] https://crrev.com/6b20e46523014c755704f827190538203cb03bb5/base/trace_event/cfi_backtrace_android.cc
[modify] https://crrev.com/6b20e46523014c755704f827190538203cb03bb5/base/trace_event/cfi_backtrace_android.h
[modify] https://crrev.com/6b20e46523014c755704f827190538203cb03bb5/base/trace_event/cfi_backtrace_android_unittest.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Apr 5 2018

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/e466189a3dd6b95eeb08fd7c9097252d8d544445

commit e466189a3dd6b95eeb08fd7c9097252d8d544445
Author: Siddhartha <ssid@google.com>
Date: Thu Apr 05 22:28:24 2018

Project Member

Comment 25 by bugdroid1@chromium.org, Apr 6 2018

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

commit 83ba87f975448f2dd6a1ca3ca1c5c7fad780a9bf
Author: Siddhartha <ssid@chromium.org>
Date: Fri Apr 06 19:21:55 2018

Use CFI stack unwinder in memlog profiler

The CFI stack unwinder is used to unwind stack frames for memlog
profiler in android official builds. It is used only in official builds
since the unwind table is only added in official builds.

BUG=819888

Change-Id: I81c14806063b031522a415eaf2ab2c57c3cd8886
Reviewed-on: https://chromium-review.googlesource.com/988815
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548894}
[modify] https://crrev.com/83ba87f975448f2dd6a1ca3ca1c5c7fad780a9bf/base/trace_event/cfi_backtrace_android.cc
[modify] https://crrev.com/83ba87f975448f2dd6a1ca3ca1c5c7fad780a9bf/base/trace_event/cfi_backtrace_android.h
[modify] https://crrev.com/83ba87f975448f2dd6a1ca3ca1c5c7fad780a9bf/base/trace_event/cfi_backtrace_android_unittest.cc
[modify] https://crrev.com/83ba87f975448f2dd6a1ca3ca1c5c7fad780a9bf/base/trace_event/heap_profiler_allocation_context_tracker.cc
[modify] https://crrev.com/83ba87f975448f2dd6a1ca3ca1c5c7fad780a9bf/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/83ba87f975448f2dd6a1ca3ca1c5c7fad780a9bf/chrome/browser/profiling_host/profiling_test_driver.cc
[modify] https://crrev.com/83ba87f975448f2dd6a1ca3ca1c5c7fad780a9bf/components/services/heap_profiling/public/cpp/allocator_shim.cc
[modify] https://crrev.com/83ba87f975448f2dd6a1ca3ca1c5c7fad780a9bf/components/services/heap_profiling/public/cpp/client.cc
[modify] https://crrev.com/83ba87f975448f2dd6a1ca3ca1c5c7fad780a9bf/components/services/heap_profiling/public/cpp/client.h

Project Member

Comment 26 by bugdroid1@chromium.org, Apr 6 2018

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

commit 99f9e4912742ced2d5cd519763e4f81a32ace27d
Author: Siddhartha <ssid@chromium.org>
Date: Fri Apr 06 22:46:07 2018

Rename unwind asset to specify it's only for 32-bit apk

This makes the apk_merger script cleaner.

BUG=819888

Change-Id: Ica38505114a459326b8b0be6fd59e2fa6636c84e
Reviewed-on: https://chromium-review.googlesource.com/999152
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548952}
[modify] https://crrev.com/99f9e4912742ced2d5cd519763e4f81a32ace27d/android_webview/tools/apk_merger.py
[modify] https://crrev.com/99f9e4912742ced2d5cd519763e4f81a32ace27d/base/trace_event/cfi_backtrace_android.cc
[modify] https://crrev.com/99f9e4912742ced2d5cd519763e4f81a32ace27d/build/config/android/extract_unwind_tables.gni
[modify] https://crrev.com/99f9e4912742ced2d5cd519763e4f81a32ace27d/chrome/android/monochrome/scripts/monochrome_apk_checker.py

Project Member

Comment 27 by bugdroid1@chromium.org, Apr 19 2018

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

commit 6fb1cc8a3f9e16af06ea33d2785bee0b76f425b2
Author: Siddhartha <ssid@chromium.org>
Date: Thu Apr 19 21:53:09 2018

Fix apk_merger.py to include unwind_cfi instead of ignoring

The unwind file is included in some channels and not in other channels.
It is only added to 32-bit apk since 64-bit ones have frame pointers.
Add use-unwind-cfi argument to use the file when needed.

BUG=819888

Change-Id: Ia4560815b2c7ef0e5ba852c8a31f7b132d86c11c
Reviewed-on: https://chromium-review.googlesource.com/998309
Reviewed-by: Tao Bai <michaelbai@chromium.org>
Commit-Queue: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552173}
[modify] https://crrev.com/6fb1cc8a3f9e16af06ea33d2785bee0b76f425b2/android_webview/tools/apk_merger.py

Project Member

Comment 28 by bugdroid1@chromium.org, Apr 19 2018

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/9b3a1e4e952a6cddb2a1bbf72596e92a112f4f16

commit 9b3a1e4e952a6cddb2a1bbf72596e92a112f4f16
Author: Siddhartha <ssid@google.com>
Date: Thu Apr 19 22:52:29 2018

Project Member

Comment 29 by bugdroid1@chromium.org, Apr 20 2018

Project Member

Comment 30 by bugdroid1@chromium.org, Apr 20 2018

Project Member

Comment 31 by bugdroid1@chromium.org, Apr 27 2018

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/5e7f0484c1cc1a0f457bc27636fe603772c293d6

commit 5e7f0484c1cc1a0f457bc27636fe603772c293d6
Author: Siddhartha <ssid@google.com>
Date: Fri Apr 27 20:21:26 2018

Project Member

Comment 32 by bugdroid1@chromium.org, Jun 6 2018

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/02815fd20bba83a481a572bde9ae6644e3c9c85b

commit 02815fd20bba83a481a572bde9ae6644e3c9c85b
Author: Siddhartha <ssid@google.com>
Date: Wed Jun 06 20:18:46 2018

When I try to change symbol_level of android arm builder, unwinding tests fails.
Is this intentional?
https://chromium-review.googlesource.com/c/chromium/src/+/1104280/2
Status: Assigned (was: Untriaged)
This bug has an owner, thus, it's been triaged. Changing status to "assigned".

Sign in to add a comment