dump-static-initializers: Add an option to show the size of static initializers, and change insta...

dump-static-initializers: Add an option to show the size of static initializers, and change instances to files.

BUG=none
TEST=none

[Lei Zhang, 2011-10-07 20:30:59]

I thought about this a bit more and I think we want both the # of files and the
size.

Currently, scripts/slave/chromium/sizes.py calculates the number of files that
have static initializers, including LazyInstances. Whereas
dump-static-initializer.py -f does not, so that's useful.

I also experimented by adding a static initializer to a file that already has
static initializers, and that doesn't change the dump-static-initializer.py -f
output because the number of files doesn't change. So we really do want the
size, as that is an indicator of how many static initializers we have.

[willchan no longer on Chromium, 2011-10-07 20:33:48]

On Fri, Oct 7, 2011 at 1:30 PM, <thestig@chromium.org> wrote:

> Reviewers: willchan,
>
> Message:
> I thought about this a bit more and I think we want both the # of files and
> the
> size.
>

I don't understand this.


>
> Currently, scripts/slave/chromium/sizes.**py calculates the number of
> files that
> have static initializers, including LazyInstances. Whereas
> dump-static-initializer.py -f does not, so that's useful.
>

I think sizes.py is wrong. It should calculate number of instances, not
number of files.


> I also experimented by adding a static initializer to a file that already
> has
> static initializers, and that doesn't change the dump-static-initializer.py
> -f
> output because the number of files doesn't change. So we really do want the
> size, as that is an indicator of how many static initializers we have.
>
> Description:
> dump-static-initializers: Add an option to show the size of static
> initializers,
> and change instances to files.
>
> BUG=none
> TEST=none
>
> Please review this at
http://codereview.chromium.**org/8207001/<http://codereview.chromium.org/8207...
>
> SVN Base: svn://chrome-svn/chrome/trunk/**src/
>
> Affected files:
>  M     tools/linux/dump-static-**initializers.py
>
>
> Index: tools/linux/dump-static-**initializers.py
> ==============================**==============================**=======
> --- tools/linux/dump-static-**initializers.py     (revision 104532)
> +++ tools/linux/dump-static-**initializers.py     (working copy)
> @@ -96,17 +96,25 @@
>
>  def main():
>   parser = optparse.OptionParser(usage='%**prog filename')
> -  parser.add_option('-i', '--instances', dest='calculate_instances',
> +  parser.add_option('-f', '--files', dest='calculate_files',
>                     action='store_true', default=False,
> -                    help='Only print out the number of static
> initializers')
> +                    help='Only print out the number of files with '
> +                         'static initializers')
> +  parser.add_option('-s', '--size', dest='calculate_size',
> +                    action='store_true', default=False,
> +                    help='Only print out the total size of static
> initializers')
>   opts, args = parser.parse_args()
>   if len(args) != 1:
>     parser.error('missing filename argument')
>     return 1
> +  if opts.calculate_files and opts.calculate_size:
> +    parser.error('--files and --size are mutually exclusive')
> +    return 1
>   binary = args[0]
>
>   demangler = Demangler()
> -  static_initializers_count = 0
> +  static_initializers_file_count = 0
> +  static_initializers_size = 0
>   for addr, size, filename in ParseNm(binary):
>     if size == 2:
>       # gcc generates a two-byte 'repz retq' initializer when there is
> nothing
> @@ -114,9 +122,12 @@
>       # Two bytes is too small to do anything, so just ignore it.
>       continue
>
> -    if (opts.calculate_instances):
> -      static_initializers_count += 1
> +    if (opts.calculate_files):
> +      static_initializers_file_count += 1
>       continue
> +    if (opts.calculate_size):
> +      static_initializers_size += size
> +      continue
>
>     print '%s (initializer offset 0x%x size 0x%x)' % (filename, addr, size)
>     for ref in ExtractSymbolReferences(**binary, addr, addr+size):
> @@ -127,8 +138,10 @@
>         print ' ', ref
>     print
>
> -  if opts.calculate_instances:
> -    print static_initializers_count
> +  if opts.calculate_files:
> +    print static_initializers_file_count
> +  if opts.calculate_size:
> +    print static_initializers_size
>   return 0
>
>
>
>
>

[Lei Zhang, 2011-10-07 23:27:38]

I think this does what we want, assuming static initializers are all in the bss
section. I'm still waiting to hear back from chromium-dev.

[willchan no longer on Chromium, 2011-10-09 05:03:08]

Let me know when you hear back.

On Fri, Oct 7, 2011 at 4:27 PM, <thestig@chromium.org> wrote:

> I think this does what we want, assuming static initializers are all in the
> bss
> section. I'm still waiting to hear back from chromium-dev.
>
>
http://codereview.chromium.**org/8207001/<http://codereview.chromium.org/8207...
>

[Lei Zhang, 2011-11-02 21:26:23]

To summarize the compiler team mailing list email thread:

* .bss holds variables whose values are 0 at load time.
* All dynamically-initialized variables with static storage duration (that is,
variables with "static" initializers) go into .bss.
* Not everything in .bss is a dynamically-initialized variable with static
storage duration.
* Statically-initialized variables with static storage duration may go into
.bss, .data, or .rodata depending on the value and const-ness.

So this can have false positives - if a static initializer accesses other
variables stored in .bss, they'd show up in the list here. e.g.

// This alone won't show up because |foo| is not accessed from a .ctor section.
// global context
int foo = 0;  // statically initialized to 0 -> bss

// Both |foo| and |bar| will show up, but |foo| is a false positive.
// global context
int foo = 0;  // statically initialized to 0 -> bss
int bar = DoSomething(foo);  // |bar| is 0 at load time -> bss

// Only |bar| will show up.
// global context
int qux = 5;  // statically initialized to 5 -> data
int bar = DoSomething(foo);  // |bar| is 0 at load time -> bss

Furthermore, C++11 allows parallel initialization which will likely break all of
this, but we can worry about that when we get there.

On 2011/10/09 05:03:08, willchan wrote:
> Let me know when you hear back.
> 
> On Fri, Oct 7, 2011 at 4:27 PM, <mailto:thestig@chromium.org> wrote:
> 
> > I think this does what we want, assuming static initializers are all in the
> > bss
> > section. I'm still waiting to hear back from chromium-dev.
> >
> >
>
http://codereview.chromium.**org/8207001/%3Chttp://codereview.chromium.org/82...>
> >

[willchan no longer on Chromium, 2011-11-03 18:29:17]

Wow, there's really nothing better than this? Just counting the symbols in .bss
seems pretty inaccurate to me =/ I'll go see if I can find a better way, but if
the compiler team recommended this, then I doubt I'll come up with anything
better.

[Lei Zhang, 2011-11-03 18:47:34]

On 2011/11/03 18:29:17, willchan wrote:
> Wow, there's really nothing better than this? Just counting the symbols in
.bss
> seems pretty inaccurate to me =/ I'll go see if I can find a better way, but
if
> the compiler team recommended this, then I doubt I'll come up with anything
> better.

The compiler team didn't recommend this, nor did they recommend any better ways
to do this. I don't know of a better way off the top of my head. Maybe analyze
the disassembly from objdump to see what's being read and what's being written
to?

Note this isn't just counting symbols in .bss, which would indeed be very
inaccurate. It's counting symbols in .bss that are accessed from .ctor.

[willchan no longer on Chromium, 2011-11-05 00:03:21]

Ian Lance Taylor said:
"""
Disassemble the file using objdump -d.  Look for functions whose names
start with _GLOBAL__I.  Within such a function, count the number of call
statements.  If the function ends with a jmp statement, add 1.
"""

I'm also fine with your solution if you want.

On 2011/11/03 18:47:34, Lei Zhang wrote:
> On 2011/11/03 18:29:17, willchan wrote:
> > Wow, there's really nothing better than this? Just counting the symbols in
> .bss
> > seems pretty inaccurate to me =/ I'll go see if I can find a better way, but
> if
> > the compiler team recommended this, then I doubt I'll come up with anything
> > better.
> 
> The compiler team didn't recommend this, nor did they recommend any better
ways
> to do this. I don't know of a better way off the top of my head. Maybe analyze
> the disassembly from objdump to see what's being read and what's being written
> to?
> 
> Note this isn't just counting symbols in .bss, which would indeed be very
> inaccurate. It's counting symbols in .bss that are accessed from .ctor.