From adb9d0c41afd351cc5aa4fae2408d38893e357b6 Mon Sep 17 00:00:00 2001 From: Djordje Mijovic Date: Thu, 3 Dec 2020 18:56:57 +0100 Subject: [PATCH] [Sol->Yul] Fixing array clearing when copying from storage to storage. --- libsolidity/codegen/YulUtilFunctions.cpp | 113 +++++++++++++++--- .../copying/array_copy_different_packing.sol | 2 + ...ay_copy_storage_storage_different_base.sol | 2 + ...ray_copy_storage_storage_static_simple.sol | 15 +++ .../copying/array_copy_target_leftover.sol | 2 + .../copying/array_copy_target_leftover2.sol | 3 +- .../copying/array_copy_target_simple.sol | 2 + .../copying/array_copy_target_simple_2.sol | 23 ++++ 8 files changed, 142 insertions(+), 20 deletions(-) create mode 100644 test/libsolidity/semanticTests/array/copying/array_copy_storage_storage_static_simple.sol create mode 100644 test/libsolidity/semanticTests/array/copying/array_copy_target_simple_2.sol diff --git a/libsolidity/codegen/YulUtilFunctions.cpp b/libsolidity/codegen/YulUtilFunctions.cpp index 66f052515..377cda4d0 100644 --- a/libsolidity/codegen/YulUtilFunctions.cpp +++ b/libsolidity/codegen/YulUtilFunctions.cpp @@ -1799,15 +1799,17 @@ string YulUtilFunctions::copyByteArrayToStorageFunction(ArrayType const& _fromTy string YulUtilFunctions::copyValueArrayStorageToStorageFunction(ArrayType const& _fromType, ArrayType const& _toType) { - solAssert( - *_fromType.copyForLocation(_toType.location(), _toType.isPointer()) == dynamic_cast(_toType), - "" - ); + solAssert(_fromType.baseType()->isValueType(), ""); + solAssert(_toType.baseType()->isValueType(), ""); + solAssert(_fromType.baseType()->isImplicitlyConvertibleTo(*_toType.baseType()), ""); + solAssert(!_fromType.isByteArray(), ""); - solAssert(_fromType.dataStoredIn(DataLocation::Storage) && _toType.baseType()->isValueType(), ""); + solAssert(!_toType.isByteArray(), ""); + solAssert(_fromType.dataStoredIn(DataLocation::Storage), ""); solAssert(_toType.dataStoredIn(DataLocation::Storage), ""); - solUnimplementedAssert(_fromType.storageStride() == _toType.storageStride(), ""); + solAssert(_fromType.storageStride() <= _toType.storageStride(), ""); + solAssert(_toType.storageStride() <= 32, ""); string functionName = "copy_array_to_storage_from_" + _fromType.identifier() + "_to_" + _toType.identifier(); return m_functionCollector.createFunction(functionName, [&](){ @@ -1819,19 +1821,60 @@ string YulUtilFunctions::copyValueArrayStorageToStorageFunction(ArrayType const& if gt(length, 0xffffffffffffffff) { () } (dst, length) - let srcPtr := (src) - - let dstPtr := (dst) + let srcSlot := (src) + let dstSlot := (dst) let fullSlots := div(length, ) - let i := 0 - for { } lt(i, fullSlots) { i := add(i, 1) } { - sstore(add(dstPtr, i), (sload(add(srcPtr, i)))) - } - let spill := sub(length, mul(i, )) - if gt(spill, 0) { - sstore(add(dstPtr, i), (sload(add(srcPtr, i)), mul(spill, ))) + + let srcSlotValue := sload(srcSlot) + let srcItemIndexInSlot := 0 + for { let i := 0 } lt(i, fullSlots) { i := add(i, 1) } { + let dstSlotValue := 0 + + dstSlotValue := (srcSlotValue) + + + for { let j := 0 } lt(j, ) { j := add(j, 1) } + { + let itemValue := ( + (srcSlotValue, mul(, srcItemIndexInSlot)) + ) + itemValue := (itemValue) + dstSlotValue := + + (dstSlotValue, mul(, j), itemValue) + + itemValue + + + + } + + + sstore(add(dstSlot, i), dstSlotValue) } + + + let spill := sub(length, mul(fullSlots, )) + if gt(spill, 0) { + let dstSlotValue := 0 + + dstSlotValue := (srcSlotValue, mul(spill, )) + + + for { let j := 0 } lt(j, spill) { j := add(j, 1) } { + let itemValue := ( + (srcSlotValue, mul(, srcItemIndexInSlot)) + ) + itemValue := (itemValue) + dstSlotValue := (dstSlotValue, mul(, j), itemValue) + + + } + + sstore(add(dstSlot, fullSlots), dstSlotValue) + } + } )"); if (_fromType.dataStoredIn(DataLocation::Storage)) @@ -1842,11 +1885,43 @@ string YulUtilFunctions::copyValueArrayStorageToStorageFunction(ArrayType const& templ("panic", panicFunction(PanicCode::ResourceError)); templ("srcDataLocation", arrayDataAreaFunction(_fromType)); templ("dstDataLocation", arrayDataAreaFunction(_toType)); + templ("srcStride", to_string(_fromType.storageStride())); unsigned itemsPerSlot = 32 / _toType.storageStride(); templ("itemsPerSlot", to_string(itemsPerSlot)); - templ("bytesPerItem", to_string(_toType.storageStride())); - templ("maskFull", maskLowerOrderBytesFunction(itemsPerSlot * _toType.storageStride())); - templ("maskBytes", maskLowerOrderBytesFunctionDynamic()); + templ("multipleItemsPerSlotDst", itemsPerSlot > 1); + bool sameType = _fromType.baseType() == _toType.baseType(); + templ("sameType", sameType); + if (sameType) + { + templ("maskFull", maskLowerOrderBytesFunction(itemsPerSlot * _toType.storageStride())); + templ("maskBytes", maskLowerOrderBytesFunctionDynamic()); + } + else + { + templ("dstStride", to_string(_toType.storageStride())); + templ("extractFromSlot", extractFromStorageValueDynamic(*_fromType.baseType())); + templ("updateByteSlice", updateByteSliceFunctionDynamic(_toType.storageStride())); + templ("convert", conversionFunction(*_fromType.baseType(), *_toType.baseType())); + templ("prepareStore", prepareStoreFunction(*_toType.baseType())); + } + templ("updateSrcSlotValue", Whiskers(R"( + + srcItemIndexInSlot := add(srcItemIndexInSlot, 1) + if eq(srcItemIndexInSlot, ) { + // here we are done with this slot, we need to read next one + srcSlot := add(srcSlot, 1) + srcSlotValue := sload(srcSlot) + srcItemIndexInSlot := 0 + } + + srcSlot := add(srcSlot, 1) + srcSlotValue := sload(srcSlot) + + )") + ("srcReadMultiPerSlot", !sameType && _fromType.storageStride() <= 16) + ("srcItemsPerSlot", to_string(32 / _fromType.storageStride())) + .render() + ); return templ.render(); }); diff --git a/test/libsolidity/semanticTests/array/copying/array_copy_different_packing.sol b/test/libsolidity/semanticTests/array/copying/array_copy_different_packing.sol index f0afcffc9..7e3bf79dc 100644 --- a/test/libsolidity/semanticTests/array/copying/array_copy_different_packing.sol +++ b/test/libsolidity/semanticTests/array/copying/array_copy_different_packing.sol @@ -17,5 +17,7 @@ contract c { } } +// ==== +// compileViaYul: also // ---- // test() -> 0x01000000000000000000000000000000000000000000000000, 0x02000000000000000000000000000000000000000000000000, 0x03000000000000000000000000000000000000000000000000, 0x04000000000000000000000000000000000000000000000000, 0x05000000000000000000000000000000000000000000000000 diff --git a/test/libsolidity/semanticTests/array/copying/array_copy_storage_storage_different_base.sol b/test/libsolidity/semanticTests/array/copying/array_copy_storage_storage_different_base.sol index e1b6cfeb1..fa35e8aeb 100644 --- a/test/libsolidity/semanticTests/array/copying/array_copy_storage_storage_different_base.sol +++ b/test/libsolidity/semanticTests/array/copying/array_copy_storage_storage_different_base.sol @@ -15,5 +15,7 @@ contract c { y = data2[4]; } } +// ==== +// compileViaYul: also // ---- // test() -> 5, 4 diff --git a/test/libsolidity/semanticTests/array/copying/array_copy_storage_storage_static_simple.sol b/test/libsolidity/semanticTests/array/copying/array_copy_storage_storage_static_simple.sol new file mode 100644 index 000000000..90e117ae1 --- /dev/null +++ b/test/libsolidity/semanticTests/array/copying/array_copy_storage_storage_static_simple.sol @@ -0,0 +1,15 @@ +contract C { + bytes1[2] data1; + bytes2[2] data2; + function test() public returns (bytes2, bytes2) { + uint i; + for (i = 0; i < data1.length; ++i) + data1[i] = bytes1(uint8(1 + i)); + data2 = data1; + return (data2[0], data2[1]); + } +} +// ==== +// compileViaYul: also +// ---- +// test() -> left(0x01), left(0x02) diff --git a/test/libsolidity/semanticTests/array/copying/array_copy_target_leftover.sol b/test/libsolidity/semanticTests/array/copying/array_copy_target_leftover.sol index 5e0cae64d..a9c2dcca0 100644 --- a/test/libsolidity/semanticTests/array/copying/array_copy_target_leftover.sol +++ b/test/libsolidity/semanticTests/array/copying/array_copy_target_leftover.sol @@ -15,5 +15,7 @@ contract c { res2 |= uint(uint16(data2[16 + i])) * 0x10000**i; } } +// ==== +// compileViaYul: also // ---- // test() -> 0xffffffff, 0x0000000000000000000000000a00090008000700060005000400030002000100, 0x0000000000000000000000000000000000000000000000000000000000000000 diff --git a/test/libsolidity/semanticTests/array/copying/array_copy_target_leftover2.sol b/test/libsolidity/semanticTests/array/copying/array_copy_target_leftover2.sol index 1bbd95f6f..fa35ec888 100644 --- a/test/libsolidity/semanticTests/array/copying/array_copy_target_leftover2.sol +++ b/test/libsolidity/semanticTests/array/copying/array_copy_target_leftover2.sol @@ -17,6 +17,7 @@ contract c { r3 = data2[5]; } } - +// ==== +// compileViaYul: also // ---- // test() -> 0x04000000000000000000000000000000000000000000000000, 0x0, 0x0 diff --git a/test/libsolidity/semanticTests/array/copying/array_copy_target_simple.sol b/test/libsolidity/semanticTests/array/copying/array_copy_target_simple.sol index ab82589c2..5f854c724 100644 --- a/test/libsolidity/semanticTests/array/copying/array_copy_target_simple.sol +++ b/test/libsolidity/semanticTests/array/copying/array_copy_target_simple.sol @@ -17,5 +17,7 @@ contract c { } } +// ==== +// compileViaYul: also // ---- // test() -> 0x01000000000000000000000000000000000000000000000000, 0x02000000000000000000000000000000000000000000000000, 0x03000000000000000000000000000000000000000000000000, 0x04000000000000000000000000000000000000000000000000, 0x0 diff --git a/test/libsolidity/semanticTests/array/copying/array_copy_target_simple_2.sol b/test/libsolidity/semanticTests/array/copying/array_copy_target_simple_2.sol new file mode 100644 index 000000000..f2358c718 --- /dev/null +++ b/test/libsolidity/semanticTests/array/copying/array_copy_target_simple_2.sol @@ -0,0 +1,23 @@ +contract c { + bytes9[7] data1; // 3 per slot + bytes32[10] data2; // 1 per slot + + function test() + public + returns (bytes32 a, bytes32 b, bytes32 c, bytes32 d, bytes32 e) + { + for (uint256 i = 0; i < data1.length; ++i) data1[i] = bytes8(uint64(i)); + data2[8] = data2[9] = bytes8(uint64(2)); + data2 = data1; + a = data2[1]; + b = data2[2]; + c = data2[3]; + d = data2[4]; + e = data2[9]; + } +} + +// ==== +// compileViaYul: also +// ---- +// test() -> 0x01000000000000000000000000000000000000000000000000, 0x02000000000000000000000000000000000000000000000000, 0x03000000000000000000000000000000000000000000000000, 0x04000000000000000000000000000000000000000000000000, 0x00