New issue
Advanced search Search tips

Issue 723798 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 25
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 717550

Blocking:
issue 469376



Sign in to add a comment

Support lto and lld in supersize

Project Member Reported by estevenson@chromium.org, May 17 2017

Issue description

Supersize doesn't work on Linux right now with the default build config (i.e. just setting is_official_build = true). The problem is that the generated linker map file and object files aren't in the expected format. The main issues are:

1) lto.tmp paths - ll paths are reported as "lto.tmp" paths. We can probably get around that via parsing .o files but it would be nice if the linker map file contained this information.

2) Size 0 symbols in linker map file, ex:
00000000024bfe80 0000000000009dbc    64                 obj/third_party/boringssl/boringssl_asm/chacha20_poly1305_x86_64.o
00000000024bff40 0000000000000000     0                         .and_masks
00000000024bff00 0000000000000000     0                         .avx2_inc
00000000024bfee0 0000000000000000     0                         .avx2_init
00000000024bfe80 0000000000000000     0                         .chacha20_consts
00000000024bff20 0000000000000000     0                         .clamp 
00000000024bfec0 0000000000000000     0                         .rol16 
00000000024bfea0 0000000000000000     0                         .rol8  
00000000024bfef0 0000000000000000     0                         .sse_inc
00000000024bfe80 0000000000000000     0                         chacha20_poly1305_constants
00000000024c44c0 000000000000268d     0                         chacha20_poly1305_open_avx2
00000000024c6b80 0000000000000000     0                         chacha20_poly1305_seal_avx2
00000000024c00e8 0000000000000000     0                         hash_ad_loop
00000000024c018b 0000000000000000     0                         hash_ad_tail
00000000024c01a1 0000000000000000     0                         hash_ad_tail_loop
00000000024c63ae 0000000000000000     0                         open_avx2_192
00000000024c67fb 0000000000000000     0                         open_avx2_320
00000000024c65dd 0000000000000000     0                         open_avx2_hash_and_xor_loop
00000000024c65d5 0000000000000000     0                         open_avx2_short
00000000024c673f 0000000000000000     0                         open_avx2_short_tail_32
00000000024c637e 0000000000000000     0                         open_avx2_tail
00000000024c6355 0000000000000000     0                         open_avx2_tail_loop
00000000024c1feb 0000000000000000     0                         open_sse_128
00000000024c1f19 0000000000000000     0                         open_sse_finalize
00000000024c0414 0000000000000000     0                         open_sse_main_loop
00000000024c1e3f 0000000000000000     0                         open_sse_tail_16
00000000024c1e0d 0000000000000000     0                         open_sse_tail_64_dec_loop
00000000024c0053 0000000000000000     0                         poly_fast_tls_ad
00000000024c0040 0000000000000207     0                         poly_hash_ad_internal
00000000024c974d 0000000000000000     0                         seal_avx2_192
00000000024c93fb 0000000000000000     0                         seal_avx2_320
00000000024c997f 0000000000000000     0                         seal_avx2_hash
00000000024c9974 0000000000000000     0                         seal_avx2_short
00000000024c9a22 0000000000000000     0                         seal_avx2_short_loop

3) running nm file.o on any .o file generated when lto is used results in "nm: *.o: File format not recognized", likely due to the generated bitcode format
 
Blockedon: 717550
Labels: Performance-Size

Comment 3 by p...@chromium.org, May 19 2017

Cc: p...@chromium.org
> 1) lto.tmp paths - ll paths are reported as "lto.tmp" paths. We can probably get around that via parsing .o files but it would be nice if the linker map file contained this information.

Right now with regular LTO we lose the mapping from input file names to symbol names because the linker internally creates one large object file ("lto.tmp") for all the inputs. With LTO it isn't really possible to accurately attribute symbol contents to input files because of inlining, but you can sort of approximate it by looking at the input files, as you mention.

> 2) Size 0 symbols in linker map file, ex:

This is not an lld specific issue. Symbol sizes in the map file are taken from object files, and symbol sizes in object files are not guaranteed to be correct. In general you can expect them to be accurate, but if the object was built from assembly they might not be.

The section size (which is displayed alongside the object file name) needs to be correct for the linker to work, so you can rely on that to be accurate.

> 3) running nm file.o on any .o file generated when lto is used results in "nm: *.o: File format not recognized", likely due to the generated bitcode format

LLVM has an llvm-nm tool that is like nm but can understand bitcode, but it isn't part of chromium's clang release. If you just want to experiment locally you could try building LLVM yourself (run "tools/clang/scripts/update.py --force-local-build") and then you should find llvm-nm under third_party/llvm-build/Release+Asserts/bin/.
Project Member

Comment 4 by bugdroid1@chromium.org, May 23 2017

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

