From 273b0f634f7beb3cdf5db0e592221dc88bc33c78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Fri, 31 Oct 2014 08:26:41 +0100 Subject: [PATCH 1/4] Deprecate Memory::require(size) function. Risk of unsigned integer overflow. --- libevmjit/Compiler.cpp | 9 +++------ libevmjit/Memory.cpp | 2 +- libevmjit/Memory.h | 7 +++---- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/libevmjit/Compiler.cpp b/libevmjit/Compiler.cpp index 1e394eba00..0c955ab3b9 100644 --- a/libevmjit/Compiler.cpp +++ b/libevmjit/Compiler.cpp @@ -785,12 +785,9 @@ void Compiler::compileBasicBlock(BasicBlock& basicBlock, bytesConstRef bytecode, gasMeter.commitCostBlock(gas); - // Require memory for the max of in and out buffers - auto inSizeReq = m_builder.CreateAdd(inOff, inSize, "inSizeReq"); - auto outSizeReq = m_builder.CreateAdd(outOff, outSize, "outSizeReq"); - auto cmp = m_builder.CreateICmpUGT(inSizeReq, outSizeReq); - auto sizeReq = m_builder.CreateSelect(cmp, inSizeReq, outSizeReq, "sizeReq"); - memory.require(sizeReq); + // Require memory for in and out buffers + memory.require(outOff, outSize); // Out buffer first as we guess it will be after the in one + memory.require(inOff, inSize); auto receiveAddress = codeAddress; if (inst == Instruction::CALLCODE) diff --git a/libevmjit/Memory.cpp b/libevmjit/Memory.cpp index 84aa105fe2..8528bd9d23 100644 --- a/libevmjit/Memory.cpp +++ b/libevmjit/Memory.cpp @@ -171,7 +171,7 @@ void Memory::require(llvm::Value* _size) void Memory::require(llvm::Value* _offset, llvm::Value* _size) { - auto sizeRequired = m_builder.CreateAdd(_offset, _size, "sizeRequired"); + auto sizeRequired = m_builder.CreateNUWAdd(_offset, _size, "sizeRequired"); require(sizeRequired); } diff --git a/libevmjit/Memory.h b/libevmjit/Memory.h index f315b9295c..90c60c5fd3 100644 --- a/libevmjit/Memory.h +++ b/libevmjit/Memory.h @@ -24,9 +24,6 @@ public: void copyBytes(llvm::Value* _srcPtr, llvm::Value* _srcSize, llvm::Value* _srcIndex, llvm::Value* _destMemIdx, llvm::Value* _byteCount); - /// Requires this amount of memory. And counts gas fee for that memory. - void require(llvm::Value* _size); - /// Requires the amount of memory to for data defined by offset and size. And counts gas fee for that memory. void require(llvm::Value* _offset, llvm::Value* _size); @@ -36,7 +33,9 @@ private: llvm::Function* createFunc(bool _isStore, llvm::Type* _type, GasMeter& _gasMeter); llvm::Function* createRequireFunc(GasMeter& _gasMeter, RuntimeManager& _runtimeManager); -private: + /// Requires this amount of memory. And counts gas fee for that memory. + void require(llvm::Value* _size); + llvm::GlobalVariable* m_data; llvm::GlobalVariable* m_size; From 272c568bfcb42257bd9f7b6dd5b06cfaa37e0ca7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Fri, 31 Oct 2014 08:50:25 +0100 Subject: [PATCH 2/4] Remove Memory::require(size) interface [#81773288] --- libevmjit/Memory.cpp | 32 ++++++++++++++------------------ libevmjit/Memory.h | 3 --- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/libevmjit/Memory.cpp b/libevmjit/Memory.cpp index 8528bd9d23..309fb29640 100644 --- a/libevmjit/Memory.cpp +++ b/libevmjit/Memory.cpp @@ -28,8 +28,7 @@ Memory::Memory(RuntimeManager& _runtimeManager, GasMeter& _gasMeter): RuntimeHelper(_runtimeManager) { auto module = getModule(); - auto i64Ty = m_builder.getInt64Ty(); - llvm::Type* argTypes[] = {i64Ty, i64Ty}; + llvm::Type* argTypes[] = {Type::i256, Type::i256}; auto dumpTy = llvm::FunctionType::get(m_builder.getVoidTy(), llvm::ArrayRef(argTypes), false); m_memDump = llvm::Function::Create(dumpTy, llvm::GlobalValue::LinkageTypes::ExternalLinkage, "evmccrt_memory_dump", module); @@ -54,19 +53,23 @@ Memory::Memory(RuntimeManager& _runtimeManager, GasMeter& _gasMeter): llvm::Function* Memory::createRequireFunc(GasMeter& _gasMeter, RuntimeManager& _runtimeManager) { - auto func = llvm::Function::Create(llvm::FunctionType::get(Type::Void, Type::i256, false), llvm::Function::PrivateLinkage, "mem.require", getModule()); + llvm::Type* argTypes[] = {Type::i256, Type::i256}; + auto func = llvm::Function::Create(llvm::FunctionType::get(Type::Void, argTypes, false), llvm::Function::PrivateLinkage, "mem.require", getModule()); + auto offset = func->arg_begin(); + offset->setName("offset"); + auto size = offset->getNextNode(); + size->setName("size"); - auto checkBB = llvm::BasicBlock::Create(func->getContext(), "check", func); - auto resizeBB = llvm::BasicBlock::Create(func->getContext(), "resize", func); - auto returnBB = llvm::BasicBlock::Create(func->getContext(), "return", func); + auto checkBB = llvm::BasicBlock::Create(func->getContext(), "Check", func); + auto resizeBB = llvm::BasicBlock::Create(func->getContext(), "Resize", func); + auto returnBB = llvm::BasicBlock::Create(func->getContext(), "Return", func); InsertPointGuard guard(m_builder); // Restores insert point at function exit // BB "check" m_builder.SetInsertPoint(checkBB); - llvm::Value* sizeRequired = func->arg_begin(); - sizeRequired->setName("sizeRequired"); - auto size = m_builder.CreateLoad(m_size, "size"); + auto sizeRequired = m_builder.CreateNUWAdd(offset, size, "sizeRequired"); + auto currSize = m_builder.CreateLoad(m_size, "currSize"); auto resizeNeeded = m_builder.CreateICmpULE(size, sizeRequired, "resizeNeeded"); m_builder.CreateCondBr(resizeNeeded, resizeBB, returnBB); // OPT branch weights? @@ -164,15 +167,9 @@ llvm::Value* Memory::getSize() return m_builder.CreateLoad(m_size); } -void Memory::require(llvm::Value* _size) -{ - m_builder.CreateCall(m_require, _size); -} - void Memory::require(llvm::Value* _offset, llvm::Value* _size) { - auto sizeRequired = m_builder.CreateNUWAdd(_offset, _size, "sizeRequired"); - require(sizeRequired); + m_builder.CreateCall2(m_require, _offset, _size); } void Memory::copyBytes(llvm::Value* _srcPtr, llvm::Value* _srcSize, llvm::Value* _srcIdx, @@ -180,8 +177,7 @@ void Memory::copyBytes(llvm::Value* _srcPtr, llvm::Value* _srcSize, llvm::Value* { auto zero256 = llvm::ConstantInt::get(Type::i256, 0); - auto reqMemSize = m_builder.CreateAdd(_destMemIdx, _reqBytes, "req_mem_size"); - require(reqMemSize); + require(_destMemIdx, _reqBytes); auto srcPtr = m_builder.CreateGEP(_srcPtr, _srcIdx, "src_idx"); diff --git a/libevmjit/Memory.h b/libevmjit/Memory.h index 90c60c5fd3..37bfb15f50 100644 --- a/libevmjit/Memory.h +++ b/libevmjit/Memory.h @@ -33,9 +33,6 @@ private: llvm::Function* createFunc(bool _isStore, llvm::Type* _type, GasMeter& _gasMeter); llvm::Function* createRequireFunc(GasMeter& _gasMeter, RuntimeManager& _runtimeManager); - /// Requires this amount of memory. And counts gas fee for that memory. - void require(llvm::Value* _size); - llvm::GlobalVariable* m_data; llvm::GlobalVariable* m_size; From 350b004e7959c9b7193e1bb8b7a9059b096c3cbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Fri, 31 Oct 2014 11:09:53 +0100 Subject: [PATCH 3/4] Change Constant::get to support negative values --- libevmjit/Compiler.cpp | 2 +- libevmjit/Type.cpp | 4 ++-- libevmjit/Type.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libevmjit/Compiler.cpp b/libevmjit/Compiler.cpp index 0c955ab3b9..8a38e257ff 100644 --- a/libevmjit/Compiler.cpp +++ b/libevmjit/Compiler.cpp @@ -366,7 +366,7 @@ void Compiler::compileBasicBlock(BasicBlock& basicBlock, bytesConstRef bytecode, case Instruction::BNOT: { auto value = stack.pop(); - auto ret = m_builder.CreateXor(value, llvm::APInt(256, -1, true), "bnot"); + auto ret = m_builder.CreateXor(value, Constant::get(-1), "bnot"); stack.push(ret); break; } diff --git a/libevmjit/Type.cpp b/libevmjit/Type.cpp index ec11acef5b..37757dfdf3 100644 --- a/libevmjit/Type.cpp +++ b/libevmjit/Type.cpp @@ -36,9 +36,9 @@ void Type::init(llvm::LLVMContext& _context) RuntimePtr = RuntimeData::getType()->getPointerTo(); } -llvm::ConstantInt* Constant::get(uint64_t _n) +llvm::ConstantInt* Constant::get(int64_t _n) { - return llvm::ConstantInt::get(Type::i256, _n); + return llvm::ConstantInt::getSigned(Type::i256, _n); } llvm::ConstantInt* Constant::get(u256 _n) diff --git a/libevmjit/Type.h b/libevmjit/Type.h index c80e467777..4658f94bb7 100644 --- a/libevmjit/Type.h +++ b/libevmjit/Type.h @@ -51,7 +51,7 @@ enum class ReturnCode struct Constant { /// Returns word-size constant - static llvm::ConstantInt* get(uint64_t _n); + static llvm::ConstantInt* get(int64_t _n); static llvm::ConstantInt* get(u256 _n); static llvm::ConstantInt* get(ReturnCode _returnCode); From 72398d2c267bf2efb8fc959b6b608a6f1aaebc85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Fri, 31 Oct 2014 11:14:25 +0100 Subject: [PATCH 4/4] Handle unsigned integer overflow in Memory::require() [Delivers #81773288] --- libevmjit/Memory.cpp | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/libevmjit/Memory.cpp b/libevmjit/Memory.cpp index 309fb29640..322c53ee38 100644 --- a/libevmjit/Memory.cpp +++ b/libevmjit/Memory.cpp @@ -8,6 +8,7 @@ #include #include +#include #include @@ -68,17 +69,26 @@ llvm::Function* Memory::createRequireFunc(GasMeter& _gasMeter, RuntimeManager& _ // BB "check" m_builder.SetInsertPoint(checkBB); - auto sizeRequired = m_builder.CreateNUWAdd(offset, size, "sizeRequired"); + auto uaddWO = llvm::Intrinsic::getDeclaration(getModule(), llvm::Intrinsic::uadd_with_overflow, Type::i256); + auto uaddRes = m_builder.CreateCall2(uaddWO, offset, size, "res"); + auto sizeRequired = m_builder.CreateExtractValue(uaddRes, 0, "sizeReq"); + auto overflow1 = m_builder.CreateExtractValue(uaddRes, 1, "overflow1"); auto currSize = m_builder.CreateLoad(m_size, "currSize"); - auto resizeNeeded = m_builder.CreateICmpULE(size, sizeRequired, "resizeNeeded"); + auto tooSmall = m_builder.CreateICmpULE(size, sizeRequired, "tooSmall"); + auto resizeNeeded = m_builder.CreateOr(tooSmall, overflow1, "resizeNeeded"); m_builder.CreateCondBr(resizeNeeded, resizeBB, returnBB); // OPT branch weights? // BB "resize" m_builder.SetInsertPoint(resizeBB); // Check gas first - auto wordsRequired = m_builder.CreateUDiv(m_builder.CreateAdd(sizeRequired, Constant::get(31)), Constant::get(32), "wordsRequired"); - sizeRequired = m_builder.CreateMul(wordsRequired, Constant::get(32), "roundedSizeRequired"); - auto words = m_builder.CreateUDiv(size, Constant::get(32), "words"); // size is always 32*k + uaddRes = m_builder.CreateCall2(uaddWO, sizeRequired, Constant::get(31), "res"); + auto wordsRequired = m_builder.CreateExtractValue(uaddRes, 0); + auto overflow2 = m_builder.CreateExtractValue(uaddRes, 1, "overflow2"); + auto overflow = m_builder.CreateOr(overflow1, overflow2, "overflow"); + wordsRequired = m_builder.CreateSelect(overflow, Constant::get(-1), wordsRequired); + wordsRequired = m_builder.CreateUDiv(wordsRequired, Constant::get(32), "wordsReq"); + sizeRequired = m_builder.CreateMul(wordsRequired, Constant::get(32), "roundedSizeReq"); + auto words = m_builder.CreateUDiv(currSize, Constant::get(32), "words"); // size is always 32*k auto newWords = m_builder.CreateSub(wordsRequired, words, "addtionalWords"); _gasMeter.checkMemory(newWords, m_builder); // Resize