What question does supersize want to answer when it calls readelf on the archive?

`ar -t` can list the contents of a thin archive; you could then operate on the thus-listed .o files directly.

Note that llvm-readelf can read them fine, but we're avoiding that due to it not having the same output as readelf:
https://bugs.llvm.org/show_bug.cgi?id=35351 (https://bugs.llvm.org/show_bug.cgi?id=35351)

I think we'd want to operate on the .o file directly anyways though, since we'll need that later when extracting strings.

Re #1 - we're using readelf on the .o files to figure out what string literals exist in each one.

Supersize applies 'readelf -S' on .a files to get a text dump of headers of embedded .o files, and parses the section names to extract string sections:

https://cs.chromium.org/chromium/src/tools/binary_size/libsupersize/nm.py?q=readelf&l=186

Not sure if there are other uses?

After discussion with estevenson@ offline, seems like `ar -t` should work for our purposes since we'll then operate on the .o files.

Thanks for the analysis, though Andrew should get some sleep.

Perhaps a secondary issue that stems from this is that we should get alerts for when the perf bot fails continuously.

Here is the failing bot: https://ci.chromium.org/buildbot/chromium.perf/Android%20Builder%20Perf/1186

 Issue 830614  has been merged into this issue.

'ar -t' lists the .o names without paths - any chance there's a way to have 'ar' list the paths, or will I need to do some path mangling to make this work?

Issue 830955 has been merged into this issue.

I'm not close to a linux box, so I can't try things. From https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=binutils/ar.c;h=66a81e70c44bc11a26d93d6208ebe3ddf8fea46b;hb=HEAD , `-t -v` and `-p` and `-p -v` might be worth a try, but from the man page on `P` maybe ar intentionally doesn't do much with qualified paths ("not posix compliant").

We need the M67 build tonight, please revert this in the meantime so we have a dev build to push tomorrow.

After updating my repo and rebuilding chrome just using ar -t was giving me full paths - not sure what changed to make that happen.. 

Ran into other issues while trying to fix supersize; I don't think we'll be able to get this fixed by tonight. Any reason not to revert 40a8d84a54ac47d3299194c5377aebd71714f1f0? We could also disable supersize on the bots but reverting is probably easier.

"any reason?" It'll make the builds of all devs slower again, and it only broke off waterfall bots would be two reasons :-) But I agree it sounds like reverting for a day sounds like the right call, if y'all can fix this tomorrow so we can reland.

Wait, better: Add a !is_android here: https://cs.chromium.org/chromium/src/build/config/compiler/BUILD.gn?type=cs&q=thin_archive+file:%5C.gn+file:config&sq=package:chromium&l=1559

Fix is here: https://chromium-review.googlesource.com/c/chromium/src/+/1003999, but we already missed the dev build sadly: https://uberchromegw.corp.google.com/i/official.android/builders/official-arm/builds/3089..

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

commit 0c15ca3a9f9dcf37ef37bb0d2b8540fbb2414977
Author: Eric Stevenson <estevenson@chromium.org>
Date: Tue Apr 10 02:56:01 2018

Disable thin archive builds on Android.

Temporary fix while support is added to supersize.

Bug:  830843 
Change-Id: I677782d52c9927ed08d2a07c5272c18a0901e3d6
Reviewed-on: https://chromium-review.googlesource.com/1003999
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549397}
[modify] https://crrev.com/0c15ca3a9f9dcf37ef37bb0d2b8540fbb2414977/build/config/compiler/BUILD.gn

Please cp to branch 3393

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

commit 21e2705d777a9768a790c44ce321f166ad32a4b8
Author: Eric Stevenson <estevenson@chromium.org>
Date: Tue Apr 10 03:25:50 2018

Disable thin archive builds on Android.

Temporary fix while support is added to supersize.

