Remove complexity in caching of llvm-nm results. NFC (#13360)

This extra argument (include_internal) and the extra complexity
it entailed had only a single call site in the test suite.

Calling llvm-nm directly in the test suite is much simpler.

Author: sbc100

tests/test_other.py

@@ -6502,8 +6502,8 @@ def test_disable_inlining(self):
 
     # To this test to be successful, foo() shouldn't have been inlined above and
     # foo() should be in the function list
-    syms = building.llvm_nm('test2.o', include_internal=True)
-    assert 'foo' in syms.defs, 'foo() should not be inlined'
+    output = self.run_process([shared.LLVM_NM, 'test2.o'], stdout=PIPE).stdout
+    self.assertContained('foo', output)
 
   def test_output_eol(self):
     for params in [[], ['--proxy-to-worker'], ['--proxy-to-worker', '-s', 'WASM=0']]:

tools/building.py

@@ -39,10 +39,8 @@
 multiprocessing_pool = None
 binaryen_checked = False
 
-# internal caches
-internal_nm_cache = {}
 # cache results of nm - it can be slow to run
-uninternal_nm_cache = {}
+nm_cache = {}
 # Stores the object files contained in different archive files passed as input
 ar_contents = {}
 _is_ar_cache = {}
@@ -150,12 +148,11 @@ def check(value):
   return list(filter(check, values))
 
 
-# clear internal caches. this is not normally needed, except if the clang/LLVM
+# clear caches. this is not normally needed, except if the clang/LLVM
 # used changes inside this invocation of Building, which can happen in the benchmarker
 # when it compares different builds.
 def clear():
-  internal_nm_cache.clear()
-  uninternal_nm_cache.clear()
+  nm_cache.clear()
   ar_contents.clear()
   _is_ar_cache.clear()
 
@@ -372,7 +369,7 @@ def make_paths_absolute(f):
 
 
 # Runs llvm-nm in parallel for the given list of files.
-# The results are populated in uninternal_nm_cache
+# The results are populated in nm_cache
 # multiprocessing_pool: An existing multiprocessing pool to reuse for the operation, or None
 # to have the function allocate its own.
 def parallel_llvm_nm(files):
@@ -383,7 +380,7 @@ def parallel_llvm_nm(files):
     for i, file in enumerate(files):
       if object_contents[i].returncode != 0:
         logger.debug('llvm-nm failed on file ' + file + ': return code ' + str(object_contents[i].returncode) + ', error: ' + object_contents[i].output)
-      uninternal_nm_cache[file] = object_contents[i]
+      nm_cache[file] = object_contents[i]
     return object_contents
 
 
@@ -398,7 +395,7 @@ def read_link_inputs(files):
 
       if absolute_path_f not in ar_contents and is_ar(absolute_path_f):
         archive_names.append(absolute_path_f)
-      elif absolute_path_f not in uninternal_nm_cache and is_bitcode(absolute_path_f):
+      elif absolute_path_f not in nm_cache and is_bitcode(absolute_path_f):
         object_names.append(absolute_path_f)
 
     # Archives contain objects, so process all archives first in parallel to obtain the object files in them.
@@ -419,7 +416,7 @@ def clean_at_exit():
 
     for o in object_names_in_archives:
       for f in o['files']:
-        if f not in uninternal_nm_cache:
+        if f not in nm_cache:
           object_names.append(f)
 
     # Next, extract symbols from all object files (either standalone or inside archives we just extracted)
@@ -730,7 +727,7 @@ def get_command_with_possible_response_file(cmd):
   return new_cmd
 
 
-def parse_symbols(output, include_internal=False):
+def parse_symbols(output):
   defs = []
   undefs = []
   commons = []
@@ -753,42 +750,37 @@ def parse_symbols(output, include_internal=False):
         undefs.append(symbol)
       elif status == 'C':
         commons.append(symbol)
-      elif (not include_internal and status == status.upper()) or \
-           (include_internal and status in ['W', 't', 'T', 'd', 'D']):
+      elif status == status.upper():
         # FIXME: using WTD in the previous line fails due to llvm-nm behavior on macOS,
         #        so for now we assume all uppercase are normally defined external symbols
         defs.append(symbol)
   return ObjectFileInfo(0, None, set(defs), set(undefs), set(commons))
 
 
-def llvm_nm_uncached(filename, stdout=PIPE, stderr=PIPE, include_internal=False):
+def llvm_nm_uncached(filename, stdout=PIPE, stderr=PIPE):
   # LLVM binary ==> list of symbols
   proc = run_process([LLVM_NM, filename], stdout=stdout, stderr=stderr, check=False)
   if proc.returncode == 0:
-    return parse_symbols(proc.stdout, include_internal)
+    return parse_symbols(proc.stdout)
   else:
     return ObjectFileInfo(proc.returncode, str(proc.stdout) + str(proc.stderr))
 
 
-def llvm_nm(filename, stdout=PIPE, stderr=PIPE, include_internal=False):
+def llvm_nm(filename, stdout=PIPE, stderr=PIPE):
   # Always use absolute paths to maximize cache usage
   filename = os.path.abspath(filename)
 
-  if include_internal and filename in internal_nm_cache:
-    return internal_nm_cache[filename]
-  elif not include_internal and filename in uninternal_nm_cache:
-    return uninternal_nm_cache[filename]
+  if filename in nm_cache:
+    return nm_cache[filename]
 
-  ret = llvm_nm_uncached(filename, stdout, stderr, include_internal)
+  ret = llvm_nm_uncached(filename, stdout, stderr)
 
   if ret.returncode != 0:
     logger.debug('llvm-nm failed on file ' + filename + ': return code ' + str(ret.returncode) + ', error: ' + ret.output)
 
-  # Even if we fail, write the results to the NM cache so that we don't keep trying to llvm-nm the failing file again later.
-  if include_internal:
-    internal_nm_cache[filename] = ret
-  else:
-    uninternal_nm_cache[filename] = ret
+  # Even if we fail, write the results to the NM cache so that we don't keep trying to llvm-nm the
+  # failing file again later.
+  nm_cache[filename] = ret
 
   return ret