ABI verifier: Add standalone tool pnacl-abicheck

lib/Analysis/NaCl/PNaClABIVerifyModule.cpp

 //===- PNaClABIVerifyModule.cpp - Verify PNaCl ABI rules --------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // Verify module-level PNaCl ABI requirements (specifically those that do not
 // require looking at the function bodies)
 //
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Pass.h"
 #include "llvm/Analysis/NaCl.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include "PNaClABITypeChecker.h"
 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) {
   // This logic is taken from PrintLinkage in lib/VMCore/AsmWriter.cpp
   switch (LT) {
     case GlobalValue::ExternalLinkage: return "external";
     case GlobalValue::PrivateLinkage:       return "private ";
     case GlobalValue::LinkerPrivateLinkage: return "linker_private ";
     case GlobalValue::LinkerPrivateWeakLinkage: return "linker_private_weak ";
     case GlobalValue::InternalLinkage:      return "internal ";
@@ skipping 11 matching lines @@
     case GlobalValue::AvailableExternallyLinkage:
       return "available_externally ";
     default:
       return "unknown";
   }
 }
 
 } // 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
     if (!TC.isValidType(MI->getType())) {
       // GVs are pointers, so print the pointed-to type for clarity
       Errors << "Variable " + MI->getName() +
           " 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
     switch (MI->getLinkage()) {
       case GlobalValue::ExternalLinkage:
       case GlobalValue::AvailableExternallyLinkage:
       case GlobalValue::InternalLinkage:
       case GlobalValue::PrivateLinkage:
         break;
       default:
         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++;
+      }
     }
   }
 
   // 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)))
+      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;
 }
 
 void PNaClABIVerifyModule::print(llvm::raw_ostream &O, const Module *M) const {
   O << ErrorsString;
 }
 
 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;
 }