ABI verifier: Add standalone tool pnacl-abicheck

This CL adds a standalone tool pnacl-abicheck for checking the ABI validity of a bitcode file.
It also adds a flag pnaclabi-verify to llc to enable the same checking in llc.
It also improves the mechanism for handling and reporting validation errors and uses it in both tools.

[email protected],[email protected]
BUG= https://code.google.com/p/nativeclient/issues/detail?id=2309

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

(cherry picked from commit cb0dcd0)

Conflicts:
	lib/Analysis/NaCl/PNaClABIVerifyModule.cpp
	tools/CMakeLists.txt
	tools/LLVMBuild.txt
	tools/Makefile
	tools/llc/llc.cpp

Author: dschuff

include/llvm/Analysis/NaCl.h

@@ -10,13 +10,44 @@
 #ifndef LLVM_ANALYSIS_NACL_H
 #define LLVM_ANALYSIS_NACL_H
 
+#include "llvm/Support/raw_ostream.h"
+#include <string>
+
 namespace llvm {
 
 class FunctionPass;
 class ModulePass;
 
-FunctionPass *createPNaClABIVerifyFunctionsPass();
-ModulePass *createPNaClABIVerifyModulePass();
+class PNaClABIErrorReporter {
+ public:
+  PNaClABIErrorReporter() : ErrorCount(0), Errors(ErrorString) {}
+  // Return the number of verification errors from the last run.
+  int getErrorCount() { return ErrorCount; }
+  // Print the error messages to O
+  void printErrors(llvm::raw_ostream &O) {
+    Errors.flush();
+    O << ErrorString;
+  }
+  // Increments the error count and returns an ostream to which the error
+  // message can be streamed.
+  raw_ostream &addError() {
+    ErrorCount++;
+    return Errors;
+  }
+  // Reset the error count and error messages.
+  void reset() {
+    ErrorCount = 0;
+    Errors.flush();
+    ErrorString.clear();
+  }
+ private:
+  int ErrorCount;
+  std::string ErrorString;
+  raw_string_ostream Errors;
+};
+
+FunctionPass *createPNaClABIVerifyFunctionsPass(PNaClABIErrorReporter * Reporter);
+ModulePass *createPNaClABIVerifyModulePass(PNaClABIErrorReporter * Reporter);
 
 }
 

lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp

@@ -14,11 +14,11 @@
 
 #include "llvm/Pass.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Analysis/NaCl.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/Support/raw_ostream.h"
-#include "llvm/Analysis/NaCl.h"
 
 #include "PNaClABITypeChecker.h"
 using namespace llvm;
