Check for metadata in PNaCl ABI checker.

Disallow all metadata by default.  There is a flag "-allow-debug-metadata",
which will be used in pnacl-ld driver, to not change the debugging workflow.
That flag will not be present in the pnacl-abicheck driver though.

We'll also run -strip-metadata within pnacl-ld, after optimizations are run,
so that at least that part is checked inside pnacl-ld.
CL for driver changes:
https://codereview.chromium.org/14358048/

BUG= http://code.google.com/p/nativeclient/issues/detail?id=3348
R=dschuff@chromium.org

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

6 files changed, 86 insertions, 19 deletions

diff --git a/include/llvm/Analysis/NaCl.h b/include/llvm/Analysis/NaCl.h
index 722b4bd7a6..7821894e05 100644
--- a/include/llvm/Analysis/NaCl.h
+++ b/include/llvm/Analysis/NaCl.h
@@ -10,6 +10,7 @@
 #ifndef LLVM_ANALYSIS_NACL_H
 #define LLVM_ANALYSIS_NACL_H
 
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 #include <string>
@@ -18,6 +19,7 @@ namespace llvm {
 
 class FunctionPass;
 class ModulePass;
+extern cl::opt<bool> PNaClABIAllowDebugMetadata;
 
 class PNaClABIErrorReporter {
  public:
@@ -58,8 +60,9 @@ class PNaClABIErrorReporter {
   bool UseFatalErrors;
 };
 
-FunctionPass *createPNaClABIVerifyFunctionsPass(PNaClABIErrorReporter * Reporter);
-ModulePass *createPNaClABIVerifyModulePass(PNaClABIErrorReporter * Reporter);
+FunctionPass *createPNaClABIVerifyFunctionsPass(
+    PNaClABIErrorReporter *Reporter);
+ModulePass *createPNaClABIVerifyModulePass(PNaClABIErrorReporter *Reporter);
 
 }
 
diff --git a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp
index cd3ede4b58..2c9a6a17b9 100644
--- a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp
+++ b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp
@@ -17,6 +17,7 @@
 #include "llvm/Analysis/NaCl.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Instructions.h"
+#include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -49,14 +50,36 @@ class PNaClABIVerifyFunctions : public FunctionPass {
   bool runOnFunction(Function &F);
   virtual void print(raw_ostream &O, const Module *M) const;
  private:
+  bool IsWhitelistedMetadata(unsigned MDKind);
   PNaClABITypeChecker TC;
   PNaClABIErrorReporter *Reporter;
   bool ReporterIsOwned;
 };
 
+// There's no built-in way to get the name of an MDNode, so use a
+// string ostream to print it.
+std::string getMDNodeString(unsigned Kind,
+                            const SmallVectorImpl<StringRef>& MDNames) {
+  std::string MDName;
+  raw_string_ostream N(MDName);
+  if (Kind < MDNames.size()) {
+    N << "!" << MDNames[Kind];
+  } else {
+    N << "!<unknown kind #" << Kind << ">";
+  }
+  return N.str();
+}
+
 } // and anonymous namespace
 
+bool PNaClABIVerifyFunctions::IsWhitelistedMetadata(unsigned MDKind) {
+  return MDKind == LLVMContext::MD_dbg && PNaClABIAllowDebugMetadata;
+}
+
 bool PNaClABIVerifyFunctions::runOnFunction(Function &F) {
+  SmallVector<StringRef, 8> MDNames;
+  F.getContext().getMDKindNames(MDNames);
+
   // TODO: only report one error per instruction?
   for (Function::const_iterator FI = F.begin(), FE = F.end();
            FI != FE; ++FI) {
@@ -181,15 +204,25 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) {
         }
       }
 
-      // Get types hiding in metadata attached to the instruction
+      // Check instruction attachment metadata.
       SmallVector<std::pair<unsigned, MDNode*>, 4> MDForInst;
-      BBI->getAllMetadataOtherThanDebugLoc(MDForInst);
+      BBI->getAllMetadata(MDForInst);
+
       for (unsigned i = 0, e = MDForInst.size(); i != e; i++) {
-        Type *T = TC.checkTypesInMDNode(MDForInst[i].second);
-        if (T) {
-          Reporter->addError() << "Function " << F.getName() <<
-              " has instruction metadata containing disallowed type: " <<
-              PNaClABITypeChecker::getTypeName(T) << "\n";
+        if (!IsWhitelistedMetadata(MDForInst[i].first)) {
+          Reporter->addError()
+              << "Function " << F.getName()
+              << " has disallowed instruction metadata: "
+              << getMDNodeString(MDForInst[i].first, MDNames) << "\n";
+        } else {
+          // If allowed, check the types hiding in the metadata.
+          Type *T = TC.checkTypesInMDNode(MDForInst[i].second);
+          if (T) {
+            Reporter->addError()
+                << "Function " << F.getName()
+                << " has instruction metadata containing disallowed type: "
+                << PNaClABITypeChecker::getTypeName(T) << "\n";
+          }
         }
       }
     }
diff --git a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp
index 88914c85a8..efb40cf43a 100644
--- a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp
+++ b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp
@@ -23,6 +23,13 @@
 #include "PNaClABITypeChecker.h"
 using namespace llvm;
 
