Subzero, MIPS32: Sandbox initial patch

pydir/build-runtime.py

 #!/usr/bin/env python2
 
 import argparse
 import os
 import shutil
 import tempfile
 
 import targets
 from utils import FindBaseNaCl, GetObjcopyCmd, shellcmd
 
 
 def Translate(ll_files, extra_args, obj, verbose, target):
   """Translate a set of input bitcode files into a single object file.
 
   Use pnacl-llc to translate textual bitcode input ll_files into object file
   obj, using extra_args as the architectural flags.
   """
-  externalize = [] if target == 'mips32' else ['-externalize']
+  externalize = ['-externalize']
   shellcmd(['cat'] + ll_files + ['|',
             'pnacl-llc',
             '-function-sections',
             '-O2',
             '-filetype=obj',
             '-bitcode-format=llvm',
             '-o', obj
     ] + extra_args + externalize, echo=verbose)
   strip_syms = [] if target == 'mips32' else ['nacl_tp_tdb_offset',
                                               'nacl_tp_tls_offset']
-  shellcmd([GetObjcopyCmd(target), obj] +
-    [('--strip-symbol=' + sym) for sym in strip_syms])
+  if target != 'mips32':

[Jim Stichnoth, 2016/11/08 13:36:31]

Is this diff actually needed?  As I understand it, "objcopy -L foo" is short for
"objcopy --localize-symbol=foo", and shouldn't that be unnecessary when
--strip-symbol is already being applied to those symbols?

[Stefan Maksimovic, 2016/11/08 14:59:55]

Initially I got the undefined reference errors pointing to the _Sz_* functions
from szrt.c when trying to run the cross tests, which was due to them being
local symbols.
I added the -externalize flag, making them global, which also made
nacl_tp_tdb_offset and nacl_tp_tls_offset global as well.

If those two symbols were not stripped, I'd get an error complaining about them
being defined multiple times.
If I tried to strip them, it would fail as well with the message that they are
named in a relocation, thus cannot be stripped.

The solution which worked for me was to keep them there, but to make them local,
which enabled me to run the cross tests successfully.

Please advise if there is anything faulty with this approach.

[Jim Stichnoth, 2016/11/08 15:34:08]

Ah sorry!  I misread the logic in the new code.  I thought you were both
localizing and stripping those symbols.  Instead, I see you are localizing them
for mips and stripping them for non-mips.

It looks like the non-mips targets also work if I change --strip-symbol to
--localize-symbol.  So I think we can just make that change globally.  Rename
strip_syms to localize_syms, initialize it unconditionally to
nacl_tp_{tdb,tls}_offset, and do:

  shellcmd([GetObjcopyCmd(target), obj] + [('--localize-symbol=' + sym) for sym
in localize_syms])

+    shellcmd([GetObjcopyCmd(target), obj] +
+      [('--strip-symbol=' + sym) for sym in strip_syms])
+  else:
+    shellcmd([GetObjcopyCmd(target), obj] +
+      [('--strip-symbol=' + sym) for sym in strip_syms] +
+      ['-L','nacl_tp_tdb_offset','-L','nacl_tp_tls_offset'])
 
 
 def PartialLink(obj_files, extra_args, lib, verbose):
   """Partially links a set of obj files into a final obj library."""
   shellcmd(['le32-nacl-ld',
             '-o', lib,
             '-r',
     ] + extra_args + obj_files, echo=verbose)
 
 
@@ skipping 182 matching lines @@
 
     finally:
         try:
             shutil.rmtree(tempdir)
         except OSError as exc:
             if exc.errno != errno.ENOENT: # ENOENT - no such file or directory
                 raise # re-raise exception
 
 if __name__ == '__main__':
     main()