commit a0d07deeaa973b547aab5cc88918bb15432f093d
Author: estevenson <estevenson@chromium.org>
Date: Tue May 23 16:04:25 2017

diagnose_bloat.py: add missing GN arg for local Linux builds.

The following args are required to generate map files that work with
//tools/binary_size/supersize on Linux:
  * is_official_build = true
  * allow_posix_link_time_opt = false
  * generate_linker_map = true

BUG= 723798 

Review-Url: https://codereview.chromium.org/2896633002
Cr-Commit-Position: refs/heads/master@{#473921}

[modify] https://crrev.com/a0d07deeaa973b547aab5cc88918bb15432f093d/tools/binary_size/diagnose_bloat.py

Owner: hua...@chromium.org
Should be able to set:
 use_lld = true

in gn args to test out lld map files in an android build (so that .map files can be parsed separately from having to worry about lto)

Comment 7 by hua...@chromium.org, Sep 27 2017

I noticed that thinlto-cache/llvmcache-* are ELF files, and the second string in the .debug_str section points to a .cc or .cpp source file!

Maybe I can use the .map file to link from sumbol to llvmcache-* file, and parse the files to extract the source file?  Meanwhile, the llvmcache-* can act as a replacement to .o file when dumping object_path?
Are you thinking about how to solve the lto path problem? Or is the linker map missing full paths even without lto?

We already basically throw away the linker map file .o path afterwards anyways:
https://cs.chromium.org/chromium/src/tools/binary_size/libsupersize/archive.py?rcl=59eb83d050230233ecb928357846718898624c1c&l=522

The reason is that for inline functions, the map file only lists a single .o file as having it, but in reality, it exists in ever .o that references the inline function and the linker throws away duplicate copies.
I was thinking about LTO, but that's premature now since we should start with LLD.

Re. llvm-nm: Thanks for the remark; I was able to build it. This is likely the best way forward. Is it possible to have this built by default?

Comment 10 by p...@chromium.org, Oct 5 2017

Cc: thakis@chromium.org
+thakis for thoughts on shipping llvm-nm.
The usual suggestion: make package.py upload it to somewhere, and make supersize download that. Not in main zip.

Comment 12 by huangs@google.com, Oct 6 2017

Right now I want to hack up a working prototype, so will get back to llvm-nm availability after.

The current issue is that the GN switch "use_thin_lto" might go away. Let:

  (1) = LLD without LTO
  (2) = LLD + thin LTO

Originally I thought I should do Supersize for (1) first, so (2) can build on it. However:

- The .map file from (2) has more information (refers to thinlto-cache/llvmcache-* files).
- If (1) disappears, there may be wasted work to have Supersize support it.

So it would be great to know for sure, whether there's a possibility to keep (1) around?

Project Member

Comment 13 by bugdroid1@chromium.org, Nov 8 2017

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

commit eee7cb628ad1f62544b6b4f95f0a2f72447673b0
Author: Samuel Huang <huangs@chromium.org>
Date: Wed Nov 08 22:58:38 2017

[Supersize] Add limited LLD support.

This CL implements a limited prototype for LLD support in Supersize, for
Linux chromium. Full support will require the following:
1. Make LLVM tools available and automatically included.
2. Add string literal extraction.
3. Add LTO support.

Current work-around:
- For (1): Manually build LLVM tools by:
    cd $CHROMIUM_SRC
    tools/clang/scripts/update.py --force-local-build
  and when running Supersize archive, add switch:
    --tool-prefix $CHROMIUM_SRC/third_party/llvm-build/Release+Asserts/bin/llvm-
- For (2): When running Supersize archive, add switch:
    --no-string-literals
- For (3): Disable LLD in build by adding the following in args.gn:
  use_lld = true
  use_gold = false
  use_thin_lto = false
  is_cfi = false

Follow-up CLs will address these issues, and focus on Clank.

Also, in nm.py 'W' is added as relevant nm name. This is needed for
LLD, but does not affect current Supersize archive for Clank linked
using Gold.

Bug:  723798 
Change-Id: Iac6b9d42b47127798a27f38d6a369d279e15652d
Reviewed-on: https://chromium-review.googlesource.com/759090
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514986}
[modify] https://crrev.com/eee7cb628ad1f62544b6b4f95f0a2f72447673b0/tools/binary_size/libsupersize/linker_map_parser.py
[modify] https://crrev.com/eee7cb628ad1f62544b6b4f95f0a2f72447673b0/tools/binary_size/libsupersize/nm.py

Project Member

Comment 14 by bugdroid1@chromium.org, Nov 11 2017

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

commit 5a4fface689c2459e219998faafbd942d1fa4af0
Author: Samuel Huang <huangs@chromium.org>
Date: Sat Nov 11 02:27:09 2017

