mirror of
				https://github.com/ethereum/solidity
				synced 2023-10-03 13:03:40 +00:00 
			
		
		
		
	Merge pull request #9150 from ethereum/issue-8670
CodeGen: Avoid double cleanup when copying to memory
This commit is contained in:
		
						commit
						1c93245704
					
				| @ -12,6 +12,7 @@ Bugfixes: | ||||
|  * SMTChecker: Fix internal error when using bitwise operators on fixed bytes type. | ||||
|  * Type Checker: Fix overload resolution in combination with ``{value: ...}``. | ||||
|  * Type Checker: Fix internal compiler error related to oversized types. | ||||
|  * Code Generator: Avoid double cleanup when copying to memory. | ||||
| 
 | ||||
| Compiler Features: | ||||
|  * Build System: Update internal dependency of jsoncpp to 1.9.3. | ||||
|  | ||||
| @ -180,7 +180,7 @@ void CompilerUtils::storeInMemory(unsigned _offset) | ||||
| 		m_context << u256(_offset) << Instruction::MSTORE; | ||||
| } | ||||
| 
 | ||||
| void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBoundaries) | ||||
| void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBoundaries, bool _cleanup) | ||||
| { | ||||
| 	// process special types (Reference, StringLiteral, Function)
 | ||||
| 	if (auto ref = dynamic_cast<ReferenceType const*>(&_type)) | ||||
| @ -189,7 +189,7 @@ void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBound | ||||
| 			ref->location() == DataLocation::Memory, | ||||
| 			"Only in-memory reference type can be stored." | ||||
| 		); | ||||
| 		storeInMemoryDynamic(*TypeProvider::uint256(), _padToWordBoundaries); | ||||
| 		storeInMemoryDynamic(*TypeProvider::uint256(), _padToWordBoundaries, _cleanup); | ||||
| 	} | ||||
| 	else if (auto str = dynamic_cast<StringLiteralType const*>(&_type)) | ||||
| 	{ | ||||
| @ -212,7 +212,7 @@ void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBound | ||||
| 	} | ||||
| 	else if (_type.isValueType()) | ||||
| 	{ | ||||
| 		unsigned numBytes = prepareMemoryStore(_type, _padToWordBoundaries); | ||||
| 		unsigned numBytes = prepareMemoryStore(_type, _padToWordBoundaries, _cleanup); | ||||
| 		m_context << Instruction::DUP2 << Instruction::MSTORE; | ||||
| 		m_context << u256(numBytes) << Instruction::ADD; | ||||
| 	} | ||||
| @ -463,6 +463,7 @@ void CompilerUtils::encodeToMemory( | ||||
| 		} | ||||
| 		else | ||||
| 		{ | ||||
| 			bool needCleanup = true; | ||||
| 			copyToStackTop(argSize - stackPos + dynPointers + 2, _givenTypes[i]->sizeOnStack()); | ||||
| 			solAssert(!!targetType, "Externalable type expected."); | ||||
| 			TypePointer type = targetType; | ||||
| @ -481,7 +482,11 @@ void CompilerUtils::encodeToMemory( | ||||
| 			) | ||||
| 				type = _givenTypes[i]; // delay conversion
 | ||||
| 			else | ||||
| 			{ | ||||
| 				convertType(*_givenTypes[i], *targetType, true); | ||||
| 				needCleanup = false; | ||||
| 			} | ||||
| 
 | ||||
| 			if (auto arrayType = dynamic_cast<ArrayType const*>(type)) | ||||
| 				ArrayUtils(m_context).copyArrayToMemory(*arrayType, _padToWordBoundaries); | ||||
| 			else if (auto arraySliceType = dynamic_cast<ArraySliceType const*>(type)) | ||||
| @ -495,7 +500,7 @@ void CompilerUtils::encodeToMemory( | ||||
| 				ArrayUtils(m_context).copyArrayToMemory(arraySliceType->arrayType(), _padToWordBoundaries); | ||||
| 			} | ||||
| 			else | ||||
| 				storeInMemoryDynamic(*type, _padToWordBoundaries); | ||||
| 				storeInMemoryDynamic(*type, _padToWordBoundaries, needCleanup); | ||||
| 		} | ||||
| 		stackPos += _givenTypes[i]->sizeOnStack(); | ||||
| 	} | ||||
| @ -1499,7 +1504,7 @@ void CompilerUtils::rightShiftNumberOnStack(unsigned _bits) | ||||
| 		m_context << (u256(1) << _bits) << Instruction::SWAP1 << Instruction::DIV; | ||||
| } | ||||
| 
 | ||||
