PNaCl: Turn on ABI verifier by default in sandboxed translator

Change pnacl-llc.cpp to enable the verifier.  This causes two problems
which we fix:

 * The ABI check for declared-but-not-defined functions fails in
   streaming mode.  Fixing this would involve changing the bitcode
   reader.  For now, disable this check when in streaming mode.  Add a
   flag to PNaClABIVerifyModule.

 * ARM's GlobalMerge pass modifies functions' global variable
   references to use ConstantExprs that the ABI checker rejects.
   Address this by disabling GlobalMerge for now.

   GlobalMerge does not provide much benefit at the moment anyway,
   because, with the FlattenGlobals pass applied, GlobalMerge doesn't
   merge variables with alignment >1.

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3465
TEST=PNaCl toolchain trybots

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

4 files changed, 34 insertions, 8 deletions

diff --git a/include/llvm/Analysis/NaCl.h b/include/llvm/Analysis/NaCl.h
index 7821894e05..f174e72608 100644
--- a/include/llvm/Analysis/NaCl.h
+++ b/include/llvm/Analysis/NaCl.h
@@ -62,7 +62,8 @@ class PNaClABIErrorReporter {
 
 FunctionPass *createPNaClABIVerifyFunctionsPass(
     PNaClABIErrorReporter *Reporter);
-ModulePass *createPNaClABIVerifyModulePass(PNaClABIErrorReporter *Reporter);
+ModulePass *createPNaClABIVerifyModulePass(PNaClABIErrorReporter *Reporter,
+                                           bool StreamingMode = false);
 
 }
 
diff --git a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp
index 675b62a974..3a422288d8 100644
--- a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp
+++ b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp
@@ -50,10 +50,12 @@ class PNaClABIVerifyModule : public ModulePass {
       ReporterIsOwned(true) {
     initializePNaClABIVerifyModulePass(*PassRegistry::getPassRegistry());
   }
-  explicit PNaClABIVerifyModule(PNaClABIErrorReporter *Reporter_) :
+  explicit PNaClABIVerifyModule(PNaClABIErrorReporter *Reporter_,
+                                bool StreamingMode) :
       ModulePass(ID),
       Reporter(Reporter_),
-      ReporterIsOwned(false) {
+      ReporterIsOwned(false),
+      StreamingMode(StreamingMode) {
     initializePNaClABIVerifyModulePass(*PassRegistry::getPassRegistry());
   }
   ~PNaClABIVerifyModule() {
@@ -69,6 +71,7 @@ class PNaClABIVerifyModule : public ModulePass {
   void checkGlobalIsFlattened(const GlobalVariable *GV);
   PNaClABIErrorReporter *Reporter;
   bool ReporterIsOwned;
+  bool StreamingMode;
 };
 
 static const char *linkageName(GlobalValue::LinkageTypes LT) {
@@ -392,7 +395,12 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) {
             << PNaClABITypeChecker::getTypeName(MI->getFunctionType())
             << "\n";
       }
-      if (MI->isDeclaration()) {
+      // This check is disabled in streaming mode because it would
+      // reject a function that is defined but not read in yet.
+      // Unfortunately this means we simply don't check this property
+      // when translating a pexe in the browser.
+      // TODO(mseaborn): Enforce this property in the bitcode reader.
+      if (!StreamingMode && MI->isDeclaration()) {
         Reporter->addError() << "Function " << MI->getName()
                              << " is declared but not defined (disallowed)\n";
       }
@@ -451,6 +459,6 @@ INITIALIZE_PASS(PNaClABIVerifyModule, "verify-pnaclabi-module",
                 "Verify module for PNaCl", false, true)
 
 ModulePass *llvm::createPNaClABIVerifyModulePass(
-    PNaClABIErrorReporter *Reporter) {
-  return new PNaClABIVerifyModule(Reporter);
+    PNaClABIErrorReporter *Reporter, bool StreamingMode) {
+  return new PNaClABIVerifyModule(Reporter, StreamingMode);
 }
diff --git a/lib/Target/ARM/ARMTargetMachine.cpp b/lib/Target/ARM/ARMTargetMachine.cpp
index 499f974b96..23a2b3ad99 100644
--- a/lib/Target/ARM/ARMTargetMachine.cpp
+++ b/lib/Target/ARM/ARMTargetMachine.cpp
@@ -155,8 +155,18 @@ TargetPassConfig *ARMBaseTargetMachine::createPassConfig(PassManagerBase &PM) {
 }
 
 bool ARMPassConfig::addPreISel() {
-  if (TM->getOptLevel() != CodeGenOpt::None && EnableGlobalMerge)
+  // @LOCALMOD-START
+  // We disable the GlobalMerge pass for PNaCl because it causes the
+  // PNaCl ABI checker to reject the program when the PNaCl translator
+  // is run in streaming mode.  This is because GlobalMerge replaces
+  // functions' GlobalVariable references with ConstantExprs which the
+  // ABI verifier rejects.
+  // TODO(mseaborn): Make the ABI checks coexist with GlobalMerge to
+  // get back the performance benefits of GlobalMerge.
+  if (TM->getOptLevel() != CodeGenOpt::None && EnableGlobalMerge &&
+      !getARMSubtarget().isTargetNaCl())
     addPass(createGlobalMergePass(TM->getTargetLowering()));
+  // @LOCALMOD-END
 
   return false;
 }
diff --git a/tools/pnacl-llc/pnacl-llc.cpp b/tools/pnacl-llc/pnacl-llc.cpp
index 84f216914a..a2a6f3ec05 100644
--- a/tools/pnacl-llc/pnacl-llc.cpp
+++ b/tools/pnacl-llc/pnacl-llc.cpp
@@ -258,6 +258,12 @@ int llc_main(int argc, char **argv) {
   // Register the target printer for --version.
   cl::AddExtraVersionPrinter(TargetRegistry::printRegisteredTargetsForVersion);
 
+  // Enable the PNaCl ABI verifier by default in sandboxed mode.
+#if defined(__native_client__)
+  PNaClABIVerify = true;
+  PNaClABIVerifyFatalErrors = true;
+#endif
+
   cl::ParseCommandLineOptions(argc, argv, "llvm system compiler\n");
 
   // Compile the module TimeCompilations times to give better compile time
@@ -329,7 +335,8 @@ static int compileModule(char **argv, LLVMContext &Context) {
     // @LOCALMOD-BEGIN
     if (PNaClABIVerify) {
       // Verify the module (but not the functions yet)
-      ModulePass *VerifyPass = createPNaClABIVerifyModulePass(&ABIErrorReporter);
+      ModulePass *VerifyPass = createPNaClABIVerifyModulePass(&ABIErrorReporter,
+                                                              LazyBitcode);
       VerifyPass->runOnModule(*mod);
       CheckABIVerifyErrors(ABIErrorReporter, "Module");
     }