PNaCl Dynamic Linking: Force __pnacl_pso_root to be an external declaration.

src/PNaClTranslator.cpp

 //===- subzero/src/PNaClTranslator.cpp - ICE from bitcode -----------------===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
@@ skipping 458 matching lines @@
   // Gives Decl a name if it doesn't already have one. Prefix and NameIndex are
   // used to generate the name. NameIndex is automatically incremented if a new
   // name is created. DeclType is literal text describing the type of name
   // being created. Also generates warning if created names may conflict with
   // named declarations.
   void installDeclarationName(Ice::GlobalDeclaration *Decl,
                               const Ice::IceString &Prefix,
                               const char *DeclType,
                               NaClBcIndexSize_t &NameIndex) {
     if (Decl->hasName()) {
+      if (Decl->isPNaClABIExternalName()) {

[Mark Seaborn, 2016/03/09 03:37:15]

As I commented on
https://codereview.chromium.org/1740033002/diff/60001/src/PNaClTranslator.cpp...,
you should only be doing this for GlobalVariables, not Functions.

I think that means you don't need isPNaClABIExternalName() to be virtual.

[Sean Klein, 2016/03/10 23:45:23]

Changed so that setLinkage is only considered for VariableDeclarations.
However, "isPNaClABIExternalName" is still virtual.

+        // Force linkage to be external for the PNaCl ABI. PNaCl bitcode has a
+        // linkage field for Functions, but not for GlobalVariables (because the
+        // latter is not needed for pexes, so it has been removed).
+        Decl->setLinkage(llvm::GlobalValue::ExternalLinkage);

[Karl, 2016/03/09 17:06:50]

This code is somewhat fragile.  The name was earlier set when the module value
symbol table was parsed. It could have gotten mangled then. By changing this
flag now (rather than when the module value symbol table is processed), you risk
mangling the name twice. Consider moving this code to
ModuleValueSymtabParser::setValueName().

[Sean Klein, 2016/03/10 23:45:23]

Code moved to "ModuleValueSymtabParser::setValueName()", and updated
"setLinkage" to only operate on Declarations with no name.

"isPnaClAbiExternalName" updated so that a string can be passed as an argument,
even when the declaration itself may not yet have a name.

+      }
       Translator.checkIfUnnamedNameSafe(Decl->getName(), DeclType, Prefix);
     } else {
       Decl->setName(Translator.createUnnamedName(Prefix, NameIndex));
       ++NameIndex;
     }
   }
 
   // Installs names for global variables without names.
   void installGlobalVarNames() {
     assert(VariableDeclarations);
@@ skipping 2713 matching lines @@
     raw_string_ostream StrBuf(Buffer);
     StrBuf << IRFilename << ": Does not contain a module!";
     llvm::report_fatal_error(StrBuf.str());
   }
   if (InputStreamFile.getBitcodeBytes().getExtent() % 4 != 0) {
     llvm::report_fatal_error("Bitcode stream should be a multiple of 4 bytes");
   }
 }
 
 } // end of namespace Ice