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

src/PNaClTranslator.cpp

Index: src/PNaClTranslator.cpp
diff --git a/src/PNaClTranslator.cpp b/src/PNaClTranslator.cpp
index 595b2340eb515b1ed3878e3596ebecc26e2bded0..7aca4949724b2b0396959b33b60754db168947b5 100644
--- a/src/PNaClTranslator.cpp
+++ b/src/PNaClTranslator.cpp
@@ -476,6 +476,12 @@ private:
                               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));