[NaCl SDK] Fix create_nmf handling of missing libraries.

Previously a library that was needed on two architectures
but only found on one would be silently ignored.

BUG=
R=binji@chromium.org

Review URL: https://codereview.chromium.org/15891011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@202688 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 90 insertions, 41 deletions

diff --git a/native_client_sdk/src/tools/create_nmf.py b/native_client_sdk/src/tools/create_nmf.py
index 6ca83ab..5cf2e147 100755
--- a/native_client_sdk/src/tools/create_nmf.py
+++ b/native_client_sdk/src/tools/create_nmf.py
@@ -190,7 +190,7 @@ class ArchFile(object):
     self.path = path
     self.url = url
     self.arch = arch
-    if arch is None:
+    if not arch:
       self.arch = ParseElfHeader(path)[0]
 
   def __repr__(self):
@@ -235,15 +235,23 @@ class NmfUtils(object):
     self.remap = remap or {}
     self.pnacl = main_files and main_files[0].endswith('pexe')
 
-  def GleanFromObjdump(self, files):
+    for filename in self.main_files:
+      if not os.path.exists(filename):
+        raise Error('Input file not found: %s' % filename)
+      if not os.path.isfile(filename):
+        raise Error('Input is not a file: %s' % filename)
+
+  def GleanFromObjdump(self, files, arch):
     '''Get architecture and dependency information for given files
 
     Args:
-      files: A dict with key=filename and value=list or set of archs.  E.g.:
-          { '/path/to/my.nexe': ['x86-32']
-            '/path/to/lib64/libmy.so': ['x86-64'],
-            '/path/to/mydata.so': ['x86-32', 'x86-64'],
-            '/path/to/my.data': None }  # Indicates all architectures
+      files: A list of files to examine.
+          [ '/path/to/my.nexe',
+            '/path/to/lib64/libmy.so',
+            '/path/to/mydata.so',
+            '/path/to/my.data' ]
+      arch: The architecure we are looking for, or None to accept any
+            architecture.
 
     Returns: A tuple with the following members:
       input_info: A dict with key=filename and value=ArchFile of input files.
@@ -257,11 +265,22 @@ class NmfUtils(object):
       self.objdump = FindObjdumpExecutable()
       if not self.objdump:
         raise Error('No objdump executable found (see --help for more info)')
-    DebugPrint('GleanFromObjdump(%s)' % ([self.objdump, '-p'] + files.keys()))
-    proc = subprocess.Popen([self.objdump, '-p'] + files.keys(),
-                            stdout=subprocess.PIPE,
+
+    full_paths = set()
+    for filename in files:
+      if os.path.exists(filename):
+        full_paths.add(filename)
+      else:
+        for path in self.FindLibsInPath(filename):
+          full_paths.add(path)
+
+    cmd = [self.objdump, '-p'] + list(full_paths)
+    DebugPrint('GleanFromObjdump[%s](%s)' % (arch, cmd))
+    proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
                             stderr=subprocess.PIPE, bufsize=-1)
+
     input_info = {}
+    found_basenames = set()
     needed = set()
     output, err_output = proc.communicate()
     if proc.returncode:
@@ -274,18 +293,26 @@ class NmfUtils(object):
       matched = FormatMatcher.match(line)
       if matched:
         filename = matched.group(1)
-        arch = OBJDUMP_ARCH_MAP[matched.group(2)]
-        if files[filename] is None or arch in files[filename]:
-          name = os.path.basename(filename)
-          input_info[filename] = ArchFile(
-              arch=arch,
-              name=name,
-              path=filename,
-              url='/'.join(self.lib_prefix + [ARCH_LOCATION[arch], name]))
+        file_arch = OBJDUMP_ARCH_MAP[matched.group(2)]
+        if arch and file_arch != arch:
+          continue
+        name = os.path.basename(filename)
+        found_basenames.add(name)
+        input_info[filename] = ArchFile(
+            arch=file_arch,
+            name=name,
+            path=filename,
+            url='/'.join(self.lib_prefix + [ARCH_LOCATION[file_arch], name]))
       matched = NeededMatcher.match(line)
       if matched:
-        if files[filename] is None or arch in files[filename]:
-          needed.add('/'.join([arch, matched.group(1)]))
+        match = '/'.join([file_arch, matched.group(1)])
+        needed.add(match)
+        Trace("NEEDED: %s" % match)
+
+    for filename in files:
+      if os.path.basename(filename) not in found_basenames:
+        raise Error('Library not found [%s]: %s' % (arch, filename))
+
     return input_info, needed
 
   def FindLibsInPath(self, name):
@@ -323,28 +350,36 @@ class NmfUtils(object):
 
     if dynamic:
       examined = set()
-      all_files, unexamined = self.GleanFromObjdump(
-          dict([(f, None) for f in self.main_files]))
-      for name, arch_file in all_files.items():
-        arch_file.url = name
+      all_files, unexamined = self.GleanFromObjdump(self.main_files, None)
+      for arch_file in all_files.itervalues():
+        arch_file.url = arch_file.path
         if unexamined:
           unexamined.add('/'.join([arch_file.arch, RUNNABLE_LD]))
+
       while unexamined:
         files_to_examine = {}
+
+        # Take all the currently unexamined files and group them
+        # by architecture.
         for arch_name in unexamined:
           arch, name = arch_name.split('/')
-          for path in self.FindLibsInPath(name):
-            files_to_examine.setdefault(path, set()).add(arch)
-        new_files, needed = self.GleanFromObjdump(files_to_examine)
-        all_files.update(new_files)
+          files_to_examine.setdefault(arch, []).append(name)
+
+        # Call GleanFromObjdump() for each architecture.
+        needed = set()
+        for arch, files in files_to_examine.iteritems():
+          new_files, new_needed = self.GleanFromObjdump(files, arch)
+          all_files.update(new_files)
+          needed |= new_needed
+
         examined |= unexamined
         unexamined = needed - examined
 
       # With the runnable-ld.so scheme we have today, the proper name of
       # the dynamic linker should be excluded from the list of files.
       ldso = [LD_NACL_MAP[arch] for arch in set(OBJDUMP_ARCH_MAP.values())]
-      for name, arch_map in all_files.items():
-        if arch_map.name in ldso:
+      for name, arch_file in all_files.items():
+        if arch_file.name in ldso:
           del all_files[name]
 
       self.needed = all_files
@@ -369,8 +404,9 @@ class NmfUtils(object):
     nexe_root = os.path.normcase(nexe_root)
 
     needed = self.GetNeeded()
-    for source, arch_file in needed.items():
+    for arch_file in needed.itervalues():
       urldest = arch_file.url
+      source = arch_file.path
 
       # for .nexe and .so files specified on the command line stage
       # them in paths relative to the .nexe (with the .nexe always
@@ -596,7 +632,7 @@ def main(argv):
                     help='Override the default "objdump" tool used to find '
                          'shared object dependencies',
                     metavar='TOOL')
-  parser.add_option('--no-default-libpath',
+  parser.add_option('--no-default-libpath', action='store_true',
                     help="Don't include the SDK default library paths")
   parser.add_option('--debug-libs', action='store_true',
                     help='Use debug library paths when constructing default '
@@ -635,12 +671,6 @@ def main(argv):
   if len(args) < 1:
     raise Error('No nexe files specified.  See --help for more info')
 
-  for filename in args:
-    if not os.path.exists(filename):
-      raise Error('Input file not found: %s' % filename)
-    if not os.path.isfile(filename):
-      raise Error('Input is not a file: %s' % filename)
-
   canonicalized = ParseExtraFiles(options.extra_files, sys.stderr)
   if canonicalized is None:
     parser.error('Bad --extra-files (-x) argument syntax')
@@ -657,6 +687,12 @@ def main(argv):
   else:
     path_prefix = []
 
+  for libpath in options.lib_path:
+    if not os.path.exists(libpath):
+      raise Error('Specified library path does not exist: %s' % libpath)
+    if not os.path.isdir(libpath):
+      raise Error('Specified library is not a directory: %s' % libpath)
+
   if not options.no_default_libpath:
     # Add default libraries paths to the end of the search path.
     config = options.debug_libs and 'Debug' or 'Release'
@@ -670,7 +706,7 @@ def main(argv):
                  remap=remap)
 
   nmf.GetManifest()
-  if options.output is None:
+  if not options.output:
     sys.stdout.write(nmf.GetJson())
   else:
     with open(options.output, 'w') as output:
diff --git a/native_client_sdk/src/tools/tests/create_nmf_test.py b/native_client_sdk/src/tools/tests/create_nmf_test.py
index fe08f6b..c0855ad 100755
--- a/native_client_sdk/src/tools/tests/create_nmf_test.py
+++ b/native_client_sdk/src/tools/tests/create_nmf_test.py
@@ -145,8 +145,9 @@ class TestNmfUtils(unittest.TestCase):
   def Mktemp(self):
     self.tempdir = tempfile.mkdtemp()
 
-  def CreateNmfUtils(self):
-    libdir = os.path.join(self.toolchain, 'x86_64-nacl', 'lib32')
+  def CreateNmfUtils(self, libdir=None):
+    if not libdir:
+      libdir = os.path.join(self.toolchain, 'x86_64-nacl', 'lib32')
     return create_nmf.NmfUtils([self.dyn_nexe],
                                lib_path=[libdir],
                                objdump=self.objdump)
@@ -194,6 +195,8 @@ class TestNmfUtils(unittest.TestCase):
   def testStageDependencies(self):
     self.Mktemp()
     nmf = self.CreateNmfUtils()
+    #create_nmf.DebugPrint.debug_mode = True
+    #create_nmf.Trace.verbose = True
 
     # Stage dependencies
     nmf.StageDependencies(self.tempdir)
@@ -209,6 +212,16 @@ class TestNmfUtils(unittest.TestCase):
     expectedContents = self.dyn_deps
     self.assertEqual(contents, expectedContents)
 
+  def testMissingArchLibrary(self):
+    self.Mktemp()
+    nmf = self.CreateNmfUtils()
+    # CreateNmfUtils uses the 32-bit library path, but not the 64-bit one
+    # so searching for a 32-bit library should succeed while searching for
+    # a 64-bit one should fail.
+    nmf.GleanFromObjdump(['libgcc_s.so.1'], 'x86-32')
+    self.assertRaises(create_nmf.Error,
+                      nmf.GleanFromObjdump, ['libgcc_s.so.1'], 'x86-64')
+
 
 if __name__ == '__main__':
   unittest.main()