> 2. The library uses relocation packing, which messes up relative address calculation.
I don't remember if we ever implemented this, but I remember suggesting in the past, in several occasions, to dump the address of a known symbol and use the diff(absolute_addr - known_symbol) to compute the actual relative address, precisely to avoid weirdness of  relro packing.

> 1. The library (libmonochrome.so) is mapped directly from the apk, so it shows as base.apk in smaps.
This knowledge could be just hardcoded in the symbolizing script to be honest. The alternative is to do what breakpad does and extract the ELF name from the mapped APK, but that's really overkill given our usecase.


>But dumping phdr information (from dl_iterate_phdrs) will provide enough information for proper symbolization:
I'd probably not go here and simply hardcode the knowledge (when you see an apk look at the lib(mono)chrome.so symbol) in the symbolizer script.

I feel that's too much hardcodings. And we can't actually hardcode #1, because there are at least two flavors (Chrome vs Monochrome).

What do you think is wrong with phdr approach?

> I feel that's too much hardcodings.
Like: if (name like "chrome.*.apk") then look_for("libchrome.so", "libmonochrome.so") ? is that "too much hardcodings" ?

>What do you think is wrong with phdr approach?

Because I saw how painful and how mnany iterations it took to get it right on breakpad. See git log of:
breakpad/src/client/linux/minidump_writer/linux_dumper.cc

Also, not sure dl_iterate_phdr() will just work. I can't remember why, but we discussed that back in the days when somebody was writing that ElfFileSoNameFromMappedFile in linux_dumper.cc. I remember they mentioned dl_iterate_phdr() but I can't remember why we ended up not using that (maybe doesn't work with the crazylinker?)

What if both libraries present? What about component builds?

dl_iterate_phdr() works (with modern linker at least), I tried it before filing this issue. My idea is to dump that information under process_mmaps node, which right now ha's a single child, "vm_regions". This looks like a perfect place: phdr info complements memory maps, and just as with memory maps, libraries can come and go, so phdr info needs to be dumped periodically.

> What if both libraries present? 
Does that happen for real? Is there any chrome configuration that can have both monochrome.so and libchrome.so? I am not aware of that.

> What about component builds?
AFAIK component builds don't use mmap from apk.

From: base/android/linker/config.gni
chromium_linker_supported = !is_component_build && !enable_profiling && !use_order_profiling && !is_asan

And from chrome/android/BUILD.gn
load_library_from_apk = chromium_linker_supported


> dl_iterate_phdr() works (with modern linker at least), I tried it before filing this issue. 
This is my honest thought. I trust you that you tried and works, but the use of dl_iterate_phdr() is unprecedentet in android and is an engineer risk because plays in the dangerous crazylinker area and we had lot of bugs about that (especially consiering that we have at least 3 different loading mechanism depending on the version of android).
Considering that we are discussing to enable this in the field at some point, I really want to avoid risking to landing this, forgetting about that, and then spending days of debugging some months from now trying to figure out why we have crashes from the field.

simply, this is not a trivial function, it has risks associated with it and I don't see a strong benefit in taking this risk as the alternative option seems very simple and straightforward.

Alternatively, if you really don't like speculating in the script, a reasonable and safer tradeoff would be:
- have the build system to define the name of the target library (the build system knows the final name of the library)
- have const char kAndroidSoName = BUILDFLAG(ANDROID_SO_NAME); in a .cc file

FWIW, I also leaning towards not using dl_iterate_phdrs (for reliability reasons primiano@ mentioned and the little control we have over fixing it). Also hardcoding library name/path in the symbolizer script sounds fragile to me (it depends on the CPU platform, for example).

The build-time path to the DSO relative to the APK is easy to make, I am planning to do something like this in LibraryLoader.java at the moment: https://chromium-review.googlesource.com/c/chromium/src/+/809046

.. and yes, we use the C preprocessor for Java, in case you were wondering :)

BTW, regarding dl_iterate_phdrs - we would only need it for Monochrome, i.e. when crazy linker is not used.

+agrieve@ to confirm the following:

"
The relro compression no longer causes "layout skew", since it's now done by lld directly (yay!). We can probably go through and delete all the offset fix-up logic for it now.
"

If this is the case, then the only thing we need is proper mapping between regions and files.

Correct! There's no longer anything special about our relocations.

+digit has been working on this as of late.

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

commit 61248f1ac6be47bcbd6012749f046eaf57bdc007
Author: Siddhartha <ssid@chromium.org>
Date: Fri Apr 27 04:38:07 2018

Add utils to read elf binary and get Build id

Adds code to parse elf binary to get build id from
.note.gnu.build-id section. The build ID is needed to symbolize
heap dumps from the users.

BUG=734705