Bug:  830843 
Change-Id: I677782d52c9927ed08d2a07c5272c18a0901e3d6
Reviewed-on: https://chromium-review.googlesource.com/1003999
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#549397}(cherry picked from commit 0c15ca3a9f9dcf37ef37bb0d2b8540fbb2414977)
Reviewed-on: https://chromium-review.googlesource.com/1004172
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Cr-Commit-Position: refs/branch-heads/3393@{#1}
Cr-Branched-From: 3298a607d7e8b39d8426591a1edc48b8cf7eaa7a-refs/heads/master@{#549377}
[modify] https://crrev.com/21e2705d777a9768a790c44ce321f166ad32a4b8/build/config/compiler/BUILD.gn

Hmm, I'm not really sure how to go about operating on the .o files directly..
* We still need to support archives unless we want to add .o -> .a mapping to nm.py. 'ar -t file.a' only shows the full path for thin archives and we have a number of prebuilt archives that aren't thin.
* We could just transform thin archives into full archives (temporarily), but this would be slow
* We'll also need to update a few other places that assume archives are valid 

Is the only reason not to use llvm-readelf this bug: https://bugs.llvm.org/show_bug.cgi?id=35351 ? I might give this route a try and have 'ar -t' provide the file path for thin archives.

That was the only reason to not use llvm-readelf. I'm sure it wouldn't be that hard to fix, and is a good fix to make anyways :P.

I noticed the _IterArchiveChunks() method is also broken for thin archives, and will need to be fixed regardless of llvm-readelf. I don't think it'd be hard to fix (just poorly implemented in the first place).

If fixed, this function could be used to pre-process all thin .a paths that turn out to be thin archives to their actual .o paths (rather than using ar -t).

Here's what I should have used as a reference rather than stack overflow for the file format:
https://github.com/pathscale/binutils/blob/3ab3398d33836b938e83e5441ee48ec59e6e5a69/gold/archive.cc#L159
https://github.com/pathscale/binutils/blob/3ab3398d33836b938e83e5441ee48ec59e6e5a69/gold/archive.cc#L486


Note that there is existing logic for mapping foo/bar.a(baz.o) -> actual/baz.o in ninja_parser.py:
https://cs.chromium.org/chromium/src/tools/binary_size/libsupersize/ninja_parser.py?rcl=a264029c44afb5fde5d61767090b9a1d30e11c89&l=35

But I don't think we should use it. It would mean breaking the --no-source-paths flow.

Sounds like a good plan to me.

I have a couple other more pressing things I'm trying to get done asap so I'm going to leave this until next week (someone else can feel free to grab it in the meantime otherwise).

I'll take a go at it.

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

commit 6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b
Author: Andrew Grieve <agrieve@chromium.org>
Date: Thu Apr 12 11:10:28 2018

SuperSize: Teach it about thin archives

"supersize archive" would crash upon encountering a .a file that was
linked as a "thin archive" (where the actual .o files are not embedded).

This fixes the tool by transforming all foo.a(bar.o) paths into their
external object paths.

Also adds main() functions for ninja_parser.py for debugging.

Bug:  830843 
Change-Id: Iddc374565b8e94a4a89b2faa343256ea2d9f569e
Reviewed-on: https://chromium-review.googlesource.com/1007317
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550141}
[modify] https://crrev.com/6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b/tools/binary_size/README.md
[add] https://crrev.com/6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b/tools/binary_size/libsupersize/ar.py
[modify] https://crrev.com/6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b/tools/binary_size/libsupersize/archive.py
[modify] https://crrev.com/6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b/tools/binary_size/libsupersize/ninja_parser.py
[modify] https://crrev.com/6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b/tools/binary_size/libsupersize/nm.py
[modify] https://crrev.com/6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b/tools/binary_size/libsupersize/testdata/Archive.golden
[modify] https://crrev.com/6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b/tools/binary_size/libsupersize/testdata/Archive_Apk.golden
[modify] https://crrev.com/6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b/tools/binary_size/libsupersize/testdata/Archive_Elf.golden
[modify] https://crrev.com/6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b/tools/binary_size/libsupersize/testdata/Archive_OutputDirectory.golden
[modify] https://crrev.com/6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b/tools/binary_size/libsupersize/testdata/Archive_Pak_Files.golden
[modify] https://crrev.com/6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b/tools/binary_size/libsupersize/testdata/Console.golden
[modify] https://crrev.com/6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b/tools/binary_size/libsupersize/testdata/Diff_Basic.golden
[modify] https://crrev.com/6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b/tools/binary_size/libsupersize/testdata/FullDescription.golden
[add] https://crrev.com/6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b/tools/binary_size/libsupersize/testdata/mock_output_directory/obj/third_party/WebKit.a
[add] https://crrev.com/6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b/tools/binary_size/libsupersize/testdata/mock_output_directory/obj/third_party/ffmpeg/libffmpeg_internal.a
[modify] https://crrev.com/6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b/tools/binary_size/libsupersize/testdata/mock_toolchain/mock_nm.py
[modify] https://crrev.com/6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b/tools/binary_size/libsupersize/testdata/test.map
[add] https://crrev.com/6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b/tools/binary_size/libsupersize/third_party/gvr-android-sdk/libgvr_shim_static_arm.a

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

commit 34025b52fc27bafc9be247ff3c66c6edf69cfdad
Author: agrieve <agrieve@chromium.org>
Date: Thu Apr 12 16:33:18 2018

Revert "Disable thin archive builds on Android."

This reverts commit 0c15ca3a9f9dcf37ef37bb0d2b8540fbb2414977.

Reason for revert: Supersize now deals with thin archives as of
6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b

Original change's description:
> Disable thin archive builds on Android.
> 
> Temporary fix while support is added to supersize.
> 
> Bug:  830843 
> Change-Id: I677782d52c9927ed08d2a07c5272c18a0901e3d6
> Reviewed-on: https://chromium-review.googlesource.com/1003999
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Commit-Queue: Eric Stevenson <estevenson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#549397}

TBR=thakis@chromium.org,estevenson@chromium.org

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

Bug:  830843 
Change-Id: I10a8c141be3808b7095f686c44127f7a102abb7d
Reviewed-on: https://chromium-review.googlesource.com/1010322
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550235}
[modify] https://crrev.com/34025b52fc27bafc9be247ff3c66c6edf69cfdad/build/config/compiler/BUILD.gn