[Clang] Add tools needed by Supersize to the llvmobjdump package.

This CL adds the following to llvmobjdump-$REV.tgz:
- {llvm-cxxfilt, llvm-nm, llvm-objdump, llvm-readobj},
- If non-Windows, sym link llvm-readelf -> llvm-readobj.
These are needed by Supersize to process LLD builds.

We didn't add symlink llvm-c++filt -> llvm-cxxfilt because that's not
built by default.

Next steps:
- Rebuild package and upload (or wait for next push).
- Update Supersize to look for the package by default (if needed),
  and download accordingly.

Bug:  723798 
Change-Id: I1991f6aee1e51e9ce9ac61faea97660cc40f640a
Reviewed-on: https://chromium-review.googlesource.com/763728
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515803}
[modify] https://crrev.com/5a4fface689c2459e219998faafbd942d1fa4af0/tools/clang/scripts/package.py

Project Member

Comment 15 by bugdroid1@chromium.org, Nov 14 2017

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

commit 82fbfa3c1e52688d48e3968fdfd9cf33f1b915b6
Author: Samuel Huang <huangs@chromium.org>
Date: Tue Nov 14 22:52:05 2017

[Supersize] Refactor: Split LazyPath to decouple output_directory and tool_prefix finding.

This CL replaces LazyPath with 3 classes:
- _PathFinder: Interface for finding a specific path or path prefix.
- OutputDirectoryFinder: _PathFinder implementation to find output
  directory.
- ToolPrefixFinder: _PathFinder implementation to find tool-prefix.

This split enables follow-up work to parse .map to deduce the linker,
and automatically assign default tool-prefix (with LLD in mind). In
particular, the data flow:
  output_directory_finder -> elf_path -> map_path -> (linker_name)
                          -> tool_prefix_finder
is made non-circular (unlike the old code when LazyPath encompasses
output_directory_finder and tool_prefix_finder).

Bug:  723798 
Change-Id: I641e9a52d982167d14a5987230216c720086d4be
Reviewed-on: https://chromium-review.googlesource.com/769356
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516477}
[modify] https://crrev.com/82fbfa3c1e52688d48e3968fdfd9cf33f1b915b6/tools/binary_size/libsupersize/archive.py
[modify] https://crrev.com/82fbfa3c1e52688d48e3968fdfd9cf33f1b915b6/tools/binary_size/libsupersize/console.py
[modify] https://crrev.com/82fbfa3c1e52688d48e3968fdfd9cf33f1b915b6/tools/binary_size/libsupersize/path_util.py

Project Member

Comment 16 by bugdroid1@chromium.org, Nov 15 2017

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

commit b8601602654c080e3194ffa2cb12e5f8a7c10c1e
Author: Samuel Huang <huangs@chromium.org>
Date: Wed Nov 15 16:42:48 2017

[Supersize] Fix LLD merged data handling; add linker detection and set LLD default tool-prefix.

Details:
- Fix a bug from https://chromium-review.googlesource.com/759090 that
  inverts '<internal>' handling logic.
- Supersize archive: Use .map file to heuristically detect linker
  (Gold or LLD). If LLD, set default tool-prefix to use Chromium build's
  LLVM binaries.
- path_util.py: Change SRC_ROOT evaluation to enable override by
  environment variable $CHECKOUT_SOURCE_ROOT .

Bug:  723798 
Change-Id: I368b0c2e10c50976ed4f5405c485611de03e1172
Reviewed-on: https://chromium-review.googlesource.com/770736
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516718}
[modify] https://crrev.com/b8601602654c080e3194ffa2cb12e5f8a7c10c1e/tools/binary_size/libsupersize/archive.py
[modify] https://crrev.com/b8601602654c080e3194ffa2cb12e5f8a7c10c1e/tools/binary_size/libsupersize/linker_map_parser.py
[modify] https://crrev.com/b8601602654c080e3194ffa2cb12e5f8a7c10c1e/tools/binary_size/libsupersize/path_util.py

Project Member

Comment 17 by bugdroid1@chromium.org, Nov 17 2017

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

commit 4a3004841fad245fb87cc4ffc06aede0d1c89f86
Author: Samuel Huang <huangs@chromium.org>
Date: Fri Nov 17 19:36:10 2017

[Clang] Add download_objdump.py script.

This CL adds download_objdump.py to make available binutil tools
that dump and analyze binary files. Currently the main user of
these tools is Supersize.