| unsigned CompilerUtils::prepareMemoryStore(Type const& _type, bool _padToWords) | ||||
| unsigned CompilerUtils::prepareMemoryStore(Type const& _type, bool _padToWords, bool _cleanup) | ||||
| { | ||||
| 	solAssert( | ||||
| 		_type.sizeOnStack() == 1, | ||||
| @ -1522,7 +1527,9 @@ unsigned CompilerUtils::prepareMemoryStore(Type const& _type, bool _padToWords) | ||||
| 
 | ||||
| 	bool leftAligned = _type.category() == Type::Category::FixedBytes; | ||||
| 
 | ||||
| 	convertType(_type, _type, true); | ||||
| 	if (_cleanup) | ||||
| 		convertType(_type, _type, true); | ||||
| 
 | ||||
| 	if (numBytes != 32 && !leftAligned && !_padToWords) | ||||
| 		// shift the value accordingly before storing
 | ||||
| 		leftShiftNumberOnStack((32 - numBytes) * 8); | ||||
|  | ||||
| @ -107,10 +107,11 @@ public: | ||||
| 	/// and also updates that. For reference types, only copies the data pointer. Fails for
 | ||||
| 	/// non-memory-references.
 | ||||
| 	/// @param _padToWords if true, adds zeros to pad to multiple of 32 bytes. Array elements
 | ||||
| 	/// @param _cleanup if true, adds code to cleanup the value before storing it.
 | ||||
| 	/// are always padded (except for byte arrays), regardless of this parameter.
 | ||||
| 	/// Stack pre: memory_offset value...
 | ||||
| 	/// Stack post: (memory_offset+length)
 | ||||
| 	void storeInMemoryDynamic(Type const& _type, bool _padToWords = true); | ||||
| 	void storeInMemoryDynamic(Type const& _type, bool _padToWords = true, bool _cleanup = true); | ||||
| 
 | ||||
| 	/// Creates code that unpacks the arguments according to their types specified by a vector of TypePointers.
 | ||||
| 	/// From memory if @a _fromMemory is true, otherwise from call data.
 | ||||
| @ -309,7 +310,8 @@ private: | ||||
| 	void cleanHigherOrderBits(IntegerType const& _typeOnStack); | ||||
| 
 | ||||
| 	/// Prepares the given type for storing in memory by shifting it if necessary.
 | ||||
| 	unsigned prepareMemoryStore(Type const& _type, bool _padToWords); | ||||
| 	/// @param _cleanup if true, also cleanup the value when preparing to store it in memory
 | ||||
| 	unsigned prepareMemoryStore(Type const& _type, bool _padToWords, bool _cleanup = true); | ||||
| 	/// Loads type from memory assuming memory offset is on stack top.
 | ||||
| 	unsigned loadFromMemoryHelper(Type const& _type, bool _fromCalldata, bool _padToWords); | ||||
| 
 | ||||
|  | ||||
| @ -708,6 +708,19 @@ BOOST_AUTO_TEST_CASE(shift_optimizer_bug) | ||||
| 	compareVersions("g(uint256)", u256(-1)); | ||||
| } | ||||
| 
 | ||||
| BOOST_AUTO_TEST_CASE(avoid_double_cleanup) | ||||
| { | ||||
| 	char const* sourceCode = R"( | ||||
| 		contract C { | ||||
| 			receive() external payable { | ||||
| 				abi.encodePacked(uint200(0)); | ||||
| 			} | ||||
| 		} | ||||
| 	)"; | ||||
| 	compileBothVersions(sourceCode, 0, "C", 50); | ||||
| 	// Check that there is no double AND instruction in the resulting code
 | ||||
| 	BOOST_CHECK_EQUAL(numInstructions(m_nonOptimizedBytecode, Instruction::AND), 1); | ||||
| } | ||||
| 
 | ||||
| BOOST_AUTO_TEST_SUITE_END() | ||||
| 
 | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user