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

7 files changed, 94 insertions, 33 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);
diff --git a/test/NaCl/PNaClABI/abi-small-arguments.ll b/test/NaCl/PNaClABI/abi-small-arguments.ll
new file mode 100644
index 0000000000..890b84c42a
--- /dev/null
+++ b/test/NaCl/PNaClABI/abi-small-arguments.ll
@@ -0,0 +1,45 @@
+; RUN: pnacl-abicheck < %s | FileCheck %s
+
+define void @arg_i1(i1 %bad) {
+  ret void
+}
+; CHECK: Function arg_i1 has disallowed type:
+
+define void @arg_i16(i32 %allowed, i16 %bad) {
+  ret void
+}
+; CHECK: Function arg_i16 has disallowed type:
+
+define i1 @return_i1() {
+  ret i1 0
+}
+; CHECK: Function return_i1 has disallowed type:
+
+define i8 @return_i8() {
+  ret i8 0
+}
+; CHECK: Function return_i8 has disallowed type:
+
+
+; Direct calls currently do not produce errors because the functions
+; are deemed to have already been flagged.
+; CHECK-NOT: disallowed
+define void @bad_direct_calls() {
+  call void @arg_i1(i1 0)
+  call void @arg_i16(i32 0, i16 0)
+  %result1 = call i1 @return_i1()
+  %result2 = call i8 @return_i8()
+  ret void
+}
+
+define void @bad_indirect_calls(i32 %ptr) {
+  %func1 = inttoptr i32 %ptr to void (i8)*
+  call void %func1(i8 0)
+; CHECK: Function bad_indirect_calls has instruction with disallowed type: void (i8)*
+
+  %func2 = inttoptr i32 %ptr to i16 ()*
+  %result3 = call i16 %func2()
+; CHECK: Function bad_indirect_calls has instruction with disallowed type: i16 ()*
+
+  ret void
+}
diff --git a/test/NaCl/PNaClABI/abi-varargs.ll b/test/NaCl/PNaClABI/abi-varargs.ll
index 2a299f5493..6eda0b9b53 100644
--- a/test/NaCl/PNaClABI/abi-varargs.ll
+++ b/test/NaCl/PNaClABI/abi-varargs.ll
@@ -3,11 +3,11 @@
 define void @varargs_func(i32 %arg, ...) {
   ret void
 }
-; CHECK: Function varargs_func is a variable-argument function (disallowed)
+; CHECK: Function varargs_func has disallowed type: void (i32, ...)
 
 define void @call_varargs_func(i32 %ptr) {
   %ptr2 = inttoptr i32 %ptr to void (i32, ...)*
   call void (i32, ...)* %ptr2(i32 123)
   ret void
 }
-; CHECK: Function call_varargs_func contains a disallowed varargs function call
+; CHECK: Function call_varargs_func has instruction with disallowed type: void (i32, ...)*
diff --git a/test/NaCl/PNaClABI/types.ll b/test/NaCl/PNaClABI/types.ll
index f7685a9d61..802e02b5ff 100644
--- a/test/NaCl/PNaClABI/types.ll
+++ b/test/NaCl/PNaClABI/types.ll
@@ -105,12 +105,12 @@ some_block:
 ; CHECK: Variable alias1 is an alias (disallowed)
 @alias1 = alias i32* @i32
 
-; CHECK: Function badReturn has disallowed return type
+; CHECK: Function badReturn has disallowed type: half* ()
 declare half* @badReturn()
 
-; CHECK: Function badArgType1 argument 1 has disallowed type
+; CHECK: Function badArgType1 has disallowed type: void (half, i32)
 declare void @badArgType1(half %a, i32 %b)
-; CHECK: Function badArgType2 argument 2 has disallowed type
+; CHECK: Function badArgType2 has disallowed type: void (i32, half)
 declare void @badArgType2(i32 %a, half %b)
 
 ; If the metadata is allowed we want to check for types.