Bug:  723798 
Change-Id: I9427f75d81538835ddb431ca7044398126ba9d11
Reviewed-on: https://chromium-review.googlesource.com/766728
Reviewed-by: Peter Collingbourne <pcc@chromium.org>
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517514}
[add] https://crrev.com/4a3004841fad245fb87cc4ffc06aede0d1c89f86/tools/clang/scripts/download_objdump.py
[modify] https://crrev.com/4a3004841fad245fb87cc4ffc06aede0d1c89f86/tools/clang/scripts/package.py
[modify] https://crrev.com/4a3004841fad245fb87cc4ffc06aede0d1c89f86/tools/clang/scripts/update.py

Project Member

Comment 18 by bugdroid1@chromium.org, Nov 20 2017

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

commit 91eaa1ed39188151662afb09c7a358517cd69d89
Author: Samuel Huang <huangs@chromium.org>
Date: Mon Nov 20 03:43:53 2017

[Supersize] Add preliminary string literal extraction under LLD.

Previously Supersize archive for binaries linked with LLD (no LTO) did
not support string literals, and required '--no-string-literals' to
avoid triggering assert. This CL fixes this this by adding preliminary
support for merged strings. Wrinkles:
- LLD .map files does not discern string literals from data literals
  (and testing align=1 seems insufficient), so we count everything as
  string literals for now.
- llvm-readobj (AKA llvm-readelf)'s dump for .a files is missing
  'File: libray.a(file.o)' output. For now we work around this by
  using regular readelf instead.

Bug:  723798 
Change-Id: Ic0c9a48dfd9ab157ed19471127a5216ee98b8d65
Reviewed-on: https://chromium-review.googlesource.com/777441
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517752}
[modify] https://crrev.com/91eaa1ed39188151662afb09c7a358517cd69d89/tools/binary_size/libsupersize/archive.py
[modify] https://crrev.com/91eaa1ed39188151662afb09c7a358517cd69d89/tools/binary_size/libsupersize/linker_map_parser.py
[modify] https://crrev.com/91eaa1ed39188151662afb09c7a358517cd69d89/tools/binary_size/libsupersize/path_util.py

Project Member

Comment 19 by bugdroid1@chromium.org, Nov 23 2017

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

commit 0228d124cccdd6eb73d6dc459eb0b2dffac36eaf
Author: Samuel Huang <huangs@chromium.org>
Date: Thu Nov 23 15:29:46 2017

[Supersize] Normalize object paths parsed from .map files.

When searching for source_path from object_path, we already reject
object_path starting with '../'. However, this does not handle the
prefix './../', which recently showed up (for Zucchini). This CL
addresses this by apply os.path.normpath() to object_path right
after extraction from .map files.

For completeness it seems we should also apply os.path.normpath() to
keys of source_mapper, which is parsed from build.ninja. However,
it seems all .a and .o entries are already normalized in build.ninja,
so looks like we don't need to do this.

Also updating tests.

Bug:  723798 
Change-Id: I9c3fee92383bed9624552c99be167635bb1a1a07
Reviewed-on: https://chromium-review.googlesource.com/785890
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518943}
[modify] https://crrev.com/0228d124cccdd6eb73d6dc459eb0b2dffac36eaf/tools/binary_size/libsupersize/integration_test.py
[modify] https://crrev.com/0228d124cccdd6eb73d6dc459eb0b2dffac36eaf/tools/binary_size/libsupersize/linker_map_parser.py
[modify] https://crrev.com/0228d124cccdd6eb73d6dc459eb0b2dffac36eaf/tools/binary_size/libsupersize/testdata/mock_toolchain/mock_nm.py
[modify] https://crrev.com/0228d124cccdd6eb73d6dc459eb0b2dffac36eaf/tools/binary_size/libsupersize/testdata/mock_toolchain/mock_readelf.py
[modify] https://crrev.com/0228d124cccdd6eb73d6dc459eb0b2dffac36eaf/tools/binary_size/libsupersize/testdata/test.map

Project Member

Comment 20 by bugdroid1@chromium.org, Nov 27 2017

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

commit 2b57b5cde8d2e119b8715af46cf00d0c920f8221
Author: Andrew Grieve <agrieve@chromium.org>
Date: Mon Nov 27 15:50:38 2017

Supersize: Fix supersize archive when use_lld=true

When run on libchrome.so, it was never finishing. This change removes
$t.22 from nm output (as opposed to just $t), and also ignores
zero-length entries in map file (that look like $t.12, or $d.23).

Bug:  723798 
Change-Id: I48badcb20a2c7137fdc8251ff75447ca7afc9fac
Reviewed-on: https://chromium-review.googlesource.com/789442
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519307}
[modify] https://crrev.com/2b57b5cde8d2e119b8715af46cf00d0c920f8221/tools/binary_size/libsupersize/linker_map_parser.py
[modify] https://crrev.com/2b57b5cde8d2e119b8715af46cf00d0c920f8221/tools/binary_size/libsupersize/nm.py
[modify] https://crrev.com/2b57b5cde8d2e119b8715af46cf00d0c920f8221/tools/binary_size/libsupersize/testdata/mock_toolchain/mock_nm.py