+namespace llvm {
+cl::opt<bool>
+PNaClABIAllowDebugMetadata("pnaclabi-allow-debug-metadata",
+  cl::desc("Allow debug metadata during PNaCl ABI verification."),
+  cl::init(false));
+}
+
 namespace {
 // This pass should not touch function bodies, to stay streaming-friendly
 class PNaClABIVerifyModule : public ModulePass {
@@ -48,6 +55,7 @@ class PNaClABIVerifyModule : public ModulePass {
   virtual void print(raw_ostream &O, const Module *M) const;
  private:
   void CheckGlobalValueCommon(const GlobalValue *GV);
+  bool IsWhitelistedMetadata(const NamedMDNode *MD);
   PNaClABITypeChecker TC;
   PNaClABIErrorReporter *Reporter;
   bool ReporterIsOwned;
@@ -105,6 +113,10 @@ void PNaClABIVerifyModule::CheckGlobalValueCommon(const GlobalValue *GV) {
   }
 }
 
+bool PNaClABIVerifyModule::IsWhitelistedMetadata(const NamedMDNode* MD) {
+  return MD->getName().startswith("llvm.dbg.") && PNaClABIAllowDebugMetadata;
+}
+
 bool PNaClABIVerifyModule::runOnModule(Module &M) {
   for (Module::const_global_iterator MI = M.global_begin(), ME = M.global_end();
        MI != ME; ++MI) {
@@ -174,11 +186,17 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) {
   // 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))) {
-        Reporter->addError() << "Named metadata node " << I->getName() <<
-            " refers to disallowed type: " <<
-            PNaClABITypeChecker::getTypeName(T) << "\n";
+    if (!IsWhitelistedMetadata(I)) {
+      Reporter->addError() << "Named metadata node " << I->getName()
+                           << " is disallowed\n";
+    } else {
+      // Check the types in the metadata.
+      for (unsigned i = 0, e = I->getNumOperands(); i != e; i++) {
+        if (Type *T = TC.checkTypesInMDNode(I->getOperand(i))) {
+          Reporter->addError() << "Named metadata node " << I->getName()
+                               << " refers to disallowed type: "
+                               << PNaClABITypeChecker::getTypeName(T) << "\n";
+        }
       }
     }
   }
diff --git a/test/NaCl/PNaClABI/types-function.ll b/test/NaCl/PNaClABI/types-function.ll
index 14267fc6c7..d1ead912cb 100644
--- a/test/NaCl/PNaClABI/types-function.ll
+++ b/test/NaCl/PNaClABI/types-function.ll
@@ -24,7 +24,7 @@ define void @types() {
 ; C;HECK: Function types has instruction operand with disallowed type: i15
 ;  call void @func(i15 1)
 
-; CHECK: Function types has instruction metadata containing disallowed type: half
+; CHECK: Function types has disallowed instruction metadata: !foo
   ret void, !foo !0
 }
 ; CHECK-NOT: disallowed
diff --git a/test/NaCl/PNaClABI/types.ll b/test/NaCl/PNaClABI/types.ll
index 6e2d73e82c..f7685a9d61 100644
--- a/test/NaCl/PNaClABI/types.ll
+++ b/test/NaCl/PNaClABI/types.ll
@@ -1,4 +1,5 @@
 ; RUN: pnacl-abicheck < %s | FileCheck %s
+; RUN: pnacl-abicheck -pnaclabi-allow-debug-metadata < %s | FileCheck %s --check-prefix=DEBUG
 ; Test types allowed by PNaCl ABI
 
 ; Basic global types
@@ -112,6 +113,17 @@ declare void @badArgType1(half %a, i32 %b)
 ; CHECK: Function badArgType2 argument 2 has disallowed type
 declare void @badArgType2(i32 %a, half %b)
 
-; CHECK: Named metadata node namedmd refers to disallowed type
+; If the metadata is allowed we want to check for types.
+; We have a hacky way to test this. The -allow-debug-metadata whitelists debug
+; metadata.  That allows us to check types within debug metadata, even though
+; debug metadata normally does not have illegal types.
+; DEBUG-NOT: Named metadata node llvm.dbg.cu is disallowed
+; DEBUG: Named metadata node llvm.dbg.cu refers to disallowed type: half
+; CHECK: Named metadata node llvm.dbg.cu is disallowed
+!llvm.dbg.cu = !{!0}
 !0 = metadata !{ half 0.0}
-!namedmd = !{!0}
+
+; CHECK: Named metadata node madeup is disallowed
+; DEBUG: Named metadata node madeup is disallowed
+!madeup = !{!1}
+!1 = metadata !{ half 1.0}
diff --git a/tools/pnacl-abicheck/pnacl-abicheck.cpp b/tools/pnacl-abicheck/pnacl-abicheck.cpp
index e8fce66acd..06b174d1b8 100644
--- a/tools/pnacl-abicheck/pnacl-abicheck.cpp
+++ b/tools/pnacl-abicheck/pnacl-abicheck.cpp
@@ -26,7 +26,7 @@ using namespace llvm;
 static cl::opt<std::string>
 InputFilename(cl::Positional, cl::desc("<input bitcode>"), cl::init("-"));
 
-static cl::opt<bool, false>
+static cl::opt<bool>
 Quiet("q", cl::desc("Do not print error messages"));
 
 // Print any errors collected by the error reporter. Return true if
@@ -59,7 +59,8 @@ int main(int argc, char **argv) {
   bool ErrorsFound = false;
   // Manually run the passes so we can tell the user which function had the
   // error. No need for a pass manager since it's just one pass.
-  OwningPtr<ModulePass> ModuleChecker(createPNaClABIVerifyModulePass(&ABIErrorReporter));
+  OwningPtr<ModulePass> ModuleChecker(
+      createPNaClABIVerifyModulePass(&ABIErrorReporter));
   ModuleChecker->runOnModule(*Mod);
   ErrorsFound |= CheckABIVerifyErrors(ABIErrorReporter, "Module");
   OwningPtr<FunctionPass> FunctionChecker(