Thanks Andrew!

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

commit 34025b52fc27bafc9be247ff3c66c6edf69cfdad
Author: agrieve <agrieve@chromium.org>
Date: Thu Apr 12 16:33:18 2018

Revert "Disable thin archive builds on Android."

This reverts commit 0c15ca3a9f9dcf37ef37bb0d2b8540fbb2414977.

Reason for revert: Supersize now deals with thin archives as of
6a15a6c45c47ff99bd66a73dc65d1c81a4a5557b

Original change's description:
> Disable thin archive builds on Android.
> 
> Temporary fix while support is added to supersize.
> 
> Bug:  830843 
> Change-Id: I677782d52c9927ed08d2a07c5272c18a0901e3d6
> Reviewed-on: https://chromium-review.googlesource.com/1003999
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Commit-Queue: Eric Stevenson <estevenson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#549397}

TBR=thakis@chromium.org,estevenson@chromium.org

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

Bug:  830843 
Change-Id: I10a8c141be3808b7095f686c44127f7a102abb7d
Reviewed-on: https://chromium-review.googlesource.com/1010322
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550235}
[modify] https://crrev.com/34025b52fc27bafc9be247ff3c66c6edf69cfdad/build/config/compiler/BUILD.gn

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

commit fe94207b7d8e8fb14437529b39be1303db24d173
Author: Samuel Huang <huangs@chromium.org>
Date: Tue May 01 16:16:17 2018

[Supersize] Fix Gold flow under thin archive usage.

After thin archive was enabled by default, Supersize support was added
in:
  https://chromium-review.googlesource.com/c/chromium/src/+/1007317 .

However, this only supports the (currently default) LLD flow. For the
legacy Gold flow (still is needed for porting features), the .map file
from thin archive would have lines looking like
  obj/base/libbase.a(obj/base/base/callback_internal.o)
instead of (for non-thin archive)
  obj/base/libbase.a(callback_internal.o)

This causes non-existant paths like
  obj/base/obj/base/base/callback_internal.o
to be used, leading to failure. This CL fixes the bug by making
CreateThinObjectPath() detect and handle the newly encountered .map
format, so Gold flow continues to work.

Bug:  830843 
Change-Id: Ie04c72d1b5411568481ef6e9ff6457b1e4c78d64
Reviewed-on: https://chromium-review.googlesource.com/1035605
Reviewed-by: Samuel Huang <huangs@chromium.org>
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Reviewed-by: Peter Wen <wnwen@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555058}
[modify] https://crrev.com/fe94207b7d8e8fb14437529b39be1303db24d173/tools/binary_size/libsupersize/ar.py