Check that address is i32 for indirect calls.

src/PNaClTranslator.cpp

 //===- subzero/src/PNaClTranslator.cpp - ICE from bitcode -----------------===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
@@ skipping 1580 matching lines @@
   // Checks if the type of operand Op is the valid pointer type, for the given
   // InstructionName. Returns true if valid. Otherwise generates an error
   // message and returns false.
   bool isValidPointerType(Ice::Operand *Op, const char *InstructionName) {
     Ice::Type PtrType = Ice::getPointerType();
     if (Op->getType() == PtrType)
       return true;
     std::string Buffer;
     raw_string_ostream StrBuf(Buffer);
     StrBuf << InstructionName << " address not " << PtrType
-           << ". Found: " << *Op;
+           << ". Found: " << Op->getType();
     Error(StrBuf.str());
     return false;
   }
 
   // Checks if loading/storing a value of type Ty is allowed. Returns true if
   // Valid. Otherwise generates an error message and returns false.
   bool isValidLoadStoreType(Ice::Type Ty, const char *InstructionName) {
     if (isLoadStoreType(Ty))
       return true;
     std::string Buffer;
@@ skipping 1043 matching lines @@
       if (!isValidRecordSizeAtLeast(2, "call"))
         return;
     } else {
       if (!isValidRecordSizeAtLeast(3, "call indirect"))
         return;
       ParamsStartIndex = 3;
     }
 
     // Extract out the called function and its return type.
     uint32_t CalleeIndex = convertRelativeToAbsIndex(Values[1], BaseIndex);
     Ice::Operand *Callee = getOperand(CalleeIndex);

[Jim Stichnoth, 2015/09/23 20:40:06]

Would it make sense to check Callee for valid type here, i.e. do the check for
both direct and indirect calls?

[Karl, 2015/09/24 21:28:22]

Type signatures are explicitly defined for direct calls. They aren't defined for
indirect call. Rather, the parts are implied by the types of the parameters, and
a record field defining the return type. The question is whether we should
consume cycles (and memory) generating the type signature for the indirect call.

The code is also complex due to having to handle no IR generation, as well.

However, I agree this code is complex. So I've tried to simplify it down into
more logical steps.

     const Ice::FuncSigType *Signature = nullptr;
     Ice::Type ReturnType = Ice::IceType_void;
     const Ice::Intrinsics::FullIntrinsicInfo *IntrinsicInfo = nullptr;
     // Name of function if a direct call/intrinsic. Null otherwise.
     Ice::FunctionDeclaration *Fcn = nullptr;
     if (Record.GetCode() == naclbitc::FUNC_CODE_INST_CALL) {
       Fcn = Context->getFunctionByID(CalleeIndex);
       Signature = &Fcn->getSignature();
       ReturnType = Signature->getReturnType();
 
       // Check if this direct call is to an Intrinsic (starts with "llvm.")
       bool BadIntrinsic;
       const Ice::IceString &Name = Fcn->getName();
       IntrinsicInfo = getTranslator().getContext()->getIntrinsicsInfo().find(
           Name, BadIntrinsic);
       if (BadIntrinsic) {
         std::string Buffer;
         raw_string_ostream StrBuf(Buffer);
         StrBuf << "Invalid PNaCl intrinsic call to " << Name;
         Error(StrBuf.str());
         if (ReturnType != Ice::IceType_void)
           appendErrorInstruction(ReturnType);
         return;
       }
     } else {
       ReturnType = Context->getSimpleTypeByID(Values[2]);
+      if (!isValidPointerType(Callee, "Call indirect")) {
+        if (ReturnType != Ice::IceType_void)
+          appendErrorInstruction(ReturnType);

[Jim Stichnoth, 2015/09/23 20:40:06]

Should you return after this error?  Or fall through and possibly add another
error instruction?

[Karl, 2015/09/24 21:28:22]

The code in general, should error recover if possible. It should only return if
there is no recovery, or it is too difficult to do.

+      }
     }
 
     // Check return type.
     if (IntrinsicInfo == nullptr && !isCallReturnType(ReturnType)) {
       std::string Buffer;
       raw_string_ostream StrBuf(Buffer);
       StrBuf << "Return type of called function is invalid: " << ReturnType;
       Error(StrBuf.str());
       ReturnType = Ice::IceType_i32;
     }
@@ skipping 594 matching lines @@
     raw_string_ostream StrBuf(Buffer);
     StrBuf << IRFilename << ": Does not contain a module!";
     llvm::report_fatal_error(StrBuf.str());
   }
   if (InputStreamFile.getBitcodeBytes().getExtent() % 4 != 0) {
     llvm::report_fatal_error("Bitcode stream should be a multiple of 4 bytes");
   }
 }
 
 } // end of namespace Ice