Improved array copy routine for value type arrays from memory or calldata to storage.

Changelog.md

@@ -5,6 +5,8 @@ Language Features:
 
 Compiler Features:
  * TypeChecker: Support using library constants in initializers of other constants.
+ * Yul IR Code Generation: Improved copy routines for arrays with packed storage layout.
+
 
 Bugfixes:
  * Commandline Interface: Disallow the following options outside of the compiler mode: ``--via-ir``,``--metadata-literal``, ``--metadata-hash``, ``--model-checker-show-unproved``, ``--model-checker-div-mod-no-slacks``, ``--model-checker-engine``, ``--model-checker-invariants``, ``--model-checker-solvers``, ``--model-checker-timeout``, ``--model-checker-contracts``, ``--model-checker-targets``.
@@ -30,6 +32,7 @@ Compiler Features:
 Bugfixes:
  * ABI Encoder: When encoding an empty string coming from storage do not add a superfluous empty slot for data.
  * Common Subexpression Eliminator: Process assembly items in chunks with maximum size of 2000. It helps to avoid extremely time-consuming searches during code optimization.
+ * Yul IR Code Generation: More robust cleanup in corner cases during memory to storage copies.
  * Yul Optimizer: Do not remove ``returndatacopy`` in cases in which it might perform out-of-bounds reads that unconditionally revert as out-of-gas. Previously, any ``returndatacopy`` that wrote to memory that was never read from was removed without accounting for the out-of-bounds condition.
 
 

libsolidity/codegen/YulUtilFunctions.cpp