Project Member

Comment 21 by bugdroid1@chromium.org, Nov 28 2017

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

commit a6c2ffd7124f1a67808652bda101249796c52bac
Author: Samuel Huang <huangs@chromium.org>
Date: Tue Nov 28 22:11:14 2017

[Supersize] Add strict check for missing tool-prefix.

When Supersize assigns tool-prefix by default, but fails to find
the tools needed, previously it quietly falls back to using other
tools. This CL changes makes this a hard failure instead. For
LLVM, also prints instruction on how to download the required
tools, now that download_objdump.py is ready.

Bug:  723798 
Change-Id: Icc7e02deca9800dc7fbf30fbe6314a98c019be25
Reviewed-on: https://chromium-review.googlesource.com/794104
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519865}
[modify] https://crrev.com/a6c2ffd7124f1a67808652bda101249796c52bac/tools/binary_size/libsupersize/path_util.py

Project Member

Comment 22 by bugdroid1@chromium.org, Nov 30 2017

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

commit 2f62b7ef8e7d6a7b765206323df3b7eadc8af638
Author: Samuel Huang <huangs@chromium.org>
Date: Thu Nov 30 01:30:39 2017

[Supersize] Console: Split _Session._ElfPathAndToolPrefixForSymbol().

Split into _ElfPathForSymbol() and _ToolPrefixForSymbol().

Bug:  723798 
Change-Id: Id30cdce8fcff59e10c9a58294254afa65d9358e4
Reviewed-on: https://chromium-review.googlesource.com/797916
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520372}
[modify] https://crrev.com/2f62b7ef8e7d6a7b765206323df3b7eadc8af638/tools/binary_size/libsupersize/console.py

Project Member

Comment 23 by bugdroid1@chromium.org, Jan 3 2018

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

commit 0509771fc7b9a573d79cd6b04a4d643ce740bb7f
Author: Samuel Huang <huangs@chromium.org>
Date: Wed Jan 03 14:49:15 2018

[Supersize] Log the number of demangle failures for each c++filt session.

Demangle failure may cause superficial diffs between .size ELFs
created from Gold (using c++filt) vs. from LLD (using llvm-cxxfilt).
This CL logs the number of such failures, using the '_Z' prefix as
a heuristical indicator.

Bug:  723798 
Change-Id: I1ffa247939f5efccfb66ebfddaf781adc0203496
Reviewed-on: https://chromium-review.googlesource.com/846530
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526689}
[modify] https://crrev.com/0509771fc7b9a573d79cd6b04a4d643ce740bb7f/tools/binary_size/libsupersize/demangle.py

Project Member

Comment 24 by bugdroid1@chromium.org, Jan 10 2018

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

commit 9fc9bd0a46cdaf404da5f6a6bf7818721c554e2d
Author: Samuel Huang <huangs@chromium.org>
Date: Wed Jan 10 15:50:02 2018

[Supersize] Gold .map file parsing: Prefer symbol names from mangled section names.

When Supersize parses Gold's .map file, it previously uses the provided
demangled name if available, and mangled section names as a fallback.

This CL reverses the priority. Advantages:
- Demangling is centralized to call to external tool (to c++filt or
  llvm-cxxfilt, which works better).
- The .map file now contributes to flags; indeed, some previously
  missing .unlikely, .rel.local, and .hot flags are now found.

Caveats:
- Supersize-archive takes a bit longer (< 2s for Chrome on Android).
- Turns out that c++filt and llvm-cxxfilt demangle .clone suffixes
  differently. For example, when demanging "_Z3foov.part.1.isra.2":
         c++filt => "foo() [clone .part.1] [clone .isra.2]"
    llvm-cxxfilt => "foo() (.part.1.isra.2)".
  This will be addressed soon.

Bug:  723798 
Change-Id: I70c3195f3a59b296a0f523337f02f36b3bed4757
Reviewed-on: https://chromium-review.googlesource.com/858118
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528325}
[modify] https://crrev.com/9fc9bd0a46cdaf404da5f6a6bf7818721c554e2d/tools/binary_size/libsupersize/linker_map_parser.py
[modify] https://crrev.com/9fc9bd0a46cdaf404da5f6a6bf7818721c554e2d/tools/binary_size/libsupersize/testdata/Archive.golden
[modify] https://crrev.com/9fc9bd0a46cdaf404da5f6a6bf7818721c554e2d/tools/binary_size/libsupersize/testdata/Archive_OutputDirectory.golden
[modify] https://crrev.com/9fc9bd0a46cdaf404da5f6a6bf7818721c554e2d/tools/binary_size/libsupersize/testdata/mock_toolchain/mock_cppfilt.py
[modify] https://crrev.com/9fc9bd0a46cdaf404da5f6a6bf7818721c554e2d/tools/binary_size/libsupersize/testdata/test.map