@@ -30,22 +30,29 @@ namespace {
 class PNaClABIVerifyFunctions : public FunctionPass {
  public:
   static char ID;
-  PNaClABIVerifyFunctions() : FunctionPass(ID), Errors(ErrorsString) {}
+  PNaClABIVerifyFunctions() : FunctionPass(ID),
+                              Reporter(new PNaClABIErrorReporter),
+                              ReporterIsOwned(true) {}
+  explicit PNaClABIVerifyFunctions(PNaClABIErrorReporter *Reporter_) :
+      FunctionPass(ID),
+      Reporter(Reporter_),
+      ReporterIsOwned(false) {}
+  ~PNaClABIVerifyFunctions() {
+    if (ReporterIsOwned)
+      delete Reporter;
+  }
   bool runOnFunction(Function &F);
   virtual void print(raw_ostream &O, const Module *M) const;
  private:
   PNaClABITypeChecker TC;
-  std::string ErrorsString;
-  raw_string_ostream Errors;
+  PNaClABIErrorReporter *Reporter;
+  bool ReporterIsOwned;
 };
 
 } // and anonymous namespace
 
 bool PNaClABIVerifyFunctions::runOnFunction(Function &F) {
-  // For now just start with new errors on each function; this may change
-  // once we want to do something with them other than just calling print()
-  ErrorsString.clear();
-  // TODO: only report one error per instruction
+  // TODO: only report one error per instruction?
   for (Function::const_iterator FI = F.begin(), FE = F.end();
            FI != FE; ++FI) {
     for (BasicBlock::const_iterator BBI = FI->begin(), BBE = FI->end();
@@ -59,9 +66,9 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) {
         case Instruction::ExtractElement:
         case Instruction::InsertElement:
         case Instruction::ShuffleVector:
-          Errors << "Function " + F.getName() +
-              " has disallowed instruction: " +
-              BBI->getOpcodeName() + "\n";
+          Reporter->addError() << "Function " << F.getName() <<
+              " has disallowed instruction: " <<
+              BBI->getOpcodeName() << "\n";
           break;
 
         // Terminator instructions
@@ -126,9 +133,9 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) {
       }
       // Check the types. First check the type of the instruction.
       if (!TC.isValidType(BBI->getType())) {
-        Errors << "Function " + F.getName() +
-            " has instruction with disallowed type: " +
-            PNaClABITypeChecker::getTypeName(BBI->getType()) + "\n";
+        Reporter->addError() << "Function " << F.getName() <<
+            " has instruction with disallowed type: " <<
+            PNaClABITypeChecker::getTypeName(BBI->getType()) << "\n";
       }
 
       // Check the instruction operands. Operands which are Instructions will
@@ -141,10 +148,11 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) {
              OI != OE; OI++) {
           if (isa<Constant>(OI) && !isa<GlobalValue>(OI)) {
             Type *T = TC.checkTypesInConstant(cast<Constant>(*OI));
-            if (T)
-              Errors << "Function " + F.getName() +
-                  " has instruction operand with disallowed type: " +
-                  PNaClABITypeChecker::getTypeName(T) + "\n";
+            if (T) {
+              Reporter->addError() << "Function " << F.getName() <<
+                  " has instruction operand with disallowed type: " <<
+                  PNaClABITypeChecker::getTypeName(T) << "\n";
+            }
           }
         }
 
@@ -153,28 +161,33 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) {
       BBI->getAllMetadataOtherThanDebugLoc(MDForInst);
       for (unsigned i = 0, e = MDForInst.size(); i != e; i++) {
         Type *T = TC.checkTypesInMDNode(MDForInst[i].second);
-        if (T)
-          Errors << "Function " + F.getName() +
-              " has instruction metadata containing disallowed type: " +
-              PNaClABITypeChecker::getTypeName(T) + "\n";
+        if (T) {
+          Reporter->addError() << "Function " << F.getName() <<
+              " has instruction metadata containing disallowed type: " <<
+              PNaClABITypeChecker::getTypeName(T) << "\n";
+        }
       }
     }
   }
 
-  Errors.flush();
   return false;
 }
 
+// This method exists so that the passes can easily be run with opt -analyze.
+// In this case the default constructor is used and we want to reset the error
+// messages after each print.
 void PNaClABIVerifyFunctions::print(llvm::raw_ostream &O, const Module *M)
     const {
-  O << ErrorsString;
+  Reporter->printErrors(O);
+  Reporter->reset();
 }
 
 char PNaClABIVerifyFunctions::ID = 0;
 
 static RegisterPass<PNaClABIVerifyFunctions> X("verify-pnaclabi-functions",
     "Verify functions for PNaCl", false, false);
 