Change-Id: If4365e232c060ba96071ba1e1c43618f9807e39c
Reviewed-on: https://chromium-review.googlesource.com/1028995
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554304}
[modify] https://crrev.com/61248f1ac6be47bcbd6012749f046eaf57bdc007/base/BUILD.gn
[add] https://crrev.com/61248f1ac6be47bcbd6012749f046eaf57bdc007/base/debug/elf_reader_linux.cc
[add] https://crrev.com/61248f1ac6be47bcbd6012749f046eaf57bdc007/base/debug/elf_reader_linux.h
[add] https://crrev.com/61248f1ac6be47bcbd6012749f046eaf57bdc007/base/debug/elf_reader_linux_unittest.cc

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

commit cc0afbf73869a90f09a0f244a303799d880a0b00
Author: Thomas Anderson <thomasanderson@chromium.org>
Date: Fri Apr 27 22:26:41 2018

Revert "Add utils to read elf binary and get Build id"

This reverts commit 61248f1ac6be47bcbd6012749f046eaf57bdc007.

Reason for revert: Suspected breaking component builds ( bug 837817 )

Original change's description:
> Add utils to read elf binary and get Build id
> 
> Adds code to parse elf binary to get build id from
> .note.gnu.build-id section. The build ID is needed to symbolize
> heap dumps from the users.
> 
> BUG=734705
> 
> Change-Id: If4365e232c060ba96071ba1e1c43618f9807e39c
> Reviewed-on: https://chromium-review.googlesource.com/1028995
> Commit-Queue: Siddhartha S <ssid@chromium.org>
> Reviewed-by: Lei Zhang <thestig@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#554304}

TBR=thestig@chromium.org,ssid@chromium.org

Change-Id: If512434b55bcc912990e3997a86f6f93722b15fb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 734705
Reviewed-on: https://chromium-review.googlesource.com/1033795
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554532}
[modify] https://crrev.com/cc0afbf73869a90f09a0f244a303799d880a0b00/base/BUILD.gn
[delete] https://crrev.com/697a2e68c91d94318128f5409d78209bd883f215/base/debug/elf_reader_linux.cc
[delete] https://crrev.com/697a2e68c91d94318128f5409d78209bd883f215/base/debug/elf_reader_linux.h
[delete] https://crrev.com/697a2e68c91d94318128f5409d78209bd883f215/base/debug/elf_reader_linux_unittest.cc

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

commit 04ec9b02e0485b9fd94de8689d0ca8b923c1ea9c
Author: Siddhartha <ssid@chromium.org>
Date: Sat Apr 28 01:31:25 2018

Reland "Add utils to read elf binary and get Build id"

This reverts commit cc0afbf73869a90f09a0f244a303799d880a0b00.

Reason for revert: Move the variable to test file and use it only on
official builds. Both Chromeos component and official build works.

Original change's description:
> Revert "Add utils to read elf binary and get Build id"
>
> This reverts commit 61248f1ac6be47bcbd6012749f046eaf57bdc007.
>
> Reason for revert: Suspected breaking component builds ( bug 837817 )
>
> Original change's description:
> > Add utils to read elf binary and get Build id
> >
> > Adds code to parse elf binary to get build id from
> > .note.gnu.build-id section. The build ID is needed to symbolize
> > heap dumps from the users.
> >
> > BUG=734705
> >
> > Change-Id: If4365e232c060ba96071ba1e1c43618f9807e39c
> > Reviewed-on: https://chromium-review.googlesource.com/1028995
> > Commit-Queue: Siddhartha S <ssid@chromium.org>
> > Reviewed-by: Lei Zhang <thestig@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#554304}
>
> TBR=thestig@chromium.org,ssid@chromium.org
>
> Change-Id: If512434b55bcc912990e3997a86f6f93722b15fb
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 734705
> Reviewed-on: https://chromium-review.googlesource.com/1033795
> Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
> Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#554532}

TBR=thestig@chromium.org,ssid@chromium.org,thomasanderson@chromium.org

Change-Id: Ie2fb8f7fe0759dc418a1bc4ffc57bdb4542ba606
Bug: 734705,  837817 
Reviewed-on: https://chromium-review.googlesource.com/1033498
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554601}
[modify] https://crrev.com/04ec9b02e0485b9fd94de8689d0ca8b923c1ea9c/base/BUILD.gn
[add] https://crrev.com/04ec9b02e0485b9fd94de8689d0ca8b923c1ea9c/base/debug/elf_reader_linux.cc
[add] https://crrev.com/04ec9b02e0485b9fd94de8689d0ca8b923c1ea9c/base/debug/elf_reader_linux.h
[add] https://crrev.com/04ec9b02e0485b9fd94de8689d0ca8b923c1ea9c/base/debug/elf_reader_linux_unittest.cc

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