Project Member

Comment 25 by bugdroid1@chromium.org, Mar 27 2018

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

commit c85449f18b8cd1f693600980b14b82ad82118be1
Author: Samuel Huang <huangs@chromium.org>
Date: Tue Mar 27 18:05:20 2018

[Supersize] Simple fix to remove LLD-LTO errors.

This CL modifies linker_map_filter.py to add a filter for object
filenames. This enables Supersize to run for LTO (under LLD) without
crashing -- although some data are missing (to be fixed in follow-ups).

Bug:  723798 
Change-Id: I90b7439368cca96921d688b0c9db95a599fa2bca
Reviewed-on: https://chromium-review.googlesource.com/982254
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546161}
[modify] https://crrev.com/c85449f18b8cd1f693600980b14b82ad82118be1/tools/binary_size/libsupersize/linker_map_parser.py

Project Member

Comment 26 by bugdroid1@chromium.org, Jun 20 2018

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

commit d734b4486a4bebf863ddca192e73ab84fb89d65b
Author: Samuel Huang <huangs@chromium.org>
Date: Wed Jun 20 04:07:33 2018

[Supersize] Add LTO detection using LLD .map files.

LLD-LTO flow (TODO) will be quite different from LLD flow. Supersize
will need to tell them apart (and store them using distinct linker names
in the .size file). This CL adds the functionality to detect LTO usage
from LLD .map files.

Previously linker detection (to discern Gold, and 2 versions of LLD
files) only needed to examine the first line of .map file. But for
LTO (LLD only), we'll need to look beyond this. The signal we chose is
to look for 'thinlto-cache' prefix at locations where paths usually
reside in .map files. To optimize for speed, we choose and scan one
'indicator section' (.ARM.exidx or .reloc, but easy to tweak).

Details:
- Refactor: Previously linker_map_parser.DetectLinkerNameFromMapFile()
  was called twice: to computie metadata, and in MapFileParser. Since
  the function is becoming more complex, we new call it once (for
  metadata), then pipe the information to MapFileParser to use.
- Add linker_map_parser._DetectLto() to handle most of the parsing. As
  optimization, we use ad-hoc string processing instead of Regex.
- Minor cleanup on version 0 vs. 1 difference for LLD.

After this change, the list of linker names are:
  {'gold', 'lld_v0', 'lld-lto_v0', 'lld_v1', 'lld-lto_v1'}.

Bug:  723798 
Change-Id: I0498e50446be32a0335bb4781d60a6b72d7e87b5
Reviewed-on: https://chromium-review.googlesource.com/1107037
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568723}
[modify] https://crrev.com/d734b4486a4bebf863ddca192e73ab84fb89d65b/tools/binary_size/libsupersize/archive.py
[modify] https://crrev.com/d734b4486a4bebf863ddca192e73ab84fb89d65b/tools/binary_size/libsupersize/integration_test.py
[modify] https://crrev.com/d734b4486a4bebf863ddca192e73ab84fb89d65b/tools/binary_size/libsupersize/linker_map_parser.py

Project Member

Comment 27 by bugdroid1@chromium.org, Jun 22 2018

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

commit 847ca33d7ecaba3272ac06702bf92a1cfdf3ed4d
Author: Samuel Huang <huangs@chromium.org>
Date: Fri Jun 22 16:01:39 2018

[Clang] Make download_objdump.py download llvm-bcanalyzer.

This CL adds llvm-bcanalyzer to the list of files packaged and
downloaded by download_objdump.py. This is needed for extracting
data from .o files (which are LLVM Bitcode Files) under LLD-LTO.

Bug:  723798 
Change-Id: I87ef05b17399d56f85ca51e042cbc5415edb8bce
Reviewed-on: https://chromium-review.googlesource.com/1106903
Reviewed-by: Hans Wennborg <hans@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569641}
[modify] https://crrev.com/847ca33d7ecaba3272ac06702bf92a1cfdf3ed4d/tools/clang/scripts/package.py

Project Member

Comment 28 by bugdroid1@chromium.org, Jul 13

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

commit 4dd7097de4e19ae7623947c26584f4a152283f2c
Author: Samuel Huang <huangs@chromium.org>
Date: Fri Jul 13 15:57:58 2018

[Supersize] Add kwargs support to concurrent.BulkForkAndCall().

concurrent.BulkForkAndCall(func, arg_tuples) calls |func()| with
arguments given by |arg_tuples|, which stores arguments over
independent forked calls as a list of tuples. Previously this requires
common parameters (e.g., tool_prefix) to be present in each tuple, and
decreases flexibility for |arg_tuples| preparation code.

