PNaCl: extend GlobalCleanup to null-out extern_weak function references, and extend the ABI checker to verify.

Also promote weak symbols to internal ones, which was required because the scons barebones tests define weak versions of memcpy/memset

BUG= https://code.google.com/p/nativeclient/issues/detail?id=3339
TEST= llvm-regression (globalcleanup.ll), scons barebones tests

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

4 files changed, 94 insertions, 23 deletions

diff --git a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp
index 857f6a24af..88914c85a8 100644
--- a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp
+++ b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp
@@ -47,6 +47,7 @@ class PNaClABIVerifyModule : public ModulePass {
   bool runOnModule(Module &M);
   virtual void print(raw_ostream &O, const Module *M) const;
  private:
+  void CheckGlobalValueCommon(const GlobalValue *GV);
   PNaClABITypeChecker TC;
   PNaClABIErrorReporter *Reporter;
   bool ReporterIsOwned;
@@ -80,6 +81,30 @@ static const char *linkageName(GlobalValue::LinkageTypes LT) {
 
 } // end anonymous namespace
 
+// Check linkage type and section attributes, which are the same for
+// GlobalVariables and Functions.
+void PNaClABIVerifyModule::CheckGlobalValueCommon(const GlobalValue *GV) {
+  assert(!isa<GlobalAlias>(GV));
+  const char *GVTypeName = isa<GlobalVariable>(GV) ?
+      "Variable " : "Function ";
+  switch (GV->getLinkage()) {
+    // TODO(dschuff): Disallow external linkage
+    case GlobalValue::ExternalLinkage:
+    case GlobalValue::AvailableExternallyLinkage:
+    case GlobalValue::InternalLinkage:
+    case GlobalValue::PrivateLinkage:
+      break;
+    default:
+      Reporter->addError() << GVTypeName << GV->getName()
+                           << " has disallowed linkage type: "
+                           << linkageName(GV->getLinkage()) << "\n";
+  }
+  if (GV->hasSection()) {
+    Reporter->addError() << GVTypeName << GV->getName() <<
+        " has disallowed \"section\" attribute\n";
+  }
+}
+
 bool PNaClABIVerifyModule::runOnModule(Module &M) {
   for (Module::const_global_iterator MI = M.global_begin(), ME = M.global_end();
        MI != ME; ++MI) {
@@ -100,28 +125,14 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) {
       }
     }
 
-    // Check GV linkage types
-    switch (MI->getLinkage()) {
-      case GlobalValue::ExternalLinkage:
-      case GlobalValue::AvailableExternallyLinkage:
-      case GlobalValue::InternalLinkage:
-      case GlobalValue::PrivateLinkage:
-        break;
-      default:
-        Reporter->addError() << "Variable " << MI->getName() <<
-            " has disallowed linkage type: " <<
-            linkageName(MI->getLinkage()) << "\n";
-    }
+    CheckGlobalValueCommon(MI);
 
-    if (MI->hasSection()) {
-      Reporter->addError() << "Variable " << MI->getName() <<
-          " has disallowed \"section\" attribute\n";
-    }
     if (MI->isThreadLocal()) {
       Reporter->addError() << "Variable " << MI->getName() <<
           " has disallowed \"thread_local\" attribute\n";
     }
   }
+
   // No aliases allowed for now.
   for (Module::alias_iterator MI = M.alias_begin(),
            E = M.alias_end(); MI != E; ++MI) {
@@ -129,7 +140,7 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) {
         " is an alias (disallowed)\n";
   }
 
