New issue
Advanced search Search tips

Issue 722997 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

supersize does not process clone symbols from .c files

Project Member Reported by agrieve@chromium.org, May 16 2017

Issue description

The [clone] logic acts upon unmangled names. However, symbols from .c files are never mangled in the first place, and so there are still symbols in .size that look like:

52)     12500 (7.8%)  t@0x2ce5ac   104     third_party/android_tools/ndk/sources/android/cpufeatures/cpu-features.c
             get_file_size.constprop.1
53)     12632 (7.9%)  t@0x2f608c   132     third_party/icu/source/common/uchar.cpp
             u_getUnicodeProperties_59.part.1
54)     12820 (8.0%)  t@0x35a6e6   188     third_party/icu/source/common/ustrcase.cpp
             ustrcase_mapWithOverlap_59.part.4
55)     12886 (8.0%)  t@0x480bd4   66      base/third_party/libevent/event.c
             min_heap_shift_up_.isra.3
56)     12918 (8.0%)  t@0x48294e   32      third_party/sqlite/amalgamation/sqlite3.c
             applyAffinity.part.280
 
428)   126787 (79.0%) r@0x29824c8  64      third_party/boringssl/src/crypto/evp/p_rsa.c
             kDummyHash.9262
429)   126979 (79.1%) r@0x29825d0  192     third_party/boringssl/src/crypto/fipsmodule/bcm.c
             kData.13847
Project Member

Comment 2 by bugdroid1@chromium.org, May 17 2017

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

commit c929e49f051f0414e27d6e3cca9c7c4163f16c61
Author: Andrew Grieve <agrieve@chromium.org>
Date: Wed May 17 19:52:53 2017

supersize: No name modifying in archive. Don't cluster cross-path.  Cluster C clones

This latest round of tweaks to NormalizeNames() made me realize that
when the logic there is wrong, we can't undo it on existing .size files.
So, this now skips calling NormalizeNames() during archive. Growse .size
files by ~100kb.

The recent change to Cluster() was merging together symbols from
different object files. This is a problem because queries are often
based on path, and when a group has multiple paths it returns None.
Cluster() now uses object_path in the key, and GroupedByFullName() is
added for when it's useful to actually group cross-paths.

Clones that are for symbols in .c files are now recognized by
NormalizeNames(). E.g.: "get_file_size.constprop.1"

Compiler-generated symbols such as "CSWTCH.123" are now ignored by nm.py
so that two different object paths that have such a symbol no longer
have their paths merged (since in this case, the symbols actually are
different).

BUG= 722999 ,  722997 

Change-Id: I6d4406c29f4b4383b825af7491dd943b2070f4e6
Reviewed-on: https://chromium-review.googlesource.com/507547
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#472539}
[modify] https://crrev.com/c929e49f051f0414e27d6e3cca9c7c4163f16c61/tools/binary_size/libsupersize/archive.py
[modify] https://crrev.com/c929e49f051f0414e27d6e3cca9c7c4163f16c61/tools/binary_size/libsupersize/console.py
[modify] https://crrev.com/c929e49f051f0414e27d6e3cca9c7c4163f16c61/tools/binary_size/libsupersize/integration_test.py
[modify] https://crrev.com/c929e49f051f0414e27d6e3cca9c7c4163f16c61/tools/binary_size/libsupersize/models.py
[modify] https://crrev.com/c929e49f051f0414e27d6e3cca9c7c4163f16c61/tools/binary_size/libsupersize/nm.py
[modify] https://crrev.com/c929e49f051f0414e27d6e3cca9c7c4163f16c61/tools/binary_size/libsupersize/testdata/Console.golden
[modify] https://crrev.com/c929e49f051f0414e27d6e3cca9c7c4163f16c61/tools/binary_size/libsupersize/testdata/Diff_NullDiff.golden
[modify] https://crrev.com/c929e49f051f0414e27d6e3cca9c7c4163f16c61/tools/binary_size/libsupersize/testdata/FullDescription.golden
[modify] https://crrev.com/c929e49f051f0414e27d6e3cca9c7c4163f16c61/tools/binary_size/libsupersize/testdata/SymbolGroupMethods.golden

Status: Fixed (was: Assigned)

Sign in to add a comment