This CL adds |kwargs| parameter to BulkForkAndCall(), thereby allowing
common parameters to be given once. As seen in the test changes, a
minor caveat is that |func()| will need to make common parameters
appear at end of argument list.

Bug:  723798 
Change-Id: I9aa91b6081b8fe8fa9c87a9ae1cb959d3f17e9df
Reviewed-on: https://chromium-review.googlesource.com/1135204
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574934}
[modify] https://crrev.com/4dd7097de4e19ae7623947c26584f4a152283f2c/tools/binary_size/libsupersize/concurrent.py
[modify] https://crrev.com/4dd7097de4e19ae7623947c26584f4a152283f2c/tools/binary_size/libsupersize/concurrent_test.py
[modify] https://crrev.com/4dd7097de4e19ae7623947c26584f4a152283f2c/tools/binary_size/libsupersize/nm.py

Project Member

Comment 29 by bugdroid1@chromium.org, Jul 13

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

commit 5fbc17319efa675776ac1f27055c0b50ad22e479
Author: Samuel Huang <huangs@chromium.org>
Date: Fri Jul 13 21:03:01 2018

[Supersize] Split nm.py into {nm.py, obj_analyzer.py, string_extract.py}.

This is a code movement CL with minimal changes. Details:
- Add obj_analyzer.py: New home for BulkObjectFilenalyzer, along with
  helper functions. Also inherits main() from nm.py for testing.
- Add string_extract.py: New home for {LookupElfRodataInfo(),
  ReadFileChunks(), and ResolveStringPieces()}. Add top-level comments.
- nm.py: Have content moved to the new files. Also, exposing
  RunNmOnIntermediates(), to be called from obj_analyzer.py.
- Update archive.py and console.py to adapt to new code locations.

Bug:  723798 
Change-Id: I1d1670f04549a416f06de1da03c1a2b03c378461
Reviewed-on: https://chromium-review.googlesource.com/1136943
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575062}
[modify] https://crrev.com/5fbc17319efa675776ac1f27055c0b50ad22e479/tools/binary_size/libsupersize/archive.py
[modify] https://crrev.com/5fbc17319efa675776ac1f27055c0b50ad22e479/tools/binary_size/libsupersize/console.py
[modify] https://crrev.com/5fbc17319efa675776ac1f27055c0b50ad22e479/tools/binary_size/libsupersize/nm.py
[add] https://crrev.com/5fbc17319efa675776ac1f27055c0b50ad22e479/tools/binary_size/libsupersize/obj_analyzer.py
[add] https://crrev.com/5fbc17319efa675776ac1f27055c0b50ad22e479/tools/binary_size/libsupersize/string_extract.py
[modify] https://crrev.com/5fbc17319efa675776ac1f27055c0b50ad22e479/tools/binary_size/supersize.pydeps

Project Member

Comment 30 by bugdroid1@chromium.org, Jul 19

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

commit 3f2f4b462643ab0396ef208ab773e418df52da65
Author: Samuel Huang <huangs@chromium.org>
Date: Thu Jul 19 21:14:54 2018

[Supersize] Add bcanalyzer.py to process LLVM Bitcode (BC) Files.

This CL adds code to that parses llvm-bcanalyzer output to extract
strings in BC .o files. This is needed for LLD-LTO support. Integration
code will be follow-up CLs. Details:
- Add bcanalyzer.py, which has:
  - IsBitcodeFile(): Decides whether a .o file is BC by file magic.
  - ParseTag(): Heuristic tag parser, exposed for testing.
  - RunBcAnalyzerOnIntermediates(): BulkForAndCall() target that will
    be the main interface to extract strings from BC files.
  - Stand-alone main(), to test string extraction on stand-alone BC
    files.
- Add bcanalyzer_test.py: Unit tests.
  - Test ParseTag() since it's rather error prone, but not the other
    low-level parse routines.
  - Test string extraction based on mock data (see below).
- Add mock_bcanalyzer.py (with wrapper llvm-bcanalyzer): Test case
  generated from a sample C++ file with string data in various form.
  The embedded llvm-bcanalyzer dump is extensively annotated to
  document the data format.

Bug:  723798 
Change-Id: I8a9ff659031afa3391f0bbeb22804c3da8578c94
Reviewed-on: https://chromium-review.googlesource.com/1140685
Reviewed-by: Samuel Huang <huangs@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576630}
[add] https://crrev.com/3f2f4b462643ab0396ef208ab773e418df52da65/tools/binary_size/libsupersize/bcanalyzer.py
[add] https://crrev.com/3f2f4b462643ab0396ef208ab773e418df52da65/tools/binary_size/libsupersize/bcanalyzer_test.py
[modify] https://crrev.com/3f2f4b462643ab0396ef208ab773e418df52da65/tools/binary_size/libsupersize/path_util.py
[add] https://crrev.com/3f2f4b462643ab0396ef208ab773e418df52da65/tools/binary_size/libsupersize/testdata/mock_toolchain/llvm-bcanalyzer
[add] https://crrev.com/3f2f4b462643ab0396ef208ab773e418df52da65/tools/binary_size/libsupersize/testdata/mock_toolchain/mock_bcanalyzer.py

