Check that address is i32 for indirect calls.
Fixes bug where code did not check that the address of an indirect call must be i32.
BUG= https://code.google.com/p/nativeclient/issues/detail?id=4321
R=stichnot@chromium.org
Review URL: https://codereview.chromium.org/1363983002 .
diff --git a/src/PNaClTranslator.cpp b/src/PNaClTranslator.cpp
index d792865..ba9eabb 100644
--- a/src/PNaClTranslator.cpp
+++ b/src/PNaClTranslator.cpp
@@ -1598,7 +1598,7 @@
std::string Buffer;
raw_string_ostream StrBuf(Buffer);
StrBuf << InstructionName << " address not " << PtrType
- << ". Found: " << *Op;
+ << ". Found: " << Op->getType();
Error(StrBuf.str());
return false;
}
@@ -2048,6 +2048,24 @@
return LocalOperands.front();
return Context->getGlobalConstantByID(0);
}
+
+ void verifyCallArgTypeMatches(Ice::FunctionDeclaration *Fcn,
+ Ice::SizeT Index, Ice::Type ArgType,
+ Ice::Type ParamType) {
+ if (ArgType != ParamType) {
+ std::string Buffer;
+ raw_string_ostream StrBuf(Buffer);
+ StrBuf << "Argument " << (Index + 1) << " of " << printName(Fcn)
+ << " expects " << ParamType << ". Found: " << ArgType;
+ Error(StrBuf.str());
+ }
+ }
+
+ const Ice::IceString printName(Ice::FunctionDeclaration *Fcn) {
+ if (Fcn)
+ return Fcn->getName();
+ return "function";
+ }
};
void FunctionParser::ExitBlock() {
@@ -2660,46 +2678,116 @@
ParamsStartIndex = 3;
}
- // Extract out the called function and its return type.
uint32_t CalleeIndex = convertRelativeToAbsIndex(Values[1], BaseIndex);
Ice::Operand *Callee = getOperand(CalleeIndex);
+
+ // Pull out signature/return type of call (if possible).
+ Ice::FunctionDeclaration *Fcn = nullptr;
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();
+ Ice::SizeT NumParams = Values.size() - ParamsStartIndex;
+ if (NumParams != Signature->getNumArgs()) {
+ std::string Buffer;
+ raw_string_ostream StrBuf(Buffer);
+ StrBuf << "Call to " << printName(Fcn) << " has " << NumParams
+ << " parameters. Signature expects: " << Signature->getNumArgs();
+ Error(StrBuf.str());
+ if (ReturnType != Ice::IceType_void)
+ setNextLocalInstIndex(nullptr);
+ return;
+ }
// 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);
+ Fcn->getName(), BadIntrinsic);
if (BadIntrinsic) {
std::string Buffer;
raw_string_ostream StrBuf(Buffer);
- StrBuf << "Invalid PNaCl intrinsic call to " << Name;
+ StrBuf << "Invalid PNaCl intrinsic call to " << Fcn->getName();
+ Error(StrBuf.str());
+ IntrinsicInfo = nullptr;
+ }
+ if (IntrinsicInfo && IntrinsicInfo->getNumArgs() != NumParams) {
+ std::string Buffer;
+ raw_string_ostream StrBuf(Buffer);
+ StrBuf << "Call to " << printName(Fcn) << " has " << NumParams
+ << " parameters. Intrinsic expects: " << Signature->getNumArgs();
Error(StrBuf.str());
if (ReturnType != Ice::IceType_void)
- appendErrorInstruction(ReturnType);
+ setNextLocalInstIndex(nullptr);
return;
}
- } else {
+ } else { // Record.GetCode() == naclbitc::FUNC_CODE_INST_CALL_INDIRECT
+ // There is no signature. Assume defined by parameter types.
ReturnType = Context->getSimpleTypeByID(Values[2]);
+ if (!isIRGenerationDisabled() && Callee != nullptr)
+ isValidPointerType(Callee, "Call indirect");
+ }
+
+ if (Callee == nullptr && !isIRGenerationDisabled())
+ return;
+
+ // Extract out the the call parameters.
+ SmallVector<Ice::Operand*, 8> Params;
+ for (Ice::SizeT Index = ParamsStartIndex; Index < Values.size(); ++Index) {
+ Ice::Operand *Op = getRelativeOperand(Values[Index], BaseIndex);
+ if (isIRGenerationDisabled())
+ continue;
+ if (Op == nullptr) {
+ std::string Buffer;
+ raw_string_ostream StrBuf(Buffer);
+ StrBuf << "Parameter " << (Index - ParamsStartIndex + 1)
+ << " of " << printName(Fcn) << " is not defined";
+ Error(StrBuf.str());
+ if (ReturnType != Ice::IceType_void)
+ setNextLocalInstIndex(nullptr);
+ return;
+ }
+ Params.push_back(Op);
}
// 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;
+ StrBuf << "Return type of " << printName(Fcn) << " is invalid: "
+ << ReturnType;
Error(StrBuf.str());
ReturnType = Ice::IceType_i32;
}
+ if (isIRGenerationDisabled()) {
+ if (ReturnType != Ice::IceType_void)
+ setNextLocalInstIndex(nullptr);
+ return;
+ }
+
+ // Type check call parameters.
+ for (Ice::SizeT Index = 0; Index < Params.size(); ++Index) {
+ Ice::Operand *Op = Params[Index];
+ Ice::Type OpType = Op->getType();
+ if (Signature)
+ verifyCallArgTypeMatches(
+ Fcn, Index, OpType, Signature->getArgType(Index));
+ if (IntrinsicInfo) {
+ verifyCallArgTypeMatches(Fcn, Index, OpType,
+ IntrinsicInfo->getArgType(Index));
+ } else if (!isCallParameterType(OpType)) {
+ std::string Buffer;
+ raw_string_ostream StrBuf(Buffer);
+ StrBuf << "Argument " << *Op << " of " << printName(Fcn)
+ << " has invalid type: " << Op->getType();
+ Error(StrBuf.str());
+ appendErrorInstruction(ReturnType);
+ }
+ }
+
// Extract call information.
uint64_t CCInfo = Values[0];
CallingConv::ID CallingConv;
@@ -2709,127 +2797,25 @@
StrBuf << "Function call calling convention value " << (CCInfo >> 1)
<< " not understood.";
Error(StrBuf.str());
- if (ReturnType != Ice::IceType_void)
- appendErrorInstruction(ReturnType);
+ appendErrorInstruction(ReturnType);
return;
}
bool IsTailCall = static_cast<bool>(CCInfo & 1);
- Ice::SizeT NumParams = Values.size() - ParamsStartIndex;
- if (Signature && NumParams != Signature->getNumArgs()) {
- std::string Buffer;
- raw_string_ostream StrBuf(Buffer);
- StrBuf << "Call has " << NumParams
- << " parameters. Signature expects: " << Signature->getNumArgs();
- Error(StrBuf.str());
- // Error recover by only checking parameters in both signature and call.
- NumParams = std::min(NumParams, Signature->getNumArgs());
- }
- if (isIRGenerationDisabled()) {
- assert(Callee == nullptr);
- // Check that parameters are defined.
- for (Ice::SizeT ParamIndex = 0; ParamIndex < NumParams; ++ParamIndex) {
- assert(getRelativeOperand(Values[ParamsStartIndex + ParamIndex],
- BaseIndex) == nullptr);
- }
- // Define value slot only if value returned.
- if (ReturnType != Ice::IceType_void)
- setNextLocalInstIndex(nullptr);
- return;
- }
+
// Create the call instruction.
Ice::Variable *Dest = (ReturnType == Ice::IceType_void)
? nullptr
: getNextInstVar(ReturnType);
std::unique_ptr<Ice::InstCall> Inst;
if (IntrinsicInfo) {
- Inst.reset(Ice::InstIntrinsicCall::create(Func.get(), NumParams, Dest,
+ Inst.reset(Ice::InstIntrinsicCall::create(Func.get(), Params.size(), Dest,
Callee, IntrinsicInfo->Info));
} else {
- Inst.reset(Ice::InstCall::create(Func.get(), NumParams, Dest, Callee,
+ Inst.reset(Ice::InstCall::create(Func.get(), Params.size(), Dest, Callee,
IsTailCall));
}
-
- // Add parameters.
- for (Ice::SizeT ParamIndex = 0; ParamIndex < NumParams; ++ParamIndex) {
- Ice::Operand *Op =
- getRelativeOperand(Values[ParamsStartIndex + ParamIndex], BaseIndex);
- if (Op == nullptr) {
- std::string Buffer;
- raw_string_ostream StrBuf(Buffer);
- StrBuf << "Parameter " << ParamIndex << " of call not defined";
- Error(StrBuf.str());
- if (ReturnType != Ice::IceType_void)
- appendErrorInstruction(ReturnType);
- return;
- }
-
- // Check that parameter type is valid.
- if (Signature) {
- if (Op->getType() != Signature->getArgType(ParamIndex)) {
- std::string Buffer;
- raw_string_ostream StrBuf(Buffer);
- StrBuf << "Call argument " << *Op << " expects "
- << Signature->getArgType(ParamIndex)
- << ". Found: " << Op->getType();
- Error(StrBuf.str());
- } else if (IntrinsicInfo == nullptr &&
- !isCallParameterType(Op->getType())) {
- // TODO(kschimpf): Move this check to the function declaration, so
- // that it only needs to be checked once.
- std::string Buffer;
- raw_string_ostream StrBuf(Buffer);
- StrBuf << "Call argument " << *Op
- << " matches declaration but has invalid type: "
- << Op->getType();
- Error(StrBuf.str());
- }
- } else if (!isCallParameterType(Op->getType())) {
- std::string Buffer;
- raw_string_ostream StrBuf(Buffer);
- StrBuf << "Call argument " << *Op
- << " has invalid type: " << Op->getType();
- Error(StrBuf.str());
- }
- Inst->addArg(Op);
- }
-
- // If intrinsic call, validate call signature.
- if (IntrinsicInfo) {
- Ice::SizeT ArgIndex = 0;
- switch (IntrinsicInfo->validateCall(Inst.get(), ArgIndex)) {
- case Ice::Intrinsics::IsValidCall:
- break;
- case Ice::Intrinsics::BadReturnType: {
- std::string Buffer;
- raw_string_ostream StrBuf(Buffer);
- StrBuf << "Intrinsic " << Fcn->getName() << " expects return type"
- << IntrinsicInfo->getReturnType()
- << ". Found: " << Inst->getReturnType();
- Error(StrBuf.str());
- break;
- }
- case Ice::Intrinsics::WrongNumOfArgs: {
- std::string Buffer;
- raw_string_ostream StrBuf(Buffer);
- StrBuf << "Intrinsic " << Fcn->getName() << " expects "
- << IntrinsicInfo->getNumArgs()
- << ". Found: " << Inst->getNumArgs();
- Error(StrBuf.str());
- break;
- }
- case Ice::Intrinsics::WrongCallArgType: {
- std::string Buffer;
- raw_string_ostream StrBuf(Buffer);
- StrBuf << "Intrinsic " << Fcn->getName() << " expects "
- << IntrinsicInfo->getArgType(ArgIndex) << " for argument "
- << (ArgIndex + 1)
- << ". Found: " << Inst->getArg(ArgIndex)->getType();
- Error(StrBuf.str());
- break;
- }
- }
- }
-
+ for (Ice::Operand *Param : Params)
+ Inst->addArg(Param);
CurrentNode->appendInst(Inst.release());
return;
}
diff --git a/tests_lit/llvm2ice_tests/Input/no-terminator-inst.tbc b/tests_lit/llvm2ice_tests/Input/no-terminator-inst.tbc
index 1f89762..112c372 100644
--- a/tests_lit/llvm2ice_tests/Input/no-terminator-inst.tbc
+++ b/tests_lit/llvm2ice_tests/Input/no-terminator-inst.tbc
@@ -24,7 +24,7 @@
11,1,2,1;
10,2;
2,3,2,1;
-34,0,5,0;
+34,0,5,1;
2,1,5,0;
65534;
5534;
diff --git a/tests_lit/parse_errs/Inputs/indirect-call-on-float.tbc b/tests_lit/parse_errs/Inputs/indirect-call-on-float.tbc
new file mode 100644
index 0000000..2dac5d4
--- /dev/null
+++ b/tests_lit/parse_errs/Inputs/indirect-call-on-float.tbc
@@ -0,0 +1,25 @@
+65535,8,2;
+1,1;
+65535,17,2;
+1,5;
+2;
+21,0,0;
+7,32;
+3;
+21,0,0,2,3;
+65534;
+8,1,0,1,0;
+8,4,0,0,0;
+65535,19,2;
+5,0;
+65534;
+65535,14,2;
+1,1,102;
+1,0,103;
+65534;
+65535,12,2;
+1,1;
+44,0,1,0;
+10;
+65534;
+65534;
diff --git a/tests_lit/parse_errs/bad-intrinsic-arg.test b/tests_lit/parse_errs/bad-intrinsic-arg.test
index 6b540e9..6063841 100644
--- a/tests_lit/parse_errs/bad-intrinsic-arg.test
+++ b/tests_lit/parse_errs/bad-intrinsic-arg.test
@@ -7,7 +7,7 @@
; RUN: -bitcode-format=pnacl -notranslate -build-on-read 2>&1 \
; RUN: | FileCheck %s
-; CHECK: Intrinsic llvm.nacl.setjmp expects i32 for argument 1. Found: double
+; CHECK: Argument 1 of llvm.nacl.setjmp expects i32. Found: double
; RUN: pnacl-bcfuzz -bitcode-as-text \
; RUN: %p/Inputs/bad-intrinsic-arg.tbc -output - \
diff --git a/tests_lit/parse_errs/call-fcn-bad-param-type.ll b/tests_lit/parse_errs/call-fcn-bad-param-type.ll
index 2314732..46c3ab7 100644
--- a/tests_lit/parse_errs/call-fcn-bad-param-type.ll
+++ b/tests_lit/parse_errs/call-fcn-bad-param-type.ll
@@ -10,6 +10,6 @@
define void @Test() {
entry:
call void @f(i8 1)
-; CHECK: Call argument 1 matches declaration but has invalid type: i8
+; CHECK: Argument 1 of f has invalid type: i8
ret void
}
diff --git a/tests_lit/parse_errs/call-fcn-bad-return-type.ll b/tests_lit/parse_errs/call-fcn-bad-return-type.ll
index b1c8cd8..4154c7d 100644
--- a/tests_lit/parse_errs/call-fcn-bad-return-type.ll
+++ b/tests_lit/parse_errs/call-fcn-bad-return-type.ll
@@ -10,7 +10,7 @@
define void @Test() {
entry:
%v = call i1 @f()
-; CHECK: Return type of called function is invalid: i1
+; CHECK: Return type of f is invalid: i1
ret void
}
diff --git a/tests_lit/parse_errs/call-indirect-i8.ll b/tests_lit/parse_errs/call-indirect-i8.ll
index 0a3b2bc..2c46ba1 100644
--- a/tests_lit/parse_errs/call-indirect-i8.ll
+++ b/tests_lit/parse_errs/call-indirect-i8.ll
@@ -8,6 +8,6 @@
entry:
%f = inttoptr i32 %f_addr to i32(i8)*
%r = call i32 %f(i8 1)
-; CHECK: Call argument 1 has invalid type: i8
+; CHECK: Argument 1 of function has invalid type: i8
ret void
}
diff --git a/tests_lit/parse_errs/indirect-call-on-float.test b/tests_lit/parse_errs/indirect-call-on-float.test
new file mode 100644
index 0000000..ef30de6
--- /dev/null
+++ b/tests_lit/parse_errs/indirect-call-on-float.test
@@ -0,0 +1,21 @@
+; Tests that we check the call address is a pointer on an indirect call.
+
+; REQUIRES: no_minimal_build
+
+; RUN: not %pnacl_sz -bitcode-as-text \
+; RUN: %p/Inputs/indirect-call-on-float.tbc \
+; RUN: -bitcode-format=pnacl -notranslate -build-on-read 2>&1 \
+; RUN: | FileCheck %s
+
+; CHECK: Call indirect address not i32. Found: float
+
+; RUN: pnacl-bcfuzz -bitcode-as-text \
+; RUN: %p/Inputs/indirect-call-on-float.tbc -output - \
+; RUN: | not pnacl-bcdis -no-records | FileCheck -check-prefix=ASM %s
+
+; ASM: function void @f1(i32 %p0, float %p1) { // BlockID = 12
+; ASM: blocks 1;
+; ASM: %b0:
+; ASM: call void %p1();
+; ASM: ret void;
+; ASM: }
diff --git a/tests_lit/parse_errs/symtab-after-fcn.test b/tests_lit/parse_errs/symtab-after-fcn.test
index 197c87b..4e519f1 100644
--- a/tests_lit/parse_errs/symtab-after-fcn.test
+++ b/tests_lit/parse_errs/symtab-after-fcn.test
@@ -9,11 +9,13 @@
; CHECK: Module valuesymtab not allowed after function blocks
; RUN: pnacl-bcfuzz -bitcode-as-text %p/Inputs/symtab-after-fcn.tbc \
-; RUN: -output - | pnacl-bcdis -no-records | FileCheck -check-prefix=ASM %s
+; RUN: -output - | not pnacl-bcdis -no-records \
+; RUN: | FileCheck -check-prefix=ASM %s
; ASM: module { // BlockID = 8
; ASM: function void @f0() { // BlockID = 12
; ASM: }
; ASM: valuesymtab { // BlockID = 14
+; ASM: Error({{.*}}): Module symbol table must appear before function blocks
; ASM: }
; ASM: }
