From b06936b11c287c4946ce475b6ddf60f3461b2a1d Mon Sep 17 00:00:00 2001 From: Djordje Mijovic Date: Mon, 21 Dec 2020 17:58:08 +0100 Subject: [PATCH 1/2] [refactor] Move copying struct to storage to it's own util function. --- libsolidity/codegen/YulUtilFunctions.cpp | 215 +++++++++++++---------- libsolidity/codegen/YulUtilFunctions.h | 4 + 2 files changed, 123 insertions(+), 96 deletions(-) diff --git a/libsolidity/codegen/YulUtilFunctions.cpp b/libsolidity/codegen/YulUtilFunctions.cpp index f58234692..6e706fa77 100644 --- a/libsolidity/codegen/YulUtilFunctions.cpp +++ b/libsolidity/codegen/YulUtilFunctions.cpp @@ -2601,111 +2601,18 @@ string YulUtilFunctions::updateStorageValueFunction( auto const& fromStructType = dynamic_cast(_fromType); auto const& toStructType = dynamic_cast(_toType); - solAssert(fromStructType.structDefinition() == toStructType.structDefinition(), ""); solAssert(_offset.value_or(0) == 0, ""); Whiskers templ(R"( function (slot, offset, value) { if offset { () } - if eq(slot, value) { leave } - <#member> - { - - } - + (slot, value) } )"); templ("functionName", functionName); templ("dynamicOffset", !_offset.has_value()); - templ("panic", panicFunction(PanicCode::Generic)); - templ("fromStorage", fromStructType.dataStoredIn(DataLocation::Storage)); - - MemberList::MemberMap structMembers = fromStructType.nativeMembers(nullptr); - MemberList::MemberMap toStructMembers = toStructType.nativeMembers(nullptr); - - vector> memberParams(structMembers.size()); - for (size_t i = 0; i < structMembers.size(); ++i) - { - Type const& memberType = *structMembers[i].type; - solAssert(memberType.memoryHeadSize() == 32, ""); - auto const& [slotDiff, offset] = toStructType.storageOffsetsOfMember(structMembers[i].name); - - Whiskers t(R"( - let memberSlot := add(slot, ) - let memberSrcPtr := add(value, ) - - - let := - - (value, memberSrcPtr) - - memberSrcPtr - - - - := () - - - - - let := (memberSrcPtr) - - - - let := - - (memberSrcPtr) - - memberSrcPtr - - - - (memberSlot, ) - )"); - bool fromCalldata = fromStructType.location() == DataLocation::CallData; - t("fromCalldata", fromCalldata); - bool fromMemory = fromStructType.location() == DataLocation::Memory; - t("fromMemory", fromMemory); - bool fromStorage = fromStructType.location() == DataLocation::Storage; - t("fromStorage", fromStorage); - t("isValueType", memberType.isValueType()); - t("memberValues", suffixedVariableNameList("memberValue_", 0, memberType.stackItems().size())); - - t("memberStorageSlotDiff", slotDiff.str()); - if (fromCalldata) - { - t("memberOffset", to_string(fromStructType.calldataOffsetOfMember(structMembers[i].name))); - t("dynamicallyEncodedMember", memberType.isDynamicallyEncoded()); - if (memberType.isDynamicallyEncoded()) - t("accessCalldataTail", accessCalldataTailFunction(memberType)); - if (memberType.isValueType()) - t("read", readFromCalldata(memberType)); - } - else if (fromMemory) - { - t("memberOffset", fromStructType.memoryOffsetOfMember(structMembers[i].name).str()); - t("read", readFromMemory(memberType)); - } - else if (fromStorage) - { - auto [srcSlotOffset, srcOffset] = fromStructType.storageOffsetsOfMember(structMembers[i].name); - t("memberOffset", formatNumber(srcSlotOffset)); - if (memberType.isValueType()) - t("read", readFromStorageValueType(memberType, srcOffset, false)); - else - solAssert(srcOffset == 0, ""); - - } - t("memberStorageSlotOffset", to_string(offset)); - t("updateStorageValue", updateStorageValueFunction( - memberType, - *toStructMembers[i].type, - optional{offset} - )); - memberParams[i]["updateMemberCall"] = t.render(); - } - templ("member", memberParams); - + templ("panic", panicFunction(util::PanicCode::Generic)); + templ("copyStructToStorage", copyStructToStorageFunction(fromStructType, toStructType)); return templ.render(); } }); @@ -3355,6 +3262,122 @@ string YulUtilFunctions::conversionFunction(Type const& _from, Type const& _to) }); } +string YulUtilFunctions::copyStructToStorageFunction(StructType const& _from, StructType const& _to) +{ + solAssert(_to.dataStoredIn(DataLocation::Storage), ""); + solAssert(_from.structDefinition() == _to.structDefinition(), ""); + + string functionName = + "copy_struct_to_storage_from_" + + _from.identifier() + + "_to_" + + _to.identifier(); + + return m_functionCollector.createFunction(functionName, [&]() { + Whiskers templ(R"( + function (slot, value) { + if eq(slot, value) { leave } + <#member> + { + + } + + } + )"); + templ("functionName", functionName); + templ("panic", panicFunction()); + templ("fromStorage", _from.dataStoredIn(DataLocation::Storage)); + + MemberList::MemberMap structMembers = _from.nativeMembers(nullptr); + MemberList::MemberMap toStructMembers = _to.nativeMembers(nullptr); + + vector> memberParams(structMembers.size()); + for (size_t i = 0; i < structMembers.size(); ++i) + { + Type const& memberType = *structMembers[i].type; + solAssert(memberType.memoryHeadSize() == 32, ""); + auto const&[slotDiff, offset] = _to.storageOffsetsOfMember(structMembers[i].name); + + Whiskers t(R"( + let memberSlot := add(slot, ) + let memberSrcPtr := add(value, ) + + + let := + + (value, memberSrcPtr) + + memberSrcPtr + + + + := () + + + + + let := (memberSrcPtr) + + + + let := + + (memberSrcPtr) + + memberSrcPtr + + + + (memberSlot, ) + )"); + bool fromCalldata = _from.location() == DataLocation::CallData; + t("fromCalldata", fromCalldata); + bool fromMemory = _from.location() == DataLocation::Memory; + t("fromMemory", fromMemory); + bool fromStorage = _from.location() == DataLocation::Storage; + t("fromStorage", fromStorage); + t("isValueType", memberType.isValueType()); + t("memberValues", suffixedVariableNameList("memberValue_", 0, memberType.stackItems().size())); + + t("memberStorageSlotDiff", slotDiff.str()); + if (fromCalldata) + { + t("memberOffset", to_string(_from.calldataOffsetOfMember(structMembers[i].name))); + t("dynamicallyEncodedMember", memberType.isDynamicallyEncoded()); + if (memberType.isDynamicallyEncoded()) + t("accessCalldataTail", accessCalldataTailFunction(memberType)); + if (memberType.isValueType()) + t("read", readFromCalldata(memberType)); + } + else if (fromMemory) + { + t("memberOffset", _from.memoryOffsetOfMember(structMembers[i].name).str()); + t("read", readFromMemory(memberType)); + } + else if (fromStorage) + { + auto[srcSlotOffset, srcOffset] = _from.storageOffsetsOfMember(structMembers[i].name); + t("memberOffset", formatNumber(srcSlotOffset)); + if (memberType.isValueType()) + t("read", readFromStorageValueType(memberType, srcOffset, false)); + else + solAssert(srcOffset == 0, ""); + + } + t("memberStorageSlotOffset", to_string(offset)); + t("updateStorageValue", updateStorageValueFunction( + memberType, + *toStructMembers[i].type, + optional{offset} + )); + memberParams[i]["updateMemberCall"] = t.render(); + } + templ("member", memberParams); + + return templ.render(); + }); +} + string YulUtilFunctions::arrayConversionFunction(ArrayType const& _from, ArrayType const& _to) { solAssert(_to.location() != DataLocation::CallData, ""); diff --git a/libsolidity/codegen/YulUtilFunctions.h b/libsolidity/codegen/YulUtilFunctions.h index 37924170b..7e78d16c4 100644 --- a/libsolidity/codegen/YulUtilFunctions.h +++ b/libsolidity/codegen/YulUtilFunctions.h @@ -483,6 +483,10 @@ public: std::string externalCodeFunction(); private: + /// @returns function that copies struct to storage + /// signature: (slot, value) -> + std::string copyStructToStorageFunction(StructType const& _from, StructType const& _to); + /// Special case of conversion functions - handles all array conversions. std::string arrayConversionFunction(ArrayType const& _from, ArrayType const& _to); From 85b8325f0b3810a503bdb461ad6dbf37c2ac949a Mon Sep 17 00:00:00 2001 From: Djordje Mijovic Date: Thu, 10 Dec 2020 10:13:16 +0100 Subject: [PATCH 2/2] [Sol->Yul] Implementing dynamic array push for arrays of structs. --- libsolidity/codegen/YulUtilFunctions.cpp | 82 +++++++++---------- libsolidity/codegen/YulUtilFunctions.h | 6 +- .../codegen/ir/IRGeneratorForStatements.cpp | 8 +- .../push/array_push_nested_from_calldata.sol | 16 ++++ .../push/array_push_nested_from_memory.sol | 19 +++++ .../array/push/array_push_struct.sol | 3 +- .../push/array_push_struct_from_calldata.sol | 20 +++++ 7 files changed, 104 insertions(+), 50 deletions(-) create mode 100644 test/libsolidity/semanticTests/array/push/array_push_nested_from_calldata.sol create mode 100644 test/libsolidity/semanticTests/array/push/array_push_nested_from_memory.sol create mode 100644 test/libsolidity/semanticTests/array/push/array_push_struct_from_calldata.sol diff --git a/libsolidity/codegen/YulUtilFunctions.cpp b/libsolidity/codegen/YulUtilFunctions.cpp index 6e706fa77..52639a5f6 100644 --- a/libsolidity/codegen/YulUtilFunctions.cpp +++ b/libsolidity/codegen/YulUtilFunctions.cpp @@ -1425,15 +1425,23 @@ string YulUtilFunctions::storageByteArrayPopFunction(ArrayType const& _type) }); } -string YulUtilFunctions::storageArrayPushFunction(ArrayType const& _type) +string YulUtilFunctions::storageArrayPushFunction(ArrayType const& _type, Type const* _fromType) { solAssert(_type.location() == DataLocation::Storage, ""); solAssert(_type.isDynamicallySized(), ""); + if (!_fromType) + _fromType = _type.baseType(); + else if (_fromType->isValueType()) + solUnimplementedAssert(*_fromType == *_type.baseType(), ""); - string functionName = "array_push_" + _type.identifier(); + string functionName = + string{"array_push_from_"} + + _fromType->identifier() + + "_to_" + + _type.identifier(); return m_functionCollector.createFunction(functionName, [&]() { return Whiskers(R"( - function (array, value) { + function (array ) { let data := sload(array) let oldLen := (data) @@ -1441,7 +1449,7 @@ string YulUtilFunctions::storageArrayPushFunction(ArrayType const& _type) switch gt(oldLen, 31) case 0 { - value := byte(0, value) + let value := byte(0 ) switch oldLen case 31 { // Here we have special case when array switches from short array to long array @@ -1464,23 +1472,24 @@ string YulUtilFunctions::storageArrayPushFunction(ArrayType const& _type) default { sstore(array, add(data, 2)) let slot, offset := (array, oldLen) - (slot, offset, value) + (slot, offset ) } let oldLen := sload(array) if iszero(lt(oldLen, )) { () } sstore(array, add(oldLen, 1)) let slot, offset := (array, oldLen) - (slot, offset, value) + (slot, offset ) })") ("functionName", functionName) + ("values", _fromType->sizeOnStack() == 0 ? "" : ", " + suffixedVariableNameList("value", 0, _fromType->sizeOnStack())) ("panic", panicFunction(PanicCode::ResourceError)) ("extractByteArrayLength", _type.isByteArray() ? extractByteArrayLengthFunction() : "") ("dataAreaFunction", arrayDataAreaFunction(_type)) ("isByteArray", _type.isByteArray()) ("indexAccess", storageArrayIndexAccessFunction(_type)) - ("storeValue", updateStorageValueFunction(*_type.baseType(), *_type.baseType())) + ("storeValue", updateStorageValueFunction(*_fromType, *_type.baseType())) ("maxArrayLength", (u256(1) << 64).str()) ("shl", shiftLeftFunctionDynamic()) ("shr", shiftRightFunction(248)) @@ -2572,49 +2581,32 @@ string YulUtilFunctions::updateStorageValueFunction( fromReferenceType->location(), fromReferenceType->isPointer() ).get() == *fromReferenceType, ""); + solAssert(toReferenceType->category() == fromReferenceType->category(), ""); + solAssert(_offset.value_or(0) == 0, ""); - if (_toType.category() == Type::Category::Array) - { - solAssert(_offset.value_or(0) == 0, ""); - - Whiskers templ(R"( - function (slot, offset, ) { - if offset { () } - (slot, ) - } - )"); - templ("functionName", functionName); - templ("dynamicOffset", !_offset.has_value()); - templ("panic", panicFunction(PanicCode::Generic)); - templ("value", suffixedVariableNameList("value_", 0, _fromType.sizeOnStack())); - templ("copyArrayToStorage", copyArrayToStorageFunction( + Whiskers templ(R"( + function (slot, offset, ) { + if offset { () } + (slot, ) + } + )"); + templ("functionName", functionName); + templ("dynamicOffset", !_offset.has_value()); + templ("panic", panicFunction(PanicCode::Generic)); + templ("value", suffixedVariableNameList("value_", 0, _fromType.sizeOnStack())); + if (_fromType.category() == Type::Category::Array) + templ("copyToStorage", copyArrayToStorageFunction( dynamic_cast(_fromType), dynamic_cast(_toType) )); - - return templ.render(); - } else - { - solAssert(_toType.category() == Type::Category::Struct, ""); + templ("copyToStorage", copyStructToStorageFunction( + dynamic_cast(_fromType), + dynamic_cast(_toType) + )); - auto const& fromStructType = dynamic_cast(_fromType); - auto const& toStructType = dynamic_cast(_toType); - solAssert(_offset.value_or(0) == 0, ""); - - Whiskers templ(R"( - function (slot, offset, value) { - if offset { () } - (slot, value) - } - )"); - templ("functionName", functionName); - templ("dynamicOffset", !_offset.has_value()); - templ("panic", panicFunction(util::PanicCode::Generic)); - templ("copyStructToStorage", copyStructToStorageFunction(fromStructType, toStructType)); - return templ.render(); - } + return templ.render(); }); } @@ -3276,16 +3268,16 @@ string YulUtilFunctions::copyStructToStorageFunction(StructType const& _from, St return m_functionCollector.createFunction(functionName, [&]() { Whiskers templ(R"( function (slot, value) { - if eq(slot, value) { leave } + if iszero(eq(slot, value)) { <#member> { } + } } )"); templ("functionName", functionName); - templ("panic", panicFunction()); templ("fromStorage", _from.dataStoredIn(DataLocation::Storage)); MemberList::MemberMap structMembers = _from.nativeMembers(nullptr); diff --git a/libsolidity/codegen/YulUtilFunctions.h b/libsolidity/codegen/YulUtilFunctions.h index 7e78d16c4..6f1642f52 100644 --- a/libsolidity/codegen/YulUtilFunctions.h +++ b/libsolidity/codegen/YulUtilFunctions.h @@ -212,8 +212,10 @@ public: std::string storageArrayPopFunction(ArrayType const& _type); /// @returns the name of a function that pushes an element to a storage array +/// @param _fromType represents the type of the element being pushed. +/// If _fromType is ReferenceType the function will perform deep copy. /// signature: (array, value) - std::string storageArrayPushFunction(ArrayType const& _type); + std::string storageArrayPushFunction(ArrayType const& _type, TypePointer _fromType = nullptr); /// @returns the name of a function that pushes the base type's zero element to a storage array and returns storage slot and offset of the added element. /// signature: (array) -> slot, offset @@ -483,7 +485,7 @@ public: std::string externalCodeFunction(); private: - /// @returns function that copies struct to storage +/// @returns the name of a function that copies a struct from calldata or memory to storage /// signature: (slot, value) -> std::string copyStructToStorageFunction(StructType const& _from, StructType const& _to); diff --git a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp index 884edb1bd..bf9e8a8e8 100644 --- a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp +++ b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp @@ -1316,9 +1316,13 @@ void IRGeneratorForStatements::endVisit(FunctionCall const& _functionCall) } else { - IRVariable argument = convert(*arguments.front(), *arrayType.baseType()); + IRVariable argument = + arrayType.baseType()->isValueType() ? + convert(*arguments.front(), *arrayType.baseType()) : + *arguments.front(); + m_code << - m_utils.storageArrayPushFunction(arrayType) << + m_utils.storageArrayPushFunction(arrayType, &argument.type()) << "(" << IRVariable(_functionCall.expression()).commaSeparatedList() << ", " << diff --git a/test/libsolidity/semanticTests/array/push/array_push_nested_from_calldata.sol b/test/libsolidity/semanticTests/array/push/array_push_nested_from_calldata.sol new file mode 100644 index 000000000..66df10a6e --- /dev/null +++ b/test/libsolidity/semanticTests/array/push/array_push_nested_from_calldata.sol @@ -0,0 +1,16 @@ +contract C { + uint8 b = 23; + uint120[][] s; + uint8 a = 17; + function f(uint120[] calldata c) public returns(uint120) { + s.push(c); + assert(s.length == 1); + assert(s[0].length == c.length); + assert(s[0].length > 0); + return s[0][0]; + } +} +// ==== +// compileViaYul: also +// ---- +// f(uint120[]): 0x20, 3, 1, 2, 3 -> 1 diff --git a/test/libsolidity/semanticTests/array/push/array_push_nested_from_memory.sol b/test/libsolidity/semanticTests/array/push/array_push_nested_from_memory.sol new file mode 100644 index 000000000..8a975e0b8 --- /dev/null +++ b/test/libsolidity/semanticTests/array/push/array_push_nested_from_memory.sol @@ -0,0 +1,19 @@ +contract C { + uint8 b = 23; + uint120[][] s; + uint8 a = 17; + function f() public returns(uint120) { + delete s; + uint120[] memory m = new uint120[](3); + m[0] = 1; + s.push(m); + assert(s.length == 1); + assert(s[0].length == m.length); + assert(s[0].length > 0); + return s[0][0]; + } +} +// ==== +// compileViaYul: also +// ---- +// f() -> 1 diff --git a/test/libsolidity/semanticTests/array/push/array_push_struct.sol b/test/libsolidity/semanticTests/array/push/array_push_struct.sol index e407fc025..38e6957b7 100644 --- a/test/libsolidity/semanticTests/array/push/array_push_struct.sol +++ b/test/libsolidity/semanticTests/array/push/array_push_struct.sol @@ -18,6 +18,7 @@ contract c { return (data[0].a, data[0].b, data[0].c[2], data[0].d[2]); } } - +// ==== +// compileViaYul: also // ---- // test() -> 2, 3, 4, 5 diff --git a/test/libsolidity/semanticTests/array/push/array_push_struct_from_calldata.sol b/test/libsolidity/semanticTests/array/push/array_push_struct_from_calldata.sol new file mode 100644 index 000000000..11ac40f0e --- /dev/null +++ b/test/libsolidity/semanticTests/array/push/array_push_struct_from_calldata.sol @@ -0,0 +1,20 @@ +pragma abicoder v2; + +contract c { + struct S { + uint16 a; + uint16 b; + uint16[3] c; + uint16[] d; + } + S[] data; + + function test(S calldata c) public returns (uint16, uint16, uint16, uint16) { + data.push(c); + return (data[0].a, data[0].b, data[0].c[2], data[0].d[2]); + } +} +// ==== +// compileViaYul: also +// ---- +// test((uint16, uint16, uint16[3], uint16[])): 0x20, 2, 3, 0, 0, 4, 0xC0, 4, 0, 0, 5, 0, 0 -> 2, 3, 4, 5