-FunctionPass *llvm::createPNaClABIVerifyFunctionsPass() {
-  return new PNaClABIVerifyFunctions();
+FunctionPass *llvm::createPNaClABIVerifyFunctionsPass(
+    PNaClABIErrorReporter *Reporter) {
+  return new PNaClABIVerifyFunctions(Reporter);
 }

lib/Analysis/NaCl/PNaClABIVerifyModule.cpp

@@ -13,13 +13,12 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/ADT/Twine.h"
-#include "llvm/DerivedTypes.h"
-#include "llvm/Module.h"
 #include "llvm/Pass.h"
-
-#include "llvm/Support/raw_ostream.h"
+#include "llvm/ADT/Twine.h"
 #include "llvm/Analysis/NaCl.h"
+#include "llvm/IR/DerivedTypes.h"
+#include "llvm/IR/Module.h"
+#include "llvm/Support/raw_ostream.h"
 
 #include "PNaClABITypeChecker.h"
 using namespace llvm;
@@ -29,13 +28,23 @@ namespace {
 class PNaClABIVerifyModule : public ModulePass {
  public:
   static char ID;
-  PNaClABIVerifyModule();
+  PNaClABIVerifyModule() : ModulePass(ID),
+                           Reporter(new PNaClABIErrorReporter),
+                           ReporterIsOwned(true) {}
+  explicit PNaClABIVerifyModule(PNaClABIErrorReporter *Reporter_) :
+      ModulePass(ID),
+      Reporter(Reporter_),
+      ReporterIsOwned(false) {}
+  ~PNaClABIVerifyModule() {
+    if (ReporterIsOwned)
+      delete Reporter;
+  }
   bool runOnModule(Module &M);
   virtual void print(raw_ostream &O, const Module *M) const;
  private:
   PNaClABITypeChecker TC;
-  std::string ErrorsString;
-  raw_string_ostream Errors;
+  PNaClABIErrorReporter *Reporter;
+  bool ReporterIsOwned;
 };
 
 static const char *linkageName(GlobalValue::LinkageTypes LT) {
@@ -66,26 +75,24 @@ static const char *linkageName(GlobalValue::LinkageTypes LT) {
 
 } // end anonymous namespace
 
-PNaClABIVerifyModule::PNaClABIVerifyModule() : ModulePass(ID),
-                                               Errors(ErrorsString) {}
-
 bool PNaClABIVerifyModule::runOnModule(Module &M) {
   for (Module::const_global_iterator MI = M.global_begin(), ME = M.global_end();
        MI != ME; ++MI) {
     // Check types of global variables and their initializers
     if (!TC.isValidType(MI->getType())) {
       // GVs are pointers, so print the pointed-to type for clarity
-      Errors << "Variable " + MI->getName() +
-          " has disallowed type: " +
+      Reporter->addError() << "Variable " << MI->getName() <<
+          " has disallowed type: " <<
           PNaClABITypeChecker::getTypeName(MI->getType()->getContainedType(0))
           + "\n";
     } else if (MI->hasInitializer()) {
       // If the type of the global is bad, no point in checking its initializer
       Type *T = TC.checkTypesInConstant(MI->getInitializer());
-      if (T)
-        Errors << "Initializer for " + MI->getName() +
-            " has disallowed type: " +
-            PNaClABITypeChecker::getTypeName(T) + "\n";
+      if (T) {
+        Reporter->addError() << "Initializer for " << MI->getName() <<
+            " has disallowed type: " <<
+            PNaClABITypeChecker::getTypeName(T) << "\n";
+      }
     }
 
     // Check GV linkage types
@@ -96,54 +103,65 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) {
       case GlobalValue::PrivateLinkage:
         break;
       default:
-        Errors << "Variable " + MI->getName() +
-            " has disallowed linkage type: " +
-            linkageName(MI->getLinkage()) + "\n";
+        Reporter->addError() << "Variable " << MI->getName() <<
+            " has disallowed linkage type: " <<
+            linkageName(MI->getLinkage()) << "\n";
     }
   }
   // No aliases allowed for now.
   for (Module::alias_iterator MI = M.alias_begin(),
-           E = M.alias_end(); MI != E; ++MI)
-    Errors << "Variable " + MI->getName() + " is an alias (disallowed)\n";
+           E = M.alias_end(); MI != E; ++MI) {
+    Reporter->addError() << "Variable " << MI->getName() <<
+        " is an alias (disallowed)\n";
+  }
 
   for (Module::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()))
-      Errors << "Function " + MI->getName() + " has disallowed return type: " +
-          PNaClABITypeChecker::getTypeName(FT->getReturnType()) + "\n";
+    if (!TC.isValidType(FT->getReturnType())) {
+      Reporter->addError() << "Function " << MI->getName() <<
+          " has disallowed return type: " <<
+          PNaClABITypeChecker::getTypeName(FT->getReturnType()) << "\n";
+    }
     for (unsigned I = 0, E = FT->getNumParams(); I < E; ++I) {
       Type *PT = FT->getParamType(I);
-      if (!TC.isValidType(PT))
-        Errors << "Function " << MI->getName() << " argument " << I + 1 <<
-            " has disallowed type: " <<
-            PNaClABITypeChecker::getTypeName(PT) + "\n";
+      if (!TC.isValidType(PT)) {
+        Reporter->addError() << "Function " << MI->getName() << " argument " <<
+            I + 1 << " has disallowed type: " <<
+            PNaClABITypeChecker::getTypeName(PT) << "\n";
+      }
     }
   }
 
   // Check named metadata nodes
   for (Module::const_named_metadata_iterator I = M.named_metadata_begin(),
            E = M.named_metadata_end(); I != E; ++I) {
     for (unsigned i = 0, e = I->getNumOperands(); i != e; i++) {
-      if (Type *T = TC.checkTypesInMDNode(I->getOperand(i)))
-        Errors << "Named metadata node " + I->getName() +
-            " refers to disallowed type: " +
-            PNaClABITypeChecker::getTypeName(T) + "\n";
+      if (Type *T = TC.checkTypesInMDNode(I->getOperand(i))) {
+        Reporter->addError() << "Named metadata node " << I->getName() <<
+            " refers to disallowed type: " <<
+            PNaClABITypeChecker::getTypeName(T) << "\n";
+      }
     }
   }