commit b7b4a541dc81ed20a32d086c23d57889e74ab1e4
Author: Siddhartha <ssid@chromium.org>
Date: Sat Apr 28 06:47:21 2018

Add util for reading library name from elf binary

1. Reading elf for the current binary could lead to seg fault if the
   full library is not loaded in memory. So, the util only takes mapped
   address and works on it. The caller must ensure the library is
   mapped.
2. Add function to read soname from the .dynamic section in elf file.
3. The Linux executable does not contain so name. So, use malloc_wrapper
   library to test.

BUG=734705

Change-Id: I1327f4786726143daba57a2aaa1eddd608143c04
Reviewed-on: https://chromium-review.googlesource.com/1031854
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554635}
[modify] https://crrev.com/b7b4a541dc81ed20a32d086c23d57889e74ab1e4/base/BUILD.gn
[modify] https://crrev.com/b7b4a541dc81ed20a32d086c23d57889e74ab1e4/base/debug/elf_reader_linux.cc
[modify] https://crrev.com/b7b4a541dc81ed20a32d086c23d57889e74ab1e4/base/debug/elf_reader_linux.h
[modify] https://crrev.com/b7b4a541dc81ed20a32d086c23d57889e74ab1e4/base/debug/elf_reader_linux_unittest.cc

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

commit f575203535340c959309b4d5354b48a08e52c014
Author: Siddhartha <ssid@chromium.org>
Date: Tue May 01 01:52:43 2018

Add library name as metadata in trace file for Android

The library name cannot be obtained from process maps since the library
can be mapped from apk directly. The name is taken by reading the elf
sections and added as metadata in trace file.
This is useful for symbolizing traces.

BUG=734705

Change-Id: I72761c3dc60fc45a9284e0d527929b4f3483eff2
Reviewed-on: https://chromium-review.googlesource.com/1035789
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554966}
[modify] https://crrev.com/f575203535340c959309b4d5354b48a08e52c014/content/browser/tracing/tracing_controller_impl.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/504f52bbe5b9899d2c76de0f0c0ab37d54e65908

commit 504f52bbe5b9899d2c76de0f0c0ab37d54e65908
Author: Siddhartha <ssid@chromium.org>
Date: Thu May 03 19:32:34 2018

Fix symbolization of heap profiles when so is mapped from apk

The path points to apk rather than the so when library is mapped from
apk directly. So, use the metadata of library name and remap the chrome
apk names to chrome library.

BUG=chromium:734705

Change-Id: Ib8a1b2aadad6505d1796abe1974f9580384fa894
Reviewed-on: https://chromium-review.googlesource.com/1036790
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Siddhartha S <ssid@chromium.org>

[modify] https://crrev.com/504f52bbe5b9899d2c76de0f0c0ab37d54e65908/tracing/tracing/extras/symbolizer/symbolize_trace.py

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

commit f3cb54067e379373eebb8a58399e36f9046a7c35
Author: catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri May 04 00:44:42 2018

Roll src/third_party/catapult/ 19282cf9d..153134ef2 (4 commits)

https://chromium.googlesource.com/catapult.git/+log/19282cf9d32d..153134ef26c3

$ git log 19282cf9d..153134ef2 --date=short --no-merges --format='%ad %ae %s'
2018-05-03 dtu [pinpoint] Increase task queue rate.
2018-05-03 szager Add support for profiling on Android using simpleperf
2018-05-02 ssid Fix symbolization of heap profiles when so is mapped from apk
2018-05-03 nednguyen Fix cache_temperature_unittes to use working pages from test archives

Created with:
  roll-dep src/third_party/catapult
BUG=chromium:734705, chromium:839127 


The AutoRoll server is located here: https://catapult-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


TBR=sullivan@chromium.org

Change-Id: I97dd890c7eda177ae8b603b830e60bfbb215da98
Reviewed-on: https://chromium-review.googlesource.com/1043079
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#555933}
[modify] https://crrev.com/f3cb54067e379373eebb8a58399e36f9046a7c35/DEPS

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

commit 42995a35d04a84eafe12fc4b3f13e7dd15fec4a7
Author: Siddhartha <ssid@chromium.org>
Date: Tue May 08 20:37:02 2018

MemoryInfra: Add module debug ID for chrome.so on Linux and Android

Bug: 734705
Change-Id: I4ea5cfb28ff531353fc78db0f7d248bd8e963170
Reviewed-on: https://chromium-review.googlesource.com/1036535
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Hector Dearman <hjd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556949}
[modify] https://crrev.com/42995a35d04a84eafe12fc4b3f13e7dd15fec4a7/services/resource_coordinator/public/cpp/memory_instrumentation/os_metrics_linux.cc