Upload symbols when uploading mojo binaries to the CDN.

mojo/tools/upload_binaries.py

Index: mojo/tools/upload_binaries.py
diff --git a/mojo/tools/upload_binaries.py b/mojo/tools/upload_binaries.py
index c2998147153fb1433e1b26740f829970659551c9..53e8100b26a84134495238292892400b4b6fadfe 100755
--- a/mojo/tools/upload_binaries.py
+++ b/mojo/tools/upload_binaries.py
@@ -9,6 +9,7 @@ https://github.com/domokit/mojo/wiki/Rolling-code-between-chromium-and-mojo#chro
 
 import argparse
 import glob
+import itertools
 import os
 import subprocess
 import sys
@@ -21,6 +22,10 @@ from mopy.config import Config
 from mopy.paths import Paths
 from mopy.version import Version
 
+sys.path.append(os.path.join(os.path.dirname(__file__),
+                             '../../third_party/pyelftools'))

[viettrungluu, 2015/07/10 18:27:01]

os.path.join(os.pardir, os.pardir, "third_party", "pyelftools")

[qsr, 2015/07/15 13:49:11]

Done.

+import elftools.elf.elffile as elffile
+
 BLACKLISTED_APPS = [
   # The network service apps are not produced out of the Mojo repo, but may
   # be present in the build dir.
@@ -79,6 +84,40 @@ def upload(config, source, dest, dry_run):
     subprocess.check_call([gsutil_exe, "cp", source, dest])
 
 
+def _get_signature(file_object):
+  """Compute a unique signature of a library file.

[viettrungluu, 2015/07/10 18:27:01]

And what if it's not a library file, or even an ELF binary?

[qsr, 2015/07/10 19:35:27]

Was certain it was returning None. Will change.

+
+  TODO(qsr): Move this function to devtools and import it from there to ensure
+  signature computation is always coherent.
+  """
+  elf = elffile.ELFFile(file_object)
+  text_section = elf.get_section_by_name('.text')

[viettrungluu, 2015/07/10 18:27:01]

Apparently, "s are mostly preferred in this file over 's.

[qsr, 2015/07/15 13:49:11]

Done.

+  file_object.seek(text_section['sh_offset'])
+  data = file_object.read(min(4096, text_section['sh_size']))

[viettrungluu, 2015/07/10 18:27:01]

It seems a little weak (in terms of "uniqueness") to look at only 4096 bytes. I
don't see why you don't just hash all of it. (Looking only at .text is a little
weak, but I guess OK for looking at the stack. But maybe you should hash the
entire stripped library (or the .mojo).

[qsr, 2015/07/10 19:35:27]

So this algo is what breakpad use, so I do not see any reason to do anything
else. But the property I really want are:

1) Do not depend on whether the symbol are there or not
2) Computed from a small part of the file. The reason for this is that when
debugging with gdb, I want to be able to compute the hash of a file without
having to transfer it completely over USB because this is slow as hell.

+  def combine((i, c)):
+    return i ^ ord(c)
+  result = 16 * [0]
+  for i in xrange(0, len(data), len(result)):

[viettrungluu, 2015/07/10 18:27:01]

... and I don't understand why you wouldn't use a proper hash function.

[qsr, 2015/07/10 19:35:27]

 Same thing, this is what breakpad is doing and I do not see any reason to
re-invent my own algo.

+    result = map(combine,
+                 itertools.izip_longest(result,
+                                        data[i:i + len(result)],
+                                        fillvalue='\0'))
+  return ''.join(["%02x" % x for x in result])
+
+
+def upload_symbols(config, build_dir, dry_run):
+  dest_dir = "gs://mojo/symbols/"
+  for name in os.listdir(build_dir):
+    path = os.path.join(build_dir, name)
+    if (os.path.isfile(path) and
+        not os.path.islink(path) and
+        os.access(path, os.X_OK) and

[viettrungluu, 2015/07/10 18:27:01]

This seems to assume that all files in the build dir with the x bit set are
interesting libraries, which seems wrong to me:

* It's not clear to me why we set the x bit for .so's in the first place, so if
we ever changed that this would break.
* This will also include tools, such as md5sum_bin.
* This will even include tools built for the host, such as md5sum_bin_host.

So for .mojo's, we should modify the rule to copy (or maybe symlink) the
unstripped library somewhere, so that we'll have a canonical(ish[*]) set of
things to upload. (We can deal with the shell itself separately.)

[*] Of course, if we're not doing clobber builds, then we could have stale
stuff.

[qsr, 2015/07/10 19:35:27]

 Getting some more artifact is not really a problem.
 Stale stuff is not a problem either. We would just re-upload old symbols.

 Will change the mojo application target to hard link to a new directory. I
don't want symlinks because stale symlink without their target might break this
script.

+        not path.endswith('.mojo')):
+      with open(path) as f:
+        dest = dest_dir + _get_signature(f)
+        upload(config, path, dest, dry_run)
+
+
 def upload_shell(config, dry_run, verbose):
   paths = Paths(config)
   zipfile_name = target(config)
@@ -179,6 +218,8 @@ def main():
   for file_to_upload in files_to_upload:
     upload_file(file_to_upload, config, args.dry_run)
 
+  upload_symbols(config, build_directory, args.dry_run)
+
   return 0
 
 if __name__ == "__main__":