Skip to content

S
swiftshader
Commit ff94f59a by Karl Schimpf
Options

Fix call instructions to check parameter types for consistency.

Checks each parameter of a call against the corresponding function signature. It also checks that parameters are valid PNaCl parameter types, unless the call is to an intrinsic. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4309 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1354673002 .
parent a83bfde6
Showing with 194 additions and 72 deletions
src/IceTypes.cpp
......@@ -37,7 +37,8 @@ enum {
};
// Define a temporary set of enum values based on ICETYPE_PROPS_TABLE
enum {
#define X(tag, IsVec, IsInt, IsFloat, IsIntArith, IsLoadStore, CompareResult) \
#define X(tag, IsVec, IsInt, IsFloat, IsIntArith, IsLoadStore, IsParam, \
CompareResult) \
_props_table_tag_##tag,
ICETYPE_PROPS_TABLE
#undef X
......@@ -51,7 +52,8 @@ enum {
ICETYPE_TABLE
#undef X
// Assert that tags in ICETYPE_PROPS_TABLE is in ICETYPE_TABLE.
#define X(tag, IsVec, IsInt, IsFloat, IsIntArith, IsLoadStore, CompareResult) \
#define X(tag, IsVec, IsInt, IsFloat, IsIntArith, IsLoadStore, IsParam, \
CompareResult) \
static_assert( \
(unsigned)_table_tag_##tag == (unsigned)_props_table_tag_##tag, \
"Inconsistency between ICETYPE_PROPS_TABLE and ICETYPE_TABLE");
......@@ -69,13 +71,15 @@ enum {
};
// Define constants for boolean flag if vector in ICETYPE_PROPS_TABLE.
enum {
#define X(tag, IsVec, IsInt, IsFloat, IsIntArith, IsLoadStore, CompareResult) \
#define X(tag, IsVec, IsInt, IsFloat, IsIntArith, IsLoadStore, IsParam, \
CompareResult) \
_props_table_IsVec_##tag = IsVec,
ICETYPE_PROPS_TABLE
#undef X
};
// Verify that the number of vector elements is consistent with IsVec.
#define X(tag, IsVec, IsInt, IsFloat, IsIntArith, IsLoadStore, CompareResult) \
#define X(tag, IsVec, IsInt, IsFloat, IsIntArith, IsLoadStore, IsParam, \
CompareResult) \
static_assert((_table_elts_##tag > 1) == _props_table_IsVec_##tag, \
"Inconsistent vector specification in ICETYPE_PROPS_TABLE");
ICETYPE_PROPS_TABLE
......@@ -107,14 +111,16 @@ struct TypePropertyFields {
bool TypeIsScalarFloatingType;
bool TypeIsVectorFloatingType;
bool TypeIsLoadStoreType;
bool TypeIsCallParameterType;
Type CompareResultType;
};
const TypePropertyFields TypePropertiesTable[] = {
#define X(tag, IsVec, IsInt, IsFloat, IsIntArith, IsLoadStore, CompareResult) \
#define X(tag, IsVec, IsInt, IsFloat, IsIntArith, IsLoadStore, IsParam, \
CompareResult) \
{ \
IsVec, IsInt, IsInt & !IsVec, IsInt & IsVec, IsIntArith, IsFloat, \
IsFloat & !IsVec, IsFloat & IsVec, IsLoadStore, CompareResult \
IsFloat & !IsVec, IsFloat & IsVec, IsLoadStore, IsParam, CompareResult \
} \
,
ICETYPE_PROPS_TABLE
......@@ -240,6 +246,14 @@ bool isLoadStoreType(Type Ty) {
return false;
}
bool isCallParameterType(Type Ty) {
size_t Index = static_cast<size_t>(Ty);
if (Index < IceType_NUM)
return TypePropertiesTable[Index].TypeIsCallParameterType;
llvm_unreachable("Invalid type for isCallParameterType()");
return false;
}
Type getCompareResultType(Type Ty) {
size_t Index = static_cast<size_t>(Ty);
if (Index < IceType_NUM)
......
src/IceTypes.def
......@@ -19,33 +19,33 @@
// sandboxes, but for now the 64-bit architectures use ELF64:
// https://code.google.com/p/nativeclient/issues/detail?id=349 TODO: Whoever
// adds AArch64 will need to set ABI e_flags.
#define TARGETARCH_TABLE \
/* enum value, printable string, is_elf64, e_machine, e_flags */ \
X(Target_X8632, "x86-32", false, EM_386, 0) \
X(Target_X8664, "x86-64", true, EM_X86_64, 0) \
X(Target_ARM32, "arm32", false, EM_ARM, EF_ARM_EABI_VER5) \
X(Target_ARM64, "arm64", true, EM_AARCH64, 0) \
X(Target_MIPS32,"mips32", false, EM_MIPS, 0) \
//#define X(tag, str, is_elf64, e_machine, e_flags)
#define TARGETARCH_TABLE \
/* enum value, printable string, is_elf64, e_machine, e_flags */ \
X(Target_X8632, "x86-32", false, EM_386, 0) \
X(Target_X8664, "x86-64", true, EM_X86_64, 0) \
X(Target_ARM32, "arm32", false, EM_ARM, EF_ARM_EABI_VER5) \
X(Target_ARM64, "arm64", true, EM_AARCH64, 0) \
X(Target_MIPS32,"mips32", false, EM_MIPS, 0) \
//#define X(tag, str, is_elf64, e_machine, e_flags)
#define ICETYPE_TABLE \
/* enum value, log_2(size), align, # elts, element type, printable */ \
/* string (size and alignment in bytes) */ \
X(IceType_void, -1, 0, 1, IceType_void, "void") \
X(IceType_i1, 0, 1, 1, IceType_i1, "i1") \
X(IceType_i8, 0, 1, 1, IceType_i8, "i8") \
X(IceType_i16, 1, 1, 1, IceType_i16, "i16") \
X(IceType_i32, 2, 1, 1, IceType_i32, "i32") \
X(IceType_i64, 3, 1, 1, IceType_i64, "i64") \
X(IceType_f32, 2, 4, 1, IceType_f32, "float") \
X(IceType_f64, 3, 8, 1, IceType_f64, "double") \
X(IceType_v4i1, 4, 1, 4, IceType_i1, "<4 x i1>") \
X(IceType_v8i1, 4, 1, 8, IceType_i1, "<8 x i1>") \
X(IceType_v16i1, 4, 1, 16, IceType_i1, "<16 x i1>") \
X(IceType_v16i8, 4, 1, 16, IceType_i8, "<16 x i8>") \
X(IceType_v8i16, 4, 2, 8, IceType_i16, "<8 x i16>") \
X(IceType_v4i32, 4, 4, 4, IceType_i32, "<4 x i32>") \
X(IceType_v4f32, 4, 4, 4, IceType_f32, "<4 x float>") \
#define ICETYPE_TABLE \
/* enum value, log_2(size), align, # elts, element type, printable */ \
/* string (size and alignment in bytes) */ \
X(IceType_void, -1, 0, 1, IceType_void, "void") \
X(IceType_i1, 0, 1, 1, IceType_i1, "i1") \
X(IceType_i8, 0, 1, 1, IceType_i8, "i8") \
X(IceType_i16, 1, 1, 1, IceType_i16, "i16") \
X(IceType_i32, 2, 1, 1, IceType_i32, "i32") \
X(IceType_i64, 3, 1, 1, IceType_i64, "i64") \
X(IceType_f32, 2, 4, 1, IceType_f32, "float") \
X(IceType_f64, 3, 8, 1, IceType_f64, "double") \
X(IceType_v4i1, 4, 1, 4, IceType_i1, "<4 x i1>") \
X(IceType_v8i1, 4, 1, 8, IceType_i1, "<8 x i1>") \
X(IceType_v16i1, 4, 1, 16, IceType_i1, "<16 x i1>") \
X(IceType_v16i8, 4, 1, 16, IceType_i8, "<16 x i8>") \
X(IceType_v8i16, 4, 2, 8, IceType_i16, "<8 x i16>") \
X(IceType_v4i32, 4, 4, 4, IceType_i32, "<4 x i32>") \
X(IceType_v4f32, 4, 4, 4, IceType_f32, "<4 x float>") \
//#define X(tag, sizeLog2, align, elts, elty, str)
// Dictionary:
......@@ -54,25 +54,27 @@
// F - Is floating point value (scalar or vector).
// IA - Is integer arithmetic type
// LS - true if load/store allowed on type.
// P - true if can be used for parameter of call.
// CR - Result type of compare instruction for argument type
// (IceType_void if disallowed)
#define ICETYPE_PROPS_TABLE \
/* Enum Value V I F IA LS CR */ \
X(IceType_void, 0, 0, 0, 0, 0, IceType_void) \
X(IceType_i1, 0, 1, 0, 0, 0, IceType_i1) \
X(IceType_i8, 0, 1, 0, 1, 1, IceType_i1) \
X(IceType_i16, 0, 1, 0, 1, 1, IceType_i1) \
X(IceType_i32, 0, 1, 0, 1, 1, IceType_i1) \
X(IceType_i64, 0, 1, 0, 1, 1, IceType_i1) \
X(IceType_f32, 0, 0, 1, 0, 1, IceType_i1) \
X(IceType_f64, 0, 0, 1, 0, 1, IceType_i1) \
X(IceType_v4i1, 1, 1, 0, 0, 0, IceType_v4i1) \
X(IceType_v8i1, 1, 1, 0, 0, 0, IceType_v8i1) \
X(IceType_v16i1, 1, 1, 0, 0, 0, IceType_v16i1) \
X(IceType_v16i8, 1, 1, 0, 1, 1, IceType_v16i1) \
X(IceType_v8i16, 1, 1, 0, 1, 1, IceType_v8i1) \
X(IceType_v4i32, 1, 1, 0, 1, 1, IceType_v4i1) \
X(IceType_v4f32, 1, 0, 1, 0, 1, IceType_v4i1) \
//#define X(tag, IsVec, IsInt, IsFloat, IsIntArith, IsLoadStore, CompareResult)
#define ICETYPE_PROPS_TABLE \
/* Enum Value V I F IA LS P CR */ \
X(IceType_void, 0, 0, 0, 0, 0, 0, IceType_void) \
X(IceType_i1, 0, 1, 0, 0, 0, 0, IceType_i1) \
X(IceType_i8, 0, 1, 0, 1, 1, 0, IceType_i1) \
X(IceType_i16, 0, 1, 0, 1, 1, 0, IceType_i1) \
X(IceType_i32, 0, 1, 0, 1, 1, 1, IceType_i1) \
X(IceType_i64, 0, 1, 0, 1, 1, 1, IceType_i1) \
X(IceType_f32, 0, 0, 1, 0, 1, 1, IceType_i1) \
X(IceType_f64, 0, 0, 1, 0, 1, 1, IceType_i1) \
X(IceType_v4i1, 1, 1, 0, 0, 0, 1, IceType_v4i1) \
X(IceType_v8i1, 1, 1, 0, 0, 0, 1, IceType_v8i1) \
X(IceType_v16i1, 1, 1, 0, 0, 0, 1, IceType_v16i1) \
X(IceType_v16i8, 1, 1, 0, 1, 1, 1, IceType_v16i1) \
X(IceType_v8i16, 1, 1, 0, 1, 1, 1, IceType_v8i1) \
X(IceType_v4i32, 1, 1, 0, 1, 1, 1, IceType_v4i1) \
X(IceType_v4f32, 1, 0, 1, 0, 1, 1, IceType_v4i1) \
//#define X(tag, IsVec, IsInt, IsFloat, IsIntArith, IsLoadStore, IsParam, \
// CompareResult)
#endif // SUBZERO_SRC_ICETYPES_DEF
src/IceTypes.h
......@@ -81,6 +81,14 @@ bool isVectorFloatingType(Type Ty);
/// Returns true if the given type can be used in a load instruction.
bool isLoadStoreType(Type Ty);
/// Returns true if the given type can be used as a parameter type in a call.
bool isCallParameterType(Type Ty);
/// Returns true if the given type can be used as the return type of a call.
inline bool isCallReturnType(Type Ty) {
return Ty == IceType_void || isCallParameterType(Ty);
}
/// Returns type generated by applying the compare instructions (icmp and fcmp)
/// to arguments of the given type. Returns IceType_void if compare is not
/// allowed.
......
src/PNaClTranslator.cpp
......@@ -2644,12 +2644,13 @@ void FunctionParser::ProcessRecord() {
// Extract out the called function and its return type.
uint32_t CalleeIndex = convertRelativeToAbsIndex(Values[1], BaseIndex);
Ice::Operand *Callee = getOperand(CalleeIndex);
const Ice::FuncSigType *Signature = nullptr;
Ice::Type ReturnType = Ice::IceType_void;
const Ice::Intrinsics::FullIntrinsicInfo *IntrinsicInfo = nullptr;
if (Record.GetCode() == naclbitc::FUNC_CODE_INST_CALL) {
Ice::FunctionDeclaration *Fcn = Context->getFunctionByID(CalleeIndex);
const Ice::FuncSigType &Signature = Fcn->getSignature();
ReturnType = Signature.getReturnType();
Signature = &Fcn->getSignature();
ReturnType = Signature->getReturnType();
// Check if this direct call is to an Intrinsic (starts with "llvm.")
bool BadIntrinsic;
......@@ -2661,13 +2662,23 @@ void FunctionParser::ProcessRecord() {
raw_string_ostream StrBuf(Buffer);
StrBuf << "Invalid PNaCl intrinsic call to " << Name;
Error(StrBuf.str());
appendErrorInstruction(ReturnType);
if (ReturnType != Ice::IceType_void)
appendErrorInstruction(ReturnType);
return;
}
} else {
ReturnType = Context->getSimpleTypeByID(Values[2]);
}
// 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;
}
// Extract call information.
uint64_t CCInfo = Values[0];
CallingConv::ID CallingConv;
......@@ -2677,12 +2688,21 @@ void FunctionParser::ProcessRecord() {
StrBuf << "Function call calling convention value " << (CCInfo >> 1)
<< " not understood.";
Error(StrBuf.str());
appendErrorInstruction(ReturnType);
if (ReturnType != Ice::IceType_void)
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.
......@@ -2695,30 +2715,67 @@ void FunctionParser::ProcessRecord() {
setNextLocalInstIndex(nullptr);
return;
}
// Create the call instruction.
Ice::Variable *Dest = (ReturnType == Ice::IceType_void)
? nullptr
: getNextInstVar(ReturnType);
Ice::InstCall *Inst = nullptr;
std::unique_ptr<Ice::InstCall> Inst;
if (IntrinsicInfo) {
Inst = Ice::InstIntrinsicCall::create(Func.get(), NumParams, Dest, Callee,
IntrinsicInfo->Info);
Inst.reset(Ice::InstIntrinsicCall::create(Func.get(), NumParams, Dest,
Callee, IntrinsicInfo->Info));
} else {
Inst = Ice::InstCall::create(Func.get(), NumParams, Dest, Callee,
IsTailCall);
Inst.reset(Ice::InstCall::create(Func.get(), NumParams, Dest, Callee,
IsTailCall));
}
// Add parameters.
for (Ice::SizeT ParamIndex = 0; ParamIndex < NumParams; ++ParamIndex) {
Inst->addArg(
getRelativeOperand(Values[ParamsStartIndex + ParamIndex], BaseIndex));
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, ArgIndex)) {
switch (IntrinsicInfo->validateCall(Inst.get(), ArgIndex)) {
case Ice::Intrinsics::IsValidCall:
break;
case Ice::Intrinsics::BadReturnType: {
......@@ -2750,7 +2807,7 @@ void FunctionParser::ProcessRecord() {
}
}
CurrentNode->appendInst(Inst);
CurrentNode->appendInst(Inst.release());
return;
}
case naclbitc::FUNC_CODE_INST_FORWARDTYPEREF: {
......
tests_lit/parse_errs/call-fcn-bad-param-type.ll 0 → 100644
; Test that even if a call parameter matches its declaration, it must still
; be a legal call parameter type (unless declaration is intrinsic).
; REQUIRES: no_minimal_build
; RUN: %p2i --expect-fail -i %s --insts | FileCheck %s
declare void @f(i8);
define void @Test() {
entry:
call void @f(i8 1)
; CHECK: Call argument 1 matches declaration but has invalid type: i8
ret void
}
tests_lit/parse_errs/call-fcn-bad-return-type.ll 0 → 100644
; Test that even if a call return type matches its declaration, it must still be
; a legal call return type (unless declaration is intrinsic).
; REQUIRES: no_minimal_build
; RUN: %p2i --expect-fail -i %s --insts | FileCheck %s
declare i1 @f();
define void @Test() {
entry:
%v = call i1 @f()
; CHECK: Return type of called function is invalid: i1
ret void
}
tests_lit/parse_errs/call-indirect-i8.ll 0 → 100644
; Tests that we don't allow illegal sized parameters on indirect calls.
; REQUIRES: no_minimal_build
; RUN: %p2i --expect-fail -i %s --insts | FileCheck %s
define void @CallIndirectI32(i32 %f_addr) {
entry:
%f = inttoptr i32 %f_addr to i32(i8)*
%r = call i32 %f(i8 1)
; CHECK: Call argument 1 has invalid type: i8
ret void
}
tests_lit/reader_tests/call-indirect.ll
; Test parsing indirect calls in Subzero.
; RUN: %p2i -i %s --insts | FileCheck %s
; RUN: %if --need=allow_disable_ir_gen --command \
; RUN: %p2i -i %s --args -notranslate -timing -no-ir-gen \
; RUN: | %if --need=allow_disable_ir_gen --command \
; RUN: FileCheck --check-prefix=NOIR %s
; RUN: %p2i -i %s --args -notranslate -timing | FileCheck --check-prefix=NOIR %s
define internal void @CallIndirectVoid(i32 %f_addr) {
entry:
......@@ -21,14 +18,14 @@ entry:
define internal i32 @CallIndirectI32(i32 %f_addr) {
entry:
%f = inttoptr i32 %f_addr to i32(i8, i1)*
%r = call i32 %f(i8 1, i1 false)
%f = inttoptr i32 %f_addr to i32(i64, i32)*
%r = call i32 %f(i64 1, i32 %f_addr)
ret i32 %r
}
; CHECK-NEXT: define internal i32 @CallIndirectI32(i32 %f_addr) {
; CHECK-NEXT: entry:
; CHECK-NEXT: %r = call i32 %f_addr(i8 1, i1 false)
; CHECK-NEXT: %r = call i32 %f_addr(i64 1, i32 %f_addr)
; CHECK-NEXT: ret i32 %r
; CHECK-NEXT: }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment