Add '-d' flag to dump-static-initializers.py

tools/linux/dump-static-initializers.py

Index: tools/linux/dump-static-initializers.py
diff --git a/tools/linux/dump-static-initializers.py b/tools/linux/dump-static-initializers.py
index 14fe46994a89a4aa3d017e174ccbce290a1484b9..5fd9903823515b8509ecc43ba2a6d4cf7628ec24 100755
--- a/tools/linux/dump-static-initializers.py
+++ b/tools/linux/dump-static-initializers.py
@@ -96,15 +96,14 @@ def QualifyFilename(filename, symbol):
 #   0000000001919920 0000000000000008 b _ZN12_GLOBAL__N_119g_nine_box_prelightE
 nm_re = re.compile(r'(\S+) (\S+) t _GLOBAL__I_(.*)')
 def ParseNm(binary):
-  """Given a binary, yield static initializers as (start, size, file) pairs."""
+  """Given a binary, yield static initializers as (file, start, size) tuples."""
 
   nm = subprocess.Popen(['nm', '-S', binary], stdout=subprocess.PIPE)
   for line in nm.stdout:
     match = nm_re.match(line)
     if match:
       addr, size, filename = match.groups()
-      yield int(addr, 16), int(size, 16), filename
-
+      yield filename, int(addr, 16), int(size, 16)
 
 # Regex matching objdump output for the symbols we're interested in.
 # Example line:
@@ -132,54 +131,84 @@ def ExtractSymbolReferences(binary, start, end):
         # Probably a relative jump within this function.
         continue
       refs.add(ref)
-      continue
-
-  for ref in sorted(refs):
-    yield ref

[Evan Martin, 2012/01/30 23:30:57]

I'm confused, is this code in the current checked-in version?

[Tyler Breisacher (Chromium), 2012/01/30 23:35:50]

Yes. There are typically only a few refs per file, I think, so it doesn't matter
much either way, but (as above) using |yield| implies that we're yielding values
continuously, instead of waiting to get all of them and returning the whole list
all at once. If we're sorting, then we can't do that so there's no point in
using yield. Unless there's something about this case that's different than the
one above?

 
+  return sorted(refs)

[Evan Martin, 2012/01/30 23:42:25]

Can you fix the function-level comment to not say "yields"?
I think that is what got me confused.

[Tyler Breisacher (Chromium), 2012/01/30 23:53:45]

Done.

 
 def main():
-  parser = optparse.OptionParser(usage='%prog filename')
-  parser.add_option('-i', '--instances', dest='calculate_instances',
+  parser = optparse.OptionParser(usage='%prog [option] filename')
+  parser.add_option('-f', '--files', dest='count_files',
+                    action='store_true', default=False,
+                    help='Print out the number of files containing static '
+                         'initializers')
+  parser.add_option('-i', '--instances', dest='count_initializers',
+                    action='store_true', default=False,
+                    help='Print out the number of static initializers')

[Evan Martin, 2012/01/30 23:42:25]

Why not just always print these things?

[Tyler Breisacher (Chromium), 2012/01/30 23:53:45]

I'm not opposed to always printing them, but I think it is useful to have the
option to ONLY print them. When I was fixing some SIs last week, I fell into
this pattern to make sure they were actually fixed:

* checkout master
* build
* run this with -f (output is, say, 115)
* checkout the branch with my fix on it
* build
* run this with -f (output is, say, 114)

-f is faster than the other options because it doesn't call |objdump|. So maybe
we should keep -f and -i as they are, and also print the numbers at the end of
the output for the other options.

On the other hand, I also could have just piped the output of this script into
|wc -l| or something, and maybe it's simpler to just advise users to do that.
Whatever you think is better.

+  parser.add_option('-d', '--diffable', dest='diffable',
                     action='store_true', default=False,
-                    help='Only print out the number of static initializers')
+                    help='Prints the filename on each line, for more easily '
+                         'diff-able output.')
   opts, args = parser.parse_args()
   if len(args) != 1:
     parser.error('missing filename argument')
     return 1
   binary = args[0]
 
+  if opts.count_files and opts.count_initializers:
+    parser.error('-f and -i are mutually exclusive')
+    return 1
+
+  if opts.diffable and (opts.count_initializers or opts.count_files):
+    parser.error('-d cannot be used with -f or -i')
+    return 1
+
   demangler = Demangler()
-  static_initializers_count = 0
-  for addr, size, filename in ParseNm(binary):
+  file_count = 0
+  initializer_count = 0
+
+  files = ParseNm(binary)
+  if (opts.diffable):
+    files = sorted(files)

[Tyler Breisacher (Chromium), 2012/01/30 23:28:16]

The only thing that's weird about this, is we're sorting by the original
filename, not the qualified filename. So the output doesn't actually look
sorted. I don't think this is really a problem, the goal was just to make sure
the order won't change from one build to the next, not necessarily to list every
SI alphabetically.

+  for filename, addr, size in files:
     if size == 2:
       # gcc generates a two-byte 'repz retq' initializer when there is nothing
       # to do.  jyasskin tells me this is fixed in gcc 4.6.
-      # Two bytes is too small to do anything, so just ignore it.
       continue
 
-    if (opts.calculate_instances):
-      static_initializers_count += 1
+    if opts.count_files:
+      file_count += 1
       continue
 
-    ref_output = ''
+    ref_output = []
     qualified_filename = QualifyFilenameAsProto(filename)
     for ref in ExtractSymbolReferences(binary, addr, addr+size):
+      if opts.count_initializers:
+        initializer_count += 1
+        continue
+
       ref = demangler.Demangle(ref)
       if qualified_filename == filename:
         qualified_filename = QualifyFilename(filename, ref)
       if ref in NOTES:
-        ref_output = ref_output + '  %s [%s]\n' % (ref, NOTES[ref])
+        ref_output.append('  %s [%s]' % (ref, NOTES[ref]))
       else:
-        ref_output = ref_output + '  ' + ref + '\n'
-    print '%s (initializer offset 0x%x size 0x%x)' % (qualified_filename,
-                                                      addr, size)
-    print ref_output
+        ref_output.append('  ' + ref)
 
-  if opts.calculate_instances:
-    print static_initializers_count
-  return 0
+    if opts.count_initializers:
+      continue
+
+    if opts.diffable:
+      print '\n'.join(qualified_filename + r for r in ref_output)
+    else:
+      print '%s (initializer offset 0x%x size 0x%x)' % (qualified_filename,
+                                                        addr, size)
+      print '\n'.join(ref_output) + '\n'
 
+  if opts.count_files:
+    print file_count
+
+  if opts.count_initializers:
+    print initializer_count
+  return 0
 
 if '__main__' == __name__:
   sys.exit(main())