From 493d4ab9edba2be18c916d80aaa100a0e51e1f51 Mon Sep 17 00:00:00 2001 From: Karl Schimpf Date: Wed, 28 Aug 2013 14:34:19 -0700 Subject: Handle pointer conversions for call instructions. This also should complete the changes associated with removing pointer cast instructions from the PNaCl bitcode file. BUG= https://code.google.com/p/nativeclient/issues/detail?id=3544 R=dschuff@chromium.org, jvoung@chromium.org Review URL: https://codereview.chromium.org/23482002 --- lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp | 99 +++++++++++++------------ lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp | 63 ++++++++++------ lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp | 19 ++--- 3 files changed, 102 insertions(+), 79 deletions(-) (limited to 'lib') diff --git a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp index fbe1fc0165..028e750d92 100644 --- a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp +++ b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp @@ -1315,16 +1315,15 @@ Value *NaClBitcodeReader::ConvertOpToScalar(Value *Op, unsigned BBIndex, Value *NaClBitcodeReader::ConvertOpToType(Value *Op, Type *T, unsigned BBIndex) { - // Note: Currently only knows how to add inttoptr and bitcast type - // conversions for non-phi nodes, since these are the only elided - // instructions in the bitcode writer. - // - // TODO(kschimpf): Generalize this as we expand elided conversions. Type *OpTy = Op->getType(); if (OpTy == T) return Op; if (OpTy->isPointerTy()) { - return CreateCast(BBIndex, Instruction::BitCast, T, Op); + if (T == IntPtrType) { + return ConvertOpToScalar(Op, BBIndex); + } else { + return CreateCast(BBIndex, Instruction::BitCast, T, Op); + } } else if (OpTy == IntPtrType) { return CreateCast(BBIndex, Instruction::IntToPtr, T, Op); } @@ -1687,7 +1686,7 @@ bool NaClBitcodeReader::ParseFunctionBody(Function *F) { return Error("Invalid LOAD record"); switch (GetPNaClVersion()) { case 1: - I = new LoadInst(Op, "", Record[OpNum+1], (1 << Record[OpNum]) >> 1); + I = new LoadInst(Op, "", Record[OpNum+1], (1 << Record[OpNum]) >> 1); break; case 2: { // Add pointer cast to op. @@ -1696,8 +1695,8 @@ bool NaClBitcodeReader::ParseFunctionBody(Function *F) { return Error("Invalid type for load instruction"); Op = ConvertOpToType(Op, T->getPointerTo(), CurBBNo); if (Op == 0) return true; - I = new LoadInst(Op, "", false, (1 << Record[OpNum]) >> 1); - break; + I = new LoadInst(Op, "", false, (1 << Record[OpNum]) >> 1); + break; } } break; @@ -1712,28 +1711,29 @@ bool NaClBitcodeReader::ParseFunctionBody(Function *F) { return Error("Invalid STORE record"); switch (GetPNaClVersion()) { case 1: - if (OpNum+2 != Record.size()) - return Error("Invalid STORE record"); - I = new StoreInst(Val, Ptr, Record[OpNum+1], (1 << Record[OpNum]) >> 1); - break; + if (OpNum+2 != Record.size()) + return Error("Invalid STORE record"); + I = new StoreInst(Val, Ptr, Record[OpNum+1], (1 << Record[OpNum]) >> 1); + break; case 2: - if (OpNum+1 != Record.size()) - return Error("Invalid STORE record"); + if (OpNum+1 != Record.size()) + return Error("Invalid STORE record"); Val = ConvertOpToScalar(Val, CurBBNo); Ptr = ConvertOpToType(Ptr, Val->getType()->getPointerTo(), CurBBNo); - I = new StoreInst(Val, Ptr, false, (1 << Record[OpNum]) >> 1); - break; + I = new StoreInst(Val, Ptr, false, (1 << Record[OpNum]) >> 1); + break; } break; } - case naclbitc::FUNC_CODE_INST_CALL: { + case naclbitc::FUNC_CODE_INST_CALL: + case naclbitc::FUNC_CODE_INST_CALL_INDIRECT: { // CALL: [cc, fnid, arg0, arg1...] - if (Record.size() < 2) + // PNaCl version 2: CALL_INDIRECT: [cc, fnid, fnty, args...] + if ((Record.size() < 2) || + (BitCode == naclbitc::FUNC_CODE_INST_CALL_INDIRECT && + Record.size() < 3)) return Error("Invalid CALL record"); - // TODO(kschimpf): Fix handling of type conversion to arguments for PNaCl, - // to handle elided casts, once the bitcode writer knows how. - unsigned CCInfo = Record[0]; unsigned OpNum = 1; @@ -1741,35 +1741,42 @@ bool NaClBitcodeReader::ParseFunctionBody(Function *F) { if (popValue(Record, &OpNum, NextValueNo, &Callee)) return Error("Invalid CALL record"); - PointerType *OpTy = dyn_cast(Callee->getType()); + // Build function type for call. FunctionType *FTy = 0; - if (OpTy) FTy = dyn_cast(OpTy->getElementType()); - if (!FTy || Record.size() < FTy->getNumParams()+OpNum) - return Error("Invalid CALL record"); - - SmallVector Args; - // Read the fixed params. - for (unsigned i = 0, e = FTy->getNumParams(); i != e; ++i, ++OpNum) { - if (FTy->getParamType(i)->isLabelTy()) - Args.push_back(getBasicBlock(Record[OpNum])); - else - Args.push_back(getValue(Record, OpNum, NextValueNo)); - if (Args.back() == 0) return Error("Invalid CALL record"); - } - - // Read type/value pairs for varargs params. - if (!FTy->isVarArg()) { - if (OpNum != Record.size()) - return Error("Invalid CALL record"); + if (BitCode == naclbitc::FUNC_CODE_INST_CALL_INDIRECT) { + // Callee type has been elided, add back in. + Type *Type = getTypeByID(Record[2]); + ++OpNum; + if (FunctionType *FcnType = dyn_cast(Type)) { + FTy = FcnType; + Callee = ConvertOpToType(Callee, FcnType->getPointerTo(), CurBBNo); + } else { + return Error("Invalid type for CALL_INDIRECT record"); + } } else { - while (OpNum != Record.size()) { - Value *Op; - if (popValue(Record, &OpNum, NextValueNo, &Op)) - return Error("Invalid CALL record"); - Args.push_back(Op); + // Get type signature from callee. + if (PointerType *OpTy = dyn_cast(Callee->getType())) { + FTy = dyn_cast(OpTy->getElementType()); } + if (FTy == 0) + return Error("Invalid type for CALL record"); + } + + unsigned NumParams = Record.size() - OpNum; + if (NumParams != FTy->getNumParams()) + return Error("Invalid CALL record"); + + // Process call arguments. + SmallVector Args; + for (unsigned Index = 0; Index < NumParams; ++Index) { + Value *Arg; + if (popValue(Record, &OpNum, NextValueNo, &Arg)) + Error("Invalid argument in CALL record"); + Arg = ConvertOpToType(Arg, FTy->getParamType(Index), CurBBNo); + Args.push_back(Arg); } + // Construct call. I = CallInst::Create(Callee, Args); cast(I)->setCallingConv(GetDecodedCallingConv(CCInfo>>1)); cast(I)->setTailCall(CCInfo & 1); diff --git a/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp b/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp index 6624ad1d7c..2ac88c92a4 100644 --- a/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp +++ b/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp @@ -975,32 +975,47 @@ static bool WriteInstruction(const Instruction &I, unsigned InstID, } break; case Instruction::Call: { - const CallInst &CI = cast(I); - PointerType *PTy = cast(CI.getCalledValue()->getType()); - FunctionType *FTy = cast(PTy->getElementType()); - - Code = naclbitc::FUNC_CODE_INST_CALL; - - Vals.push_back((GetEncodedCallingConv(CI.getCallingConv()) << 1) - | unsigned(CI.isTailCall())); - pushValue(CI.getCalledValue(), InstID, Vals, VE, Stream); // Callee - - // Emit value #'s for the fixed parameters. - for (unsigned i = 0, e = FTy->getNumParams(); i != e; ++i) { - // Check for labels (can happen with asm labels). - if (FTy->getParamType(i)->isLabelTy()) - Vals.push_back(VE.getValueID(CI.getArgOperand(i))); - else - // fixed param. - pushValue(CI.getArgOperand(i), InstID, Vals, VE, Stream); + // CALL: [cc, fnid, args...] + // PNaCl version 2: CALL_INDIRECT: [cc, fnid, fnty, args...] + + const CallInst &Call = cast(I); + const Value* Callee = Call.getCalledValue(); + Vals.push_back((GetEncodedCallingConv(Call.getCallingConv()) << 1) + | unsigned(Call.isTailCall())); + + pushValue(Callee, InstID, Vals, VE, Stream); + + switch (PNaClVersion) { + case 1: + Code = naclbitc::FUNC_CODE_INST_CALL; + break; + case 2: + if (Callee == VE.ElideCasts(Callee)) { + // Since the call pointer has not been elided, we know that + // the call pointer has the type signature of the called + // function. This implies that the reader can use the type + // signature of the callee to figure out how to add casts to + // the arguments. + Code = naclbitc::FUNC_CODE_INST_CALL; + } else { + // If the cast was elided, a pointer conversion to a pointer + // was applied, meaning that this is an indirect call. For the + // reader, this implies that we can't use the type signature + // of the callee to resolve elided call arguments, since it is + // not known. Hence, we must send the type signature to the + // reader. + Code = naclbitc::FUNC_CODE_INST_CALL_INDIRECT; + PointerType *FcnPtrType = + cast(Callee->getType()); + FunctionType *FcnType = + cast(FcnPtrType->getElementType()); + Vals.push_back(VE.getTypeID(FcnType)); + } + break; } - // Emit type/value pairs for varargs params. - if (FTy->isVarArg()) { - for (unsigned i = FTy->getNumParams(), e = CI.getNumArgOperands(); - i != e; ++i) - // varargs - pushValue(CI.getArgOperand(i), InstID, Vals, VE, Stream); + for (unsigned I = 0, E = Call.getNumArgOperands(); I < E; ++I) { + pushValue(Call.getArgOperand(I), InstID, Vals, VE, Stream); } break; } diff --git a/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp b/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp index 060a6d63f4..8cfdf13240 100644 --- a/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp +++ b/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp @@ -438,9 +438,6 @@ static bool ExpectsNormalizedPtr(const Value *V, const Instruction *Arg) { const Instruction *I = dyn_cast(V); if (I == 0) return false; - // TODO(kschimpf) Expand this list to any operation that can handle - // normalized pointers. That is loads and stores, function calls, and - // instrinsic calls. switch (I->getOpcode()) { default: return false; @@ -448,6 +445,10 @@ static bool ExpectsNormalizedPtr(const Value *V, const Instruction *Arg) { return I->getOperand(0) == Arg; case Instruction::Store: return I->getOperand(1) == Arg; + case Instruction::Call: + // For function calls, the function operand is normalized, and for + // intrinsic calls, all pointer arguments are normalized. + return true; } } @@ -493,12 +494,13 @@ static bool ExpectsScalarValue(const Value *V, const Instruction *Arg) { const SelectInst *Op = dyn_cast(I); return Arg == Op->getTrueValue() || Arg == Op->getFalseValue(); } + case Instruction::Call: { + // All operands (except the first, which must be a function pointer), + // can be scalar values. + const CallInst *Call = cast(I); + return Call->getCalledValue() != Arg; + } } - // TODO(kschimpf): Need to think more about how to handle following - // instructions: - // case Instruction::IntToPtr: - // case Instruction::BitCast: - // case Instruction::Call: } } @@ -526,7 +528,6 @@ static inline bool IsInherentPtr(const Value *V) { // llvm/lib/Transforms/NaCl/ReplacePtrsWithInts.cpp. const Value *NaClValueEnumerator::ElideCasts(const Value *V) { if (PNaClVersion == 1) return V; - // TODO(kschimpf): Expand this out to cover all cases. if (const Instruction *I = dyn_cast(V)) { switch (I->getOpcode()) { default: -- cgit v1.2.3-18-g5258