-  Errors.flush();
   return false;
 }
 
+// This method exists so that the passes can easily be run with opt -analyze.
+// In this case the default constructor is used and we want to reset the error
+// messages after each print (this is more of an issue for the FunctionPass
+// than the ModulePass)
 void PNaClABIVerifyModule::print(llvm::raw_ostream &O, const Module *M) const {
-  O << ErrorsString;
+  Reporter->printErrors(O);
+  Reporter->reset();
 }
 
 char PNaClABIVerifyModule::ID = 0;
 
 static RegisterPass<PNaClABIVerifyModule> X("verify-pnaclabi-module",
     "Verify module for PNaCl", false, false);
 
-ModulePass *llvm::createPNaClABIVerifyModulePass() {
-  return new PNaClABIVerifyModule();
+ModulePass *llvm::createPNaClABIVerifyModulePass(
+    PNaClABIErrorReporter *Reporter) {
+  return new PNaClABIVerifyModule(Reporter);
 }

tools/CMakeLists.txt

@@ -38,6 +38,7 @@ add_llvm_tool_subdirectory(llvm-stress)
 add_llvm_tool_subdirectory(llvm-mcmarkup)
 
 add_llvm_tool_subdirectory(llvm-symbolizer)
+add_subdirectory(pnacl-abicheck)
 
 add_llvm_tool_subdirectory(llvm-c-test)
 

tools/LLVMBuild.txt

@@ -16,7 +16,7 @@
 ;===------------------------------------------------------------------------===;
 
 [common]
-subdirectories = bugpoint llc lli llvm-ar llvm-as llvm-bcanalyzer llvm-cov llvm-diff llvm-dis llvm-dwarfdump llvm-extract llvm-jitlistener llvm-link llvm-lto llvm-mc llvm-nm llvm-objdump llvm-rtdyld llvm-size macho-dump opt llvm-mcmarkup
+subdirectories = bugpoint llc lli llvm-ar llvm-as llvm-bcanalyzer llvm-cov llvm-diff llvm-dis llvm-dwarfdump llvm-extract llvm-jitlistener llvm-link llvm-lto llvm-mc llvm-nm llvm-objdump llvm-rtdyld llvm-size macho-dump opt llvm-mcmarkup pnacl-abicheck
 
 [component_0]
 type = Group

tools/Makefile

@@ -31,7 +31,7 @@ PARALLEL_DIRS := opt llvm-as llvm-dis llc llvm-ar llvm-nm llvm-link \
                  lli llvm-extract llvm-mc bugpoint llvm-bcanalyzer llvm-diff \
                  macho-dump llvm-objdump llvm-readobj llvm-rtdyld \
                  llvm-dwarfdump llvm-cov llvm-size llvm-stress llvm-mcmarkup \
-                 llvm-symbolizer obj2yaml yaml2obj llvm-c-test
+                 llvm-symbolizer obj2yaml yaml2obj llvm-c-test pnacl-abicheck
 
 # If Intel JIT Events support is configured, build an extra tool to test it.
 ifeq ($(USE_INTEL_JITEVENTS), 1)