Blocking: 469376
As far as I know this is the only remaining blocker for enabling ThinLTO in official Android builds.
Ah wow! The biggest CL had been landed. There are 2 more CLs: refactoring & integration. Should land early next week.
Project Member

Comment 33 by bugdroid1@chromium.org, Jul 23

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

commit 3df5254f608bfec7ee6407faed8c7232b935f318
Author: Samuel Huang <huangs@chromium.org>
Date: Mon Jul 23 21:36:19 2018

[Supersize] Refactor to prepare for LTO string support integration.

This CL refactors Supersize to prepare for bcanalyzer.py integration.
Details:
- Variable and function renames.
- Split _BulkObjectFileAnalyzerWorker.AnalyzePaths():
  - _ClassifyPaths(): Explicitly store .a files and .o files. For LTO
    we'll split .o into ELF and BC buckets.
  - _MakeBatches(): Reusable later for ELF and BC separately.
  - _DoBulkFork(): Reusable.
  - _RunNm(): Absorbs nm-specific code, will add alternative.
- Split _BulkObjectFileAnalyzerWorker.AnalyzeStringLiterals()
  - _ReadElfStringData(): Separation of concern.
  - _GetEncodedRangesFromStringAddresses(): ELF-specific code. Will
    add alternative.
  - Restructure how results are merged, with more comments.
- Split ResolveStringPiecesIndirect() (was ResolveStringPieces()):
  - _AnnotateStringData(): Reusable.
  - Will add alternative: ResolveStringPieces().

Bug:  723798 
Change-Id: Ib8e0e1785ae11652a17d060c9629e3986df0f93a
Reviewed-on: https://chromium-review.googlesource.com/1146775
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577274}
[modify] https://crrev.com/3df5254f608bfec7ee6407faed8c7232b935f318/tools/binary_size/libsupersize/archive.py
[modify] https://crrev.com/3df5254f608bfec7ee6407faed8c7232b935f318/tools/binary_size/libsupersize/nm.py
[modify] https://crrev.com/3df5254f608bfec7ee6407faed8c7232b935f318/tools/binary_size/libsupersize/obj_analyzer.py
[modify] https://crrev.com/3df5254f608bfec7ee6407faed8c7232b935f318/tools/binary_size/libsupersize/string_extract.py

Project Member

Comment 34 by bugdroid1@chromium.org, Jul 24

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

commit b08bfffdba1699bae4cf9106399b7ca11bc71d73
Author: Samuel Huang <huangs@chromium.org>
Date: Tue Jul 24 18:44:39 2018

[Supersize] Add LLD-LTO string literal support.

This is the last of the series of CLs to add LLD-LTO string literal
support to Supersize by parsing BC .o files when LLD-LTO is enabled.
Details:
- Add ResolveStringPieces() (alongside ResolveStringPiecesIndirect()) to
  process strings extracted from BC files.
- Check each .o files processed to decide if it's a BC file.
- nm is still used to process all .o files (including BC files) to
  extract symbols.
- Any BC .o files found are processed by the new code that calls
  llvm-bcanalyzer. This provides flexibility since some .o files may
  still be ELF.

Bug:  723798 
Change-Id: I5b7d1db9f91a2cd6749fe4c57595ac46eee664a8
Reviewed-on: https://chromium-review.googlesource.com/1130006
Reviewed-by: Samuel Huang <huangs@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577622}
[modify] https://crrev.com/b08bfffdba1699bae4cf9106399b7ca11bc71d73/tools/binary_size/libsupersize/obj_analyzer.py
[modify] https://crrev.com/b08bfffdba1699bae4cf9106399b7ca11bc71d73/tools/binary_size/libsupersize/string_extract.py
[modify] https://crrev.com/b08bfffdba1699bae4cf9106399b7ca11bc71d73/tools/binary_size/supersize.pydeps

Status: Fixed (was: Available)
Implemented support for strings, which accounts for most of the feature gap. There might be miscellaneous stuff left, but we'll deal with them as them come.
pcc@: Now that Supersize support has been added, is ThinLTO going to be enabled for official Android builds?
Yes, I've been trying to enable it this and last week. The most recent attempt is https://chromium-review.googlesource.com/1155977

Sign in to add a comment