@@ -1797,8 +1797,11 @@ string YulUtilFunctions::copyArrayToStorageFunction(ArrayType const& _fromType,
 
 	if (_fromType.isByteArrayOrString())
 		return copyByteArrayToStorageFunction(_fromType, _toType);
-	if (_fromType.dataStoredIn(DataLocation::Storage) && _toType.baseType()->isValueType())
-		return copyValueArrayStorageToStorageFunction(_fromType, _toType);
+	if (_toType.baseType()->isValueType())
+		return copyValueArrayToStorageFunction(_fromType, _toType);
+
+	solAssert(_toType.storageStride() == 32);
+	solAssert(!_fromType.baseType()->isValueType());
 
 	string functionName = "copy_array_to_storage_from_" + _fromType.identifier() + "_to_" + _toType.identifier();
 	return m_functionCollector.createFunction(functionName, [&](){
@@ -1812,7 +1815,6 @@ string YulUtilFunctions::copyArrayToStorageFunction(ArrayType const& _fromType,
 				let srcPtr := <srcDataLocation>(value)
 
 				let elementSlot := <dstDataLocation>(slot)
-				let elementOffset := 0
 
 				for { let i := 0 } lt(i, length) {i := add(i, 1)} {
 					<?fromCalldata>
@@ -1822,10 +1824,6 @@ string YulUtilFunctions::copyArrayToStorageFunction(ArrayType const& _fromType,
 						<!dynamicallyEncodedBase>
 							srcPtr
 						</dynamicallyEncodedBase>
-
-						<?isValueType>
-							<elementValues> := <readFromCalldataOrMemory>(<elementValues>)
-						</isValueType>
 					</fromCalldata>
 
 					<?fromMemory>
@@ -1836,19 +1834,11 @@ string YulUtilFunctions::copyArrayToStorageFunction(ArrayType const& _fromType,
 						let <elementValues> := srcPtr
 					</fromStorage>
 
-					<updateStorageValue>(elementSlot, elementOffset, <elementValues>)
+					<updateStorageValue>(elementSlot, <elementValues>)
 
 					srcPtr := add(srcPtr, <srcStride>)
 
-					<?multipleItemsPerSlot>
-						elementOffset := add(elementOffset, <storageStride>)
-						if gt(elementOffset, sub(32, <storageStride>)) {
-							elementOffset := 0
-							elementSlot := add(elementSlot, 1)
-						}
-					<!multipleItemsPerSlot>
 					elementSlot := add(elementSlot, <storageSize>)
-					</multipleItemsPerSlot>
 				}
 			}
 		)");
@@ -1870,7 +1860,6 @@ string YulUtilFunctions::copyArrayToStorageFunction(ArrayType const& _fromType,
 		}
 		templ("resizeArray", resizeArrayFunction(_toType));
 		templ("arrayLength",arrayLengthFunction(_fromType));
-		templ("isValueType", _fromType.baseType()->isValueType());
 		templ("dstDataLocation", arrayDataAreaFunction(_toType));
 		if (fromMemory || (fromCalldata && _fromType.baseType()->isValueType()))
 			templ("readFromCalldataOrMemory", readFromMemoryOrCalldata(*_fromType.baseType(), fromCalldata));
@@ -1879,7 +1868,7 @@ string YulUtilFunctions::copyArrayToStorageFunction(ArrayType const& _fromType,
 			0,
 			_fromType.baseType()->stackItems().size()
 		));
-		templ("updateStorageValue", updateStorageValueFunction(*_fromType.baseType(), *_toType.baseType()));
+		templ("updateStorageValue", updateStorageValueFunction(*_fromType.baseType(), *_toType.baseType(), 0));
 		templ("srcStride",
 			fromCalldata ?
 			to_string(_fromType.calldataStride()) :
@@ -1887,8 +1876,6 @@ string YulUtilFunctions::copyArrayToStorageFunction(ArrayType const& _fromType,
 				to_string(_fromType.memoryStride()) :
 				formatNumber(_fromType.baseType()->storageSize())
 		);
-		templ("multipleItemsPerSlot", _toType.storageStride() <= 16);
-		templ("storageStride", to_string(_toType.storageStride()));
 		templ("storageSize", _toType.baseType()->storageSize().str());
 
 		return templ.render();
@@ -1973,8 +1960,7 @@ string YulUtilFunctions::copyByteArrayToStorageFunction(ArrayType const& _fromTy
 	});
 }
 
-
-string YulUtilFunctions::copyValueArrayStorageToStorageFunction(ArrayType const& _fromType, ArrayType const& _toType)
+string YulUtilFunctions::copyValueArrayToStorageFunction(ArrayType const& _fromType, ArrayType const& _toType)
 {
 	solAssert(_fromType.baseType()->isValueType(), "");
 	solAssert(_toType.baseType()->isValueType(), "");
@@ -1982,7 +1968,6 @@ string YulUtilFunctions::copyValueArrayStorageToStorageFunction(ArrayType const&
 
 	solAssert(!_fromType.isByteArrayOrString(), "");
 	solAssert(!_toType.isByteArrayOrString(), "");
-	solAssert(_fromType.dataStoredIn(DataLocation::Storage), "");
 	solAssert(_toType.dataStoredIn(DataLocation::Storage), "");
 
 	solAssert(_fromType.storageStride() <= _toType.storageStride(), "");
@@ -1991,32 +1976,41 @@ string YulUtilFunctions::copyValueArrayStorageToStorageFunction(ArrayType const&
 	string functionName = "copy_array_to_storage_from_" + _fromType.identifier() + "_to_" + _toType.identifier();
 	return m_functionCollector.createFunction(functionName, [&](){
 		Whiskers templ(R"(
-			function <functionName>(dst, src) {
+			function <functionName>(dst, src<?isFromDynamicCalldata>, len</isFromDynamicCalldata>) {
+				<?isFromStorage>
 				if eq(dst, src) { leave }
-				let length := <arrayLength>(src)
+				</isFromStorage>
+				let length := <arrayLength>(src<?isFromDynamicCalldata>, len</isFromDynamicCalldata>)
 				// Make sure array length is sane
 				if gt(length, 0xffffffffffffffff) { <panic>() }
 				<resizeArray>(dst, length)
 
-				let srcSlot := <srcDataLocation>(src)
+				let srcPtr := <srcDataLocation>(src)
 				let dstSlot := <dstDataLocation>(dst)
 
 				let fullSlots := div(length, <itemsPerSlot>)
 
-				let srcSlotValue := sload(srcSlot)
+				<?isFromStorage>
+				let srcSlotValue := sload(srcPtr)
 				let srcItemIndexInSlot := 0
+				</isFromStorage>
+
 				for { let i := 0 } lt(i, fullSlots) { i := add(i, 1) } {
 					let dstSlotValue := 0
-					<?sameType>
+					<?sameTypeFromStorage>
 						dstSlotValue := <maskFull>(srcSlotValue)
-						<updateSrcSlotValue>
-					<!sameType>
+						<updateSrcPtr>
+					<!sameTypeFromStorage>
 						<?multipleItemsPerSlotDst>for { let j := 0 } lt(j, <itemsPerSlot>) { j := add(j, 1) } </multipleItemsPerSlotDst>
 						{
-							let itemValue := <convert>(
+							<?isFromStorage>
+							let <elementValues> := <convert>(
 								<extractFromSlot>(srcSlotValue, mul(<srcStride>, srcItemIndexInSlot))
 							)
-							itemValue := <prepareStore>(itemValue)
+							<!isFromStorage>
+							let <elementValues> := <readFromCalldataOrMemory>(srcPtr)
+							</isFromStorage>
+							let itemValue := <prepareStore>(<elementValues>)
 							dstSlotValue :=
 							<?multipleItemsPerSlotDst>
 								<updateByteSlice>(dstSlotValue, mul(<dstStride>, j), itemValue)
@@ -2024,9 +2018,9 @@ string YulUtilFunctions::copyValueArrayStorageToStorageFunction(ArrayType const&
 								itemValue
 							</multipleItemsPerSlotDst>
 
-							<updateSrcSlotValue>
+							<updateSrcPtr>
 						}
-					</sameType>
+					</sameTypeFromStorage>
 
 					sstore(add(dstSlot, i), dstSlotValue)
 				}
@@ -2035,20 +2029,24 @@ string YulUtilFunctions::copyValueArrayStorageToStorageFunction(ArrayType const&
 					let spill := sub(length, mul(fullSlots, <itemsPerSlot>))
 					if gt(spill, 0) {
 						let dstSlotValue := 0
-						<?sameType>
+						<?sameTypeFromStorage>
 							dstSlotValue := <maskBytes>(srcSlotValue, mul(spill, <srcStride>))
-							<updateSrcSlotValue>
-						<!sameType>
+							<updateSrcPtr>
+						<!sameTypeFromStorage>
 							for { let j := 0 } lt(j, spill) { j := add(j, 1) } {
-								let itemValue := <convert>(
+								<?isFromStorage>
+								let <elementValues> := <convert>(
 									<extractFromSlot>(srcSlotValue, mul(<srcStride>, srcItemIndexInSlot))
 								)
-								itemValue := <prepareStore>(itemValue)
+								<!isFromStorage>
+								let <elementValues> := <readFromCalldataOrMemory>(srcPtr)
+								</isFromStorage>
+								let itemValue := <prepareStore>(<elementValues>)
 								dstSlotValue := <updateByteSlice>(dstSlotValue, mul(<dstStride>, j), itemValue)
 
-								<updateSrcSlotValue>
+								<updateSrcPtr>
 							}
-						</sameType>
+						</sameTypeFromStorage>
 						sstore(add(dstSlot, fullSlots), dstSlotValue)
 					}
 				</multipleItemsPerSlotDst>
@@ -2056,26 +2054,37 @@ string YulUtilFunctions::copyValueArrayStorageToStorageFunction(ArrayType const&
 		)");
 		if (_fromType.dataStoredIn(DataLocation::Storage))
 			solAssert(!_fromType.isValueType(), "");
+
+		bool fromCalldata = _fromType.dataStoredIn(DataLocation::CallData);
+		bool fromStorage = _fromType.dataStoredIn(DataLocation::Storage);
 		templ("functionName", functionName);
 		templ("resizeArray", resizeArrayFunction(_toType));
 		templ("arrayLength", arrayLengthFunction(_fromType));
 		templ("panic", panicFunction(PanicCode::ResourceError));
+		templ("isFromDynamicCalldata", _fromType.isDynamicallySized() && fromCalldata);
+		templ("isFromStorage", fromStorage);
+		templ("readFromCalldataOrMemory", readFromMemoryOrCalldata(*_fromType.baseType(), fromCalldata));
 		templ("srcDataLocation", arrayDataAreaFunction(_fromType));
 		templ("dstDataLocation", arrayDataAreaFunction(_toType));
 		templ("srcStride", to_string(_fromType.storageStride()));
+		templ("elementValues", suffixedVariableNameList(
+			"elementValue_",
+			0,
+			_fromType.baseType()->stackItems().size()
+		));
 		unsigned itemsPerSlot = 32 / _toType.storageStride();
 		templ("itemsPerSlot", to_string(itemsPerSlot));
 		templ("multipleItemsPerSlotDst", itemsPerSlot > 1);
-		bool sameType = *_fromType.baseType() == *_toType.baseType();
+		bool sameTypeFromStorage = fromStorage && (*_fromType.baseType() == *_toType.baseType());
 		if (auto functionType = dynamic_cast<FunctionType const*>(_fromType.baseType()))
 		{
 			solAssert(functionType->equalExcludingStateMutability(
 				dynamic_cast<FunctionType const&>(*_toType.baseType())
 			));
-			sameType = true;
+			sameTypeFromStorage = fromStorage;
 		}
-		templ("sameType", sameType);
-		if (sameType)
+		templ("sameTypeFromStorage", sameTypeFromStorage);
+		if (sameTypeFromStorage)
 		{
 			templ("maskFull", maskLowerOrderBytesFunction(itemsPerSlot * _toType.storageStride()));
 			templ("maskBytes", maskLowerOrderBytesFunctionDynamic());
@@ -2088,24 +2097,32 @@ string YulUtilFunctions::copyValueArrayStorageToStorageFunction(ArrayType const&
 			templ("convert", conversionFunction(*_fromType.baseType(), *_toType.baseType()));
 			templ("prepareStore", prepareStoreFunction(*_toType.baseType()));
 		}
-		templ("updateSrcSlotValue", Whiskers(R"(
+		if (fromStorage)
+			templ("updateSrcPtr", Whiskers(R"(
 				<?srcReadMultiPerSlot>
 					srcItemIndexInSlot := add(srcItemIndexInSlot, 1)
 					if eq(srcItemIndexInSlot, <srcItemsPerSlot>) {
 						// here we are done with this slot, we need to read next one
-					srcSlot := add(srcSlot, 1)
-					srcSlotValue := sload(srcSlot)
+						srcPtr := add(srcPtr, 1)
+						srcSlotValue := sload(srcPtr)
 						srcItemIndexInSlot := 0
 					}
 				<!srcReadMultiPerSlot>
-				srcSlot := add(srcSlot, 1)
-				srcSlotValue := sload(srcSlot)
+					srcPtr := add(srcPtr, 1)
+					srcSlotValue := sload(srcPtr)
 				</srcReadMultiPerSlot>
 				)")
-			("srcReadMultiPerSlot", !sameType && _fromType.storageStride() <= 16)
+				("srcReadMultiPerSlot", !sameTypeFromStorage && _fromType.storageStride() <= 16)
 				("srcItemsPerSlot", to_string(32 / _fromType.storageStride()))
 				.render()
 			);
+		else
+			templ("updateSrcPtr", Whiskers(R"(
+					srcPtr := add(srcPtr, <srcStride>)
+				)")
+				("srcStride", fromCalldata ? to_string(_fromType.calldataStride()) : to_string(_fromType.memoryStride()))
+				.render()
+			);
 
 		return templ.render();
 	});

libsolidity/codegen/YulUtilFunctions.h

@@ -258,9 +258,9 @@ public:
 	/// signature (to_slot, from_ptr) ->
 	std::string copyByteArrayToStorageFunction(ArrayType const& _fromType, ArrayType const& _toType);
 
-	/// @returns the name of a function that will copy an array of value types from storage to storage.
-	/// signature (to_slot, from_slot) ->
-	std::string copyValueArrayStorageToStorageFunction(ArrayType const& _fromType, ArrayType const& _toType);
+	/// @returns the name of a function that will copy an array of value types to storage.
+	/// signature (to_slot, from_ptr[, from_length]) ->
+	std::string copyValueArrayToStorageFunction(ArrayType const& _fromType, ArrayType const& _toType);
 
 	/// Returns the name of a function that will convert a given length to the
 	/// size in memory (number of storage slots or calldata/memory bytes) it