-  for (Module::iterator MI = M.begin(), ME = M.end(); MI != ME; ++MI) {
+  for (Module::const_iterator MI = M.begin(), ME = M.end(); MI != ME; ++MI) {
     // Check types of functions and their arguments
     FunctionType *FT = MI->getFunctionType();
     if (!TC.isValidType(FT->getReturnType())) {
@@ -152,10 +163,8 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) {
           " is a variable-argument function (disallowed)\n";
     }
 
-    if (MI->hasSection()) {
-      Reporter->addError() << "Function " << MI->getName() <<
-          " has disallowed \"section\" attribute\n";
-    }
+    CheckGlobalValueCommon(MI);
+
     if (MI->hasGC()) {
       Reporter->addError() << "Function " << MI->getName() <<
           " has disallowed \"gc\" attribute\n";
diff --git a/lib/Transforms/NaCl/GlobalCleanup.cpp b/lib/Transforms/NaCl/GlobalCleanup.cpp
index 9a28063af6..00e15b39a5 100644
--- a/lib/Transforms/NaCl/GlobalCleanup.cpp
+++ b/lib/Transforms/NaCl/GlobalCleanup.cpp
@@ -56,6 +56,10 @@ static bool CleanUpLinkage(GlobalValue *GV) {
       GV->eraseFromParent();
       return true;
     }
+    case GlobalValue::WeakAnyLinkage: {
+      GV->setLinkage(GlobalValue::InternalLinkage);
+      return true;
+    }
     default:
       // default with fall through to avoid compiler warning
       return false;
@@ -80,6 +84,11 @@ bool GlobalCleanup::runOnModule(Module &M) {
     GlobalVariable *GV = I++;
     Modified |= CleanUpLinkage(GV);
   }
+
+  for (Module::iterator I = M.begin(), E = M.end(); I != E; ) {
+    Function *F = I++;
+    Modified |= CleanUpLinkage(F);
+  }
   return Modified;
 }
 
diff --git a/test/NaCl/PNaClABI/linkagetypes.ll b/test/NaCl/PNaClABI/linkagetypes.ll
index 7aa0ea8122..e2e2e6127b 100644
--- a/test/NaCl/PNaClABI/linkagetypes.ll
+++ b/test/NaCl/PNaClABI/linkagetypes.ll
@@ -9,7 +9,7 @@ target triple = "le32-unknown-nacl"
 @gv_private = private global i8 0
 @gv_linker_private = linker_private global i32 0
 ; CHECK-NOT: disallowed
-; CHECK: gv_linker_private has disallowed linkage type: linker_private
+; CHECK: Variable gv_linker_private has disallowed linkage type: linker_private
 @gv_linker_private_weak = linker_private_weak global i32 0
 ; CHECK: gv_linker_private_weak has disallowed linkage type: linker_private_weak
 @gv_internal = internal global i8 0
@@ -35,3 +35,40 @@ target triple = "le32-unknown-nacl"
 @gv_extern_weak = extern_weak global i8
 ; CHECK: gv_extern_weak has disallowed linkage type: extern_weak
 @gv_avilable_externally = available_externally global i8 0
+
+; CHECK-NOT: private_func
+define private void @private_func() {
+  ret void
+}
+; internal linkage is allowed, and should not appear in error output.
+; CHECK-NOT: internal_func
+define internal void @internal_func() {
+  ret void
+}
+; TODO(dschuff): Disallow external linkage
+; CHECK-NOT: external_func
+declare external void @external_func()
+; CHECK: linkonce_func has disallowed linkage type: linkonce
+define linkonce void @linkonce_func() {
+  ret void
+}
+; CHECK-NEXT: linkonce_odr_func has disallowed linkage type: linkonce_odr
+define linkonce_odr void @linkonce_odr_func() {
+  ret void
+}
+; CHECK-NEXT: weak_func has disallowed linkage type: weak
+define weak void @weak_func() {
+  ret void
+}
+; CHECK-NEXT: weak_odr_func has disallowed linkage type: weak_odr
+define weak_odr void @weak_odr_func() {
+  ret void
+}
+; CHECK-NEXT: dllimport_func has disallowed linkage type: dllimport
+declare dllimport void @dllimport_func()
+; CHECK-NEXT: dllexport_func has disallowed linkage type: dllexport
+define dllexport void @dllexport_func() {
+  ret void
+}
+; CHECK-NEXT: Function extern_weak_func has disallowed linkage type: extern_weak
+declare extern_weak void @extern_weak_func()
\ No newline at end of file
diff --git a/test/Transforms/NaCl/globalcleanup.ll b/test/Transforms/NaCl/globalcleanup.ll
index 3af5df003e..44e5b45e16 100644
--- a/test/Transforms/NaCl/globalcleanup.ll
+++ b/test/Transforms/NaCl/globalcleanup.ll
@@ -13,7 +13,10 @@
 ; GV-NOT: @extern_weak_const
 ; GV-NOT: @extern_weak_gv
 
-; CHECK define void @_start
+; CHECK: @weak_gv = internal global
+@weak_gv = weak global i32 0
+
+; CHECK: define void @_start
 define void @_start() {
   ret void
 }
@@ -32,3 +35,16 @@ define i32* @ewc() {
   ret i32* @extern_weak_gv
 }
 
+; GV-NOT: @extern_weak_func
+declare extern_weak i32 @extern_weak_func()
+; CHECK: @ewf
+define i32 @ewf() {
+; CHECK: %ret = call i32 null()
+  %ret = call i32 @extern_weak_func()
+  ret i32 %ret
+}
+
+; CHECK: define internal void @weak_func
+define weak void @weak_func() {
+  ret void
+}