ABI verifier: Add standalone tool pnacl-abicheck

lib/Analysis/NaCl/PNaClABIVerifyModule.cpp

Index: lib/Analysis/NaCl/PNaClABIVerifyModule.cpp
diff --git a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp
index 59addab087d0ae85ce6c9c53fa8b1c40fef60075..60ce60a33617348ffe4998385aa6c958e868f376 100644
--- a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp
+++ b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp
@@ -25,16 +25,20 @@ using namespace llvm;
 
 namespace {
 // This pass should not touch function bodies, to stay streaming-friendly
-class PNaClABIVerifyModule : public ModulePass {
+class PNaClABIVerifyModule : public ModulePass, public PNaClABIVerifierPass {

[jvoung (off chromium), 2013/03/12 22:16:21]

Only the module Pass seems to inherit from PNaClABIVerifierPass.  Should the
function pass also inherit?  How will PNaClABIVerifierPass interface be used?

[Derek Schuff, 2013/03/13 21:51:01]

Both should inherit, and use is demonstrated in llc.cpp now. The standalone
verifier will do something similar.
On 2013/03/12 22:16:21, jvoung (cr) wrote:

  public:
   static char ID;
   PNaClABIVerifyModule();
   bool runOnModule(Module &M);
   virtual void print(raw_ostream &O, const Module *M) const;
+  virtual void setStrict(bool Strict_) { Strict = Strict_; }
+  virtual int getErrorCount() { return ErrorCount; }
  private:
   PNaClABITypeChecker TC;
   std::string ErrorsString;
   raw_string_ostream Errors;
+  bool Strict;
+  int ErrorCount;
 };
 
 static const char *linkageName(GlobalValue::LinkageTypes LT) {
@@ -66,9 +70,13 @@ static const char *linkageName(GlobalValue::LinkageTypes LT) {
 } // end anonymous namespace
 
 PNaClABIVerifyModule::PNaClABIVerifyModule() : ModulePass(ID),
-                                               Errors(ErrorsString) {}
+                                               Errors(ErrorsString),
+                                               Strict(false),
+                                               ErrorCount(0) {}
 
 bool PNaClABIVerifyModule::runOnModule(Module &M) {
+  ErrorsString.clear();
+  ErrorCount = 0;
   for (Module::const_global_iterator MI = M.global_begin(), ME = M.global_end();
        MI != ME; ++MI) {
     // Check types of global variables and their initializers
@@ -78,13 +86,16 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) {
           " has disallowed type: " +
           PNaClABITypeChecker::getTypeName(MI->getType()->getContainedType(0))
           + "\n";
+      ErrorCount++;

[jvoung (off chromium), 2013/03/12 22:16:21]

Perhaps just combine in one method the adding of an error to the Errors, with
counting up ErrorCount?

[Derek Schuff, 2013/03/13 21:51:01]

Yeah i thought about that but the problem is that it breaks the nice ability to
chain the << operators to the Errors stream (which is ok if all the tokens are
strings as they are here, but below some of them are ints). I'm open to ideas
here.
On 2013/03/12 22:16:21, jvoung (cr) wrote:

[jvoung (off chromium), 2013/03/13 22:40:09]

The new version seems good.  If you are going to use <<, then perhaps that could
be used more uniformly over the +.


On 2013/03/13 21:51:01, Derek Schuff wrote:

[Derek Schuff, 2013/03/13 22:52:23]

Done.

     } 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)
+      if (T) {
         Errors << "Initializer for " + MI->getName() +
             " has disallowed type: " +
             PNaClABITypeChecker::getTypeName(T) + "\n";
+        ErrorCount++;
+      }
     }
 
     // Check GV linkage types
@@ -98,25 +109,32 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) {
         Errors << "Variable " + MI->getName() +
             " has disallowed linkage type: " +
             linkageName(MI->getLinkage()) + "\n";
+        ErrorCount++;
     }
   }
   // No aliases allowed for now.
   for (Module::alias_iterator MI = M.alias_begin(),
-           E = M.alias_end(); MI != E; ++MI)
+           E = M.alias_end(); MI != E; ++MI) {
     Errors << "Variable " + MI->getName() + " is an alias (disallowed)\n";
+    ErrorCount++;
+  }
 
   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()))
+    if (!TC.isValidType(FT->getReturnType())) {
       Errors << "Function " + MI->getName() + " has disallowed return type: " +
           PNaClABITypeChecker::getTypeName(FT->getReturnType()) + "\n";
+      ErrorCount++;
+    }
     for (unsigned I = 0, E = FT->getNumParams(); I < E; ++I) {
       Type *PT = FT->getParamType(I);
-      if (!TC.isValidType(PT))
+      if (!TC.isValidType(PT)) {
         Errors << "Function " << MI->getName() << " argument " << I + 1 <<
             " has disallowed type: " <<
             PNaClABITypeChecker::getTypeName(PT) + "\n";
+        ErrorCount++;
+      }
     }
   }
 
@@ -124,13 +142,18 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) {
   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)))
+      if (Type *T = TC.checkTypesInMDNode(I->getOperand(i))) {
         Errors << "Named metadata node " + I->getName() +
             " refers to disallowed type: " +
             PNaClABITypeChecker::getTypeName(T) + "\n";
+        ErrorCount++;
+      }
     }
   }
   Errors.flush();
+  if (Strict && ErrorCount > 0) {
+    report_fatal_error("Module is not valid PNaCl bitcode:\n" + ErrorsString);
+  }
   return false;
 }
 
@@ -143,6 +166,8 @@ 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(bool Strict) {
+  PNaClABIVerifyModule * P = new PNaClABIVerifyModule();

[Mark Seaborn, 2013/03/12 22:25:56]

Spacing should be "*P"

[Derek Schuff, 2013/03/13 21:51:01]

Done.

+  P->setStrict(Strict);
+  return P;
 }