PNaCl ABI checker: Reject integer arguments smaller than i32

We can simplify the varargs checks by doing the checking on the
FunctionType rather than having separate checks for the Function and
the CallInst.  (We couldn't do that prior to enabling
ReplacePtrsWithInts because at that point the IR contained bitcasts to
and from varargs FunctionTypes.)

That simplification means we can also add the check for
argument/return types in a single place too.

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3342
TEST=*.ll tests + PNaCl toolchain trybots + GCC torture tests + LLVM test suite

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

4 files changed, 44 insertions, 28 deletions

diff --git a/lib/Analysis/NaCl/PNaClABITypeChecker.cpp b/lib/Analysis/NaCl/PNaClABITypeChecker.cpp
index 8501a4f9e2..064f7862b1 100644
--- a/lib/Analysis/NaCl/PNaClABITypeChecker.cpp
+++ b/lib/Analysis/NaCl/PNaClABITypeChecker.cpp
@@ -20,6 +20,33 @@
 
 using namespace llvm;
 
+bool PNaClABITypeChecker::isValidParamType(const Type *Ty) {
+  if (!isValidType(Ty))
+    return false;
+  if (const IntegerType *IntTy = dyn_cast<IntegerType>(Ty)) {
+    // PNaCl requires function arguments and return values to be 32
+    // bits or larger.  This avoids exposing architecture
+    // ABI-dependent differences about whether arguments or return
+    // values are zero-extended when calling a function with the wrong
+    // prototype.
+    if (IntTy->getBitWidth() < 32)
+      return false;
+  }
+  return true;
+}
+
+bool PNaClABITypeChecker::isValidFunctionType(const FunctionType *FTy) {
+  if (FTy->isVarArg())
+    return false;
+  if (!isValidParamType(FTy->getReturnType()))
+    return false;
+  for (unsigned I = 0, E = FTy->getNumParams(); I < E; ++I) {
+    if (!isValidParamType(FTy->getParamType(I)))
+      return false;
+  }
+  return true;
+}
+
 bool PNaClABITypeChecker::isValidType(const Type *Ty) {
   if (VisitedTypes.count(Ty))
     return VisitedTypes[Ty];
@@ -59,6 +86,8 @@ bool PNaClABITypeChecker::isValidType(const Type *Ty) {
       //          Width == 32 || Width == 64);
       break;
     case Type::FunctionTyID:
+      Valid = isValidFunctionType(cast<FunctionType>(Ty));
+      break;
     case Type::StructTyID:
     case Type::ArrayTyID:
     case Type::PointerTyID:
diff --git a/lib/Analysis/NaCl/PNaClABITypeChecker.h b/lib/Analysis/NaCl/PNaClABITypeChecker.h
index eed2dc76b6..cd898e2328 100644
--- a/lib/Analysis/NaCl/PNaClABITypeChecker.h
+++ b/lib/Analysis/NaCl/PNaClABITypeChecker.h
@@ -21,10 +21,17 @@
 
 namespace llvm {
 class Constant;
+class FunctionType;
 class MDNode;
 class Value;
 
 class PNaClABITypeChecker {
+  // Returns true if Ty is a valid argument or return value type for PNaCl.
+  bool isValidParamType(const Type *Ty);
+
+  // Returns true if Ty is a valid function type for PNaCl.
+  bool isValidFunctionType(const FunctionType *FTy);
+
  public:
   // Returns true if Ty is a valid type for PNaCl.
   bool isValidType(const Type *Ty);
diff --git a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp
index 2082c85d01..d1d5f38405 100644
--- a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp
+++ b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp
@@ -165,14 +165,6 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) {
             Reporter->addError() << "Function " << F.getName() <<
                 " contains disallowed inline assembly\n";
           }
-          // Pointers to varargs function types are not yet
-          // disallowed, but we do disallow defining or calling
-          // functions of varargs types.
-          if (cast<CallInst>(BBI)->getCalledValue()->getType()
-              ->getPointerElementType()->isFunctionVarArg()) {
-            Reporter->addError() << "Function " << F.getName() <<
-                " contains a disallowed varargs function call\n";
-          }
           break;
       }
       // Check the types. First check the type of the instruction.
diff --git a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp
index d651e7bda7..0e406e6512 100644
--- a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp
+++ b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp
@@ -293,26 +293,14 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) {
                            << " is a disallowed LLVM intrinsic\n";
     }
 
-    // Check types of functions and their arguments
-    FunctionType *FT = MI->getFunctionType();
-    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)) {
-        Reporter->addError() << "Function " << MI->getName() << " argument " <<
-            I + 1 << " has disallowed type: " <<
-            PNaClABITypeChecker::getTypeName(PT) << "\n";
-      }
-    }
-    // Pointers to varargs function types are not yet disallowed, but
-    // we do disallow defining or calling functions of varargs types.
-    if (MI->isVarArg()) {
-      Reporter->addError() << "Function " << MI->getName() <<
-          " is a variable-argument function (disallowed)\n";
+    // Check types of functions and their arguments.  Not necessary
+    // for intrinsics, whose types are fixed anyway, and which have
+    // argument types that we disallow such as i8.
+    if (!MI->isIntrinsic() && !TC.isValidType(MI->getType())) {
+      Reporter->addError() << "Function " << MI->getName()
+          << " has disallowed type: "
+          << PNaClABITypeChecker::getTypeName(MI->getFunctionType())
+          << "\n";
     }
 
     checkGlobalValueCommon(MI);