From e9dcfb0b624e5443942451fc865c154a2c5a73d7 Mon Sep 17 00:00:00 2001 From: bitshift Date: Fri, 9 Mar 2018 17:46:24 +0100 Subject: [PATCH 1/5] Implements pop() for value type arrays. --- libsolidity/ast/Types.cpp | 9 +++ libsolidity/ast/Types.h | 2 + libsolidity/codegen/ArrayUtils.cpp | 33 +++++++++++ libsolidity/codegen/ArrayUtils.h | 5 ++ libsolidity/codegen/ExpressionCompiler.cpp | 27 ++++++++- test/libsolidity/SolidityEndToEndTest.cpp | 66 ++++++++++++++++++++++ 6 files changed, 141 insertions(+), 1 deletion(-) diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 59668e1d4..1c6003cd1 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -1689,6 +1689,7 @@ MemberList::MemberMap ArrayType::nativeMembers(ContractDefinition const*) const { members.push_back({"length", make_shared(256)}); if (isDynamicallySized() && location() == DataLocation::Storage) + { members.push_back({"push", make_shared( TypePointers{baseType()}, TypePointers{make_shared(256)}, @@ -1696,6 +1697,14 @@ MemberList::MemberMap ArrayType::nativeMembers(ContractDefinition const*) const strings{string()}, isByteArray() ? FunctionType::Kind::ByteArrayPush : FunctionType::Kind::ArrayPush )}); + members.push_back({"pop", make_shared( + TypePointers{}, + TypePointers{}, + strings{string()}, + strings{string()}, + isByteArray() ? FunctionType::Kind::ByteArrayPop : FunctionType::Kind::ArrayPop + )}); + } } return members; } diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index 7c6f179bb..29c7db861 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -914,7 +914,9 @@ public: AddMod, ///< ADDMOD MulMod, ///< MULMOD ArrayPush, ///< .push() to a dynamically sized array in storage + ArrayPop, ///< .pop() from a dynamically sized array in storage ByteArrayPush, ///< .push() to a dynamically sized byte array in storage + ByteArrayPop, ///< .pop() from a dynamically sized byte array in storage ObjectCreation, ///< array creation using new Assert, ///< assert() Require, ///< require() diff --git a/libsolidity/codegen/ArrayUtils.cpp b/libsolidity/codegen/ArrayUtils.cpp index 0fe66d2d9..b434fddd2 100644 --- a/libsolidity/codegen/ArrayUtils.cpp +++ b/libsolidity/codegen/ArrayUtils.cpp @@ -823,6 +823,39 @@ void ArrayUtils::incrementDynamicArraySize(ArrayType const& _type) const })", {"ref"}); } +void ArrayUtils::popStorageArrayElement(ArrayType const& _type) const +{ + solAssert(_type.location() == DataLocation::Storage, ""); + solAssert(_type.isDynamicallySized(), ""); + if (!_type.isByteArray() && _type.baseType()->storageBytes() < 32) + solAssert(_type.baseType()->isValueType(), "Invalid storage size for non-value type."); + + // stack: ArrayReference + retrieveLength(_type); + // stack: ArrayReference oldLength + m_context << Instruction::DUP1; + // stack: ArrayReference oldLength oldLength + m_context << Instruction::ISZERO; + m_context.appendConditionalInvalid(); + + if (_type.isByteArray()) + { + } + else + { + // Stack: ArrayReference oldLength + m_context << u256(1) << Instruction::SWAP1 << Instruction::SUB; + // Stack ArrayReference newLength + m_context << Instruction::DUP2 << Instruction::DUP2; + // Stack ArrayReference newLength ArrayReference newLength; + accessIndex(_type, false); + // Stack: ArrayReference newLength storage_slot byte_offset + StorageItem(m_context, _type).setToZero(SourceLocation(), true); + // Stack: ArrayReference newLength + m_context << Instruction::SSTORE; + } +} + void ArrayUtils::clearStorageLoop(TypePointer const& _type) const { m_context.callLowLevelFunction( diff --git a/libsolidity/codegen/ArrayUtils.h b/libsolidity/codegen/ArrayUtils.h index 99786397d..84d591d7e 100644 --- a/libsolidity/codegen/ArrayUtils.h +++ b/libsolidity/codegen/ArrayUtils.h @@ -73,6 +73,11 @@ public: /// Stack pre: reference (excludes byte offset) /// Stack post: new_length void incrementDynamicArraySize(ArrayType const& _type) const; + /// Decrements the size of a dynamic array by one if length is nonzero. Returns otherwise. + /// Clears the removed data element. In case of a byte array, this might move the data. + /// Stack pre: reference + /// Stack post: + void popStorageArrayElement(ArrayType const& _type) const; /// Appends a loop that clears a sequence of storage slots of the given type (excluding end). /// Stack pre: end_ref start_ref /// Stack post: end_ref diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index 4bcc1fa91..ac7610fc5 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -866,6 +866,20 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) StorageByteArrayElement(m_context).storeValue(*type, _functionCall.location(), true); break; } + case FunctionType::Kind::ByteArrayPop: + case FunctionType::Kind::ArrayPop: + { + _functionCall.expression().accept(*this); + solAssert(function.parameterTypes().empty(), ""); + + ArrayType const& arrayType = dynamic_cast( + *dynamic_cast(_functionCall.expression()).expression().annotation().type + ); + solAssert(arrayType.dataStoredIn(DataLocation::Storage), ""); + + ArrayUtils(m_context).popStorageArrayElement(arrayType); + break; + } case FunctionType::Kind::ObjectCreation: { ArrayType const& arrayType = dynamic_cast(*_functionCall.annotation().type); @@ -1348,10 +1362,21 @@ bool ExpressionCompiler::visit(MemberAccess const& _memberAccess) else if (member == "push") { solAssert( - type.isDynamicallySized() && type.location() == DataLocation::Storage, + type.isDynamicallySized() && + type.location() == DataLocation::Storage && + type.category() == Type::Category::Array, "Tried to use .push() on a non-dynamically sized array" ); } + else if (member == "pop") + { + solAssert( + type.isDynamicallySized() && + type.location() == DataLocation::Storage && + type.category() == Type::Category::Array, + "Tried to use .pop() on a non-dynamically sized array" + ); + } else solAssert(false, "Illegal array member."); break; diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 40962294a..b4cf69503 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -5111,6 +5111,72 @@ BOOST_AUTO_TEST_CASE(byte_array_push_transition) ABI_CHECK(callContractFunction("test()"), encodeArgs(0)); } +BOOST_AUTO_TEST_CASE(array_pop) +{ + char const* sourceCode = R"( + contract c { + uint[] data; + function test() public returns (uint x, uint y, uint l) { + data.push(7); + x = data.push(3); + data.pop(); + y = data.push(2); + l = data.length; + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("test()"), encodeArgs(2, 2, 2)); +} + +BOOST_AUTO_TEST_CASE(array_pop_empty) +{ + char const* sourceCode = R"( + contract c { + uint[] data; + function test() public returns (bool) { + data.pop(); + return true; + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("test()"), encodeArgs()); +} + +BOOST_AUTO_TEST_CASE(bytearray_pop) +{ + char const* sourceCode = R"( + contract c { + bytes data; + function test() public returns (uint x, uint y, uint l) { + data.push(7); + x = data.push(3); + data.pop(); + data.pop(); + y = data.push(2); + l = data.length; + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("test()"), encodeArgs(2, 1, 1)); +} + +BOOST_AUTO_TEST_CASE(bytearray_pop_empty) +{ + char const* sourceCode = R"( + contract c { + bytes data; + function test() public returns (bool) { + data.pop(); + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("test()"), encodeArgs()); +} + BOOST_AUTO_TEST_CASE(external_array_args) { char const* sourceCode = R"( From 7156a01acc822ab66c189435421564afc8b1c922 Mon Sep 17 00:00:00 2001 From: Erik Kundt Date: Fri, 16 Mar 2018 17:06:38 +0100 Subject: [PATCH 2/5] Implements pop() for byte arrays. --- libsolidity/codegen/ArrayUtils.cpp | 75 ++++++++++++++++--- test/libsolidity/SolidityEndToEndTest.cpp | 87 +++++++++++++++++++++-- 2 files changed, 146 insertions(+), 16 deletions(-) diff --git a/libsolidity/codegen/ArrayUtils.cpp b/libsolidity/codegen/ArrayUtils.cpp index b434fddd2..096b2d4e7 100644 --- a/libsolidity/codegen/ArrayUtils.cpp +++ b/libsolidity/codegen/ArrayUtils.cpp @@ -830,19 +830,74 @@ void ArrayUtils::popStorageArrayElement(ArrayType const& _type) const if (!_type.isByteArray() && _type.baseType()->storageBytes() < 32) solAssert(_type.baseType()->isValueType(), "Invalid storage size for non-value type."); - // stack: ArrayReference - retrieveLength(_type); - // stack: ArrayReference oldLength - m_context << Instruction::DUP1; - // stack: ArrayReference oldLength oldLength - m_context << Instruction::ISZERO; - m_context.appendConditionalInvalid(); - if (_type.isByteArray()) { + m_context.appendInlineAssembly(R"({ + let slot_value := sload(ref) + switch and(slot_value, 1) + case 0 { + // short byte array + let length := and(div(slot_value, 2), 0x3f) + if iszero(length) { invalid() } + + // Zero-out the suffix of the byte array by masking it. + // Do not zero-out the least significant byte, but mask the + // higher bits of the length. + // (((1<<(8 * (32 - length))) - 1) << 8) + 128 + let mask := add(mul(0x100, sub(exp(0x100, sub(32, length)), 1)), 0x80) + slot_value := and(not(mask), slot_value) + + // Reduce the length by 1 + slot_value := sub(slot_value, 2) + sstore(ref, slot_value) + } + case 1 { + // long byte array + let length := div(slot_value, 2) + mstore(0, ref) + + switch length + case 32 + { + let slot := keccak256(0, 0x20) + let data := sload(slot) + sstore(slot, 0) + data := and(data, not(0xff)) + sstore(ref, or(data, 62)) + } + default + { + let slot := div(sub(length, 1), 32) + let offset := and(sub(length, 1), 0x1f) + slot := add(keccak256(0, 0x20), slot) + let data := sload(slot) + + // Zero-out the suffix of the byte array by masking it. + // ((1<<(8 * (32 - offset))) - 1) + let mask := sub(exp(0x100, sub(32, offset)), 1) + data := and(not(mask), data) + sstore(slot, data) + + // Reduce the length by 1 + slot_value := sub(slot_value, 2) + sstore(ref, slot_value) + } + } + })", {"ref"}); + m_context << Instruction::POP; } else - { + { + + // stack: ArrayReference + retrieveLength(_type); + // stack: ArrayReference oldLength + m_context << Instruction::DUP1; + // stack: ArrayReference oldLength oldLength + m_context << Instruction::ISZERO; + m_context.appendConditionalInvalid(); + + // Stack: ArrayReference oldLength m_context << u256(1) << Instruction::SWAP1 << Instruction::SUB; // Stack ArrayReference newLength @@ -852,7 +907,7 @@ void ArrayUtils::popStorageArrayElement(ArrayType const& _type) const // Stack: ArrayReference newLength storage_slot byte_offset StorageItem(m_context, _type).setToZero(SourceLocation(), true); // Stack: ArrayReference newLength - m_context << Instruction::SSTORE; + m_context << Instruction::SWAP1 << Instruction::SSTORE; } } diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index b4cf69503..411012233 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -5115,21 +5115,23 @@ BOOST_AUTO_TEST_CASE(array_pop) { char const* sourceCode = R"( contract c { + uint256 a; uint[] data; - function test() public returns (uint x, uint y, uint l) { + function test() public returns (uint x, uint l) { data.push(7); x = data.push(3); data.pop(); - y = data.push(2); + x = data.length; + data.pop(); l = data.length; } } )"; compileAndRun(sourceCode); - ABI_CHECK(callContractFunction("test()"), encodeArgs(2, 2, 2)); + ABI_CHECK(callContractFunction("test()"), encodeArgs(1, 0)); } -BOOST_AUTO_TEST_CASE(array_pop_empty) +BOOST_AUTO_TEST_CASE(array_pop_empty_exception) { char const* sourceCode = R"( contract c { @@ -5144,7 +5146,23 @@ BOOST_AUTO_TEST_CASE(array_pop_empty) ABI_CHECK(callContractFunction("test()"), encodeArgs()); } -BOOST_AUTO_TEST_CASE(bytearray_pop) +BOOST_AUTO_TEST_CASE(array_pop_storage_empty) +{ + char const* sourceCode = R"( + contract c { + uint[] data; + function test() public { + data.push(7); + data.pop(); + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("test()"), encodeArgs()); + BOOST_CHECK(storageEmpty(m_contractAddress)); +} + +BOOST_AUTO_TEST_CASE(byte_array_pop) { char const* sourceCode = R"( contract c { @@ -5163,13 +5181,31 @@ BOOST_AUTO_TEST_CASE(bytearray_pop) ABI_CHECK(callContractFunction("test()"), encodeArgs(2, 1, 1)); } -BOOST_AUTO_TEST_CASE(bytearray_pop_empty) +BOOST_AUTO_TEST_CASE(byte_array_pop_long) +{ + char const* sourceCode = R"( + contract c { + bytes data; + function test() public returns (uint l) { + for (uint i = 0; i < 33; i++) + data.push(byte(i)); + data.pop(); + l = data.length; + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("test()"), encodeArgs(32)); +} + +BOOST_AUTO_TEST_CASE(byte_array_pop_empty_exception) { char const* sourceCode = R"( contract c { bytes data; function test() public returns (bool) { data.pop(); + return true; } } )"; @@ -5177,6 +5213,45 @@ BOOST_AUTO_TEST_CASE(bytearray_pop_empty) ABI_CHECK(callContractFunction("test()"), encodeArgs()); } +BOOST_AUTO_TEST_CASE(byte_array_pop_storage_empty) +{ + char const* sourceCode = R"( + contract c { + bytes data; + function test() public { + data.push(7); + data.push(5); + data.push(3); + data.pop(); + data.pop(); + data.pop(); + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("test()"), encodeArgs()); + BOOST_CHECK(storageEmpty(m_contractAddress)); +} + +BOOST_AUTO_TEST_CASE(byte_array_pop_storage_empty_long) +{ + char const* sourceCode = R"( + contract c { + bytes data; + function test() public returns (uint l) { + for (uint i = 0; i < 33; i++) + data.push(3); + for (uint j = 0; j < 33; j++) + data.pop(); + l = data.length; + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("test()"), encodeArgs(0)); + BOOST_CHECK(storageEmpty(m_contractAddress)); +} + BOOST_AUTO_TEST_CASE(external_array_args) { char const* sourceCode = R"( From 34b5eca1f8d9a8f04db20139601c6e944532f4e4 Mon Sep 17 00:00:00 2001 From: Erik Kundt Date: Wed, 4 Apr 2018 18:21:06 +0200 Subject: [PATCH 3/5] Improves assembly and adds more tests. --- libsolidity/ast/Types.cpp | 2 +- libsolidity/ast/Types.h | 1 - libsolidity/codegen/ArrayUtils.cpp | 32 ++---- libsolidity/codegen/ExpressionCompiler.cpp | 14 +-- test/libsolidity/SolidityEndToEndTest.cpp | 125 ++++++++++++++++++++- 5 files changed, 139 insertions(+), 35 deletions(-) diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 1c6003cd1..8376b4bcc 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -1702,7 +1702,7 @@ MemberList::MemberMap ArrayType::nativeMembers(ContractDefinition const*) const TypePointers{}, strings{string()}, strings{string()}, - isByteArray() ? FunctionType::Kind::ByteArrayPop : FunctionType::Kind::ArrayPop + FunctionType::Kind::ArrayPop )}); } } diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index 29c7db861..b2f34dee4 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -916,7 +916,6 @@ public: ArrayPush, ///< .push() to a dynamically sized array in storage ArrayPop, ///< .pop() from a dynamically sized array in storage ByteArrayPush, ///< .push() to a dynamically sized byte array in storage - ByteArrayPop, ///< .pop() from a dynamically sized byte array in storage ObjectCreation, ///< array creation using new Assert, ///< assert() Require, ///< require() diff --git a/libsolidity/codegen/ArrayUtils.cpp b/libsolidity/codegen/ArrayUtils.cpp index 096b2d4e7..d78e64a92 100644 --- a/libsolidity/codegen/ArrayUtils.cpp +++ b/libsolidity/codegen/ArrayUtils.cpp @@ -837,29 +837,23 @@ void ArrayUtils::popStorageArrayElement(ArrayType const& _type) const switch and(slot_value, 1) case 0 { // short byte array - let length := and(div(slot_value, 2), 0x3f) + let length := and(div(slot_value, 2), 0x1f) if iszero(length) { invalid() } - // Zero-out the suffix of the byte array by masking it. - // Do not zero-out the least significant byte, but mask the - // higher bits of the length. - // (((1<<(8 * (32 - length))) - 1) << 8) + 128 - let mask := add(mul(0x100, sub(exp(0x100, sub(32, length)), 1)), 0x80) - slot_value := and(not(mask), slot_value) - - // Reduce the length by 1 - slot_value := sub(slot_value, 2) + // Zero-out the suffix inlcluding the least significant byte. + let mask := sub(exp(0x100, sub(33, length)), 1) + length := sub(length, 1) + slot_value := or(and(not(mask), slot_value), mul(length, 2)) sstore(ref, slot_value) } case 1 { // long byte array let length := div(slot_value, 2) + let slot := keccak256(0, 0x20) mstore(0, ref) - switch length case 32 { - let slot := keccak256(0, 0x20) let data := sload(slot) sstore(slot, 0) data := and(data, not(0xff)) @@ -867,14 +861,14 @@ void ArrayUtils::popStorageArrayElement(ArrayType const& _type) const } default { - let slot := div(sub(length, 1), 32) - let offset := and(sub(length, 1), 0x1f) - slot := add(keccak256(0, 0x20), slot) + let slot_offset := div(sub(length, 1), 32) + let length_offset := and(sub(length, 1), 0x1f) + slot := add(slot, slot_offset) let data := sload(slot) // Zero-out the suffix of the byte array by masking it. // ((1<<(8 * (32 - offset))) - 1) - let mask := sub(exp(0x100, sub(32, offset)), 1) + let mask := sub(exp(0x100, sub(32, length_offset)), 1) data := and(not(mask), data) sstore(slot, data) @@ -887,8 +881,7 @@ void ArrayUtils::popStorageArrayElement(ArrayType const& _type) const m_context << Instruction::POP; } else - { - + { // stack: ArrayReference retrieveLength(_type); // stack: ArrayReference oldLength @@ -897,7 +890,6 @@ void ArrayUtils::popStorageArrayElement(ArrayType const& _type) const m_context << Instruction::ISZERO; m_context.appendConditionalInvalid(); - // Stack: ArrayReference oldLength m_context << u256(1) << Instruction::SWAP1 << Instruction::SUB; // Stack ArrayReference newLength @@ -905,7 +897,7 @@ void ArrayUtils::popStorageArrayElement(ArrayType const& _type) const // Stack ArrayReference newLength ArrayReference newLength; accessIndex(_type, false); // Stack: ArrayReference newLength storage_slot byte_offset - StorageItem(m_context, _type).setToZero(SourceLocation(), true); + StorageItem(m_context, *_type.baseType()).setToZero(SourceLocation(), true); // Stack: ArrayReference newLength m_context << Instruction::SWAP1 << Instruction::SSTORE; } diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index ac7610fc5..93d440c85 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -866,7 +866,6 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) StorageByteArrayElement(m_context).storeValue(*type, _functionCall.location(), true); break; } - case FunctionType::Kind::ByteArrayPop: case FunctionType::Kind::ArrayPop: { _functionCall.expression().accept(*this); @@ -1359,22 +1358,13 @@ bool ExpressionCompiler::visit(MemberAccess const& _memberAccess) break; } } - else if (member == "push") + else if (member == "push" || member == "pop") { solAssert( type.isDynamicallySized() && type.location() == DataLocation::Storage && type.category() == Type::Category::Array, - "Tried to use .push() on a non-dynamically sized array" - ); - } - else if (member == "pop") - { - solAssert( - type.isDynamicallySized() && - type.location() == DataLocation::Storage && - type.category() == Type::Category::Array, - "Tried to use .pop() on a non-dynamically sized array" + "Tried to use ." + member + "() on a non-dynamically sized array" ); } else diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 411012233..b0ab15c8e 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -5115,7 +5115,6 @@ BOOST_AUTO_TEST_CASE(array_pop) { char const* sourceCode = R"( contract c { - uint256 a; uint[] data; function test() public returns (uint x, uint l) { data.push(7); @@ -5131,6 +5130,86 @@ BOOST_AUTO_TEST_CASE(array_pop) ABI_CHECK(callContractFunction("test()"), encodeArgs(1, 0)); } +BOOST_AUTO_TEST_CASE(array_pop_uint16_transition) +{ + char const* sourceCode = R"( + contract c { + uint16[] data; + function test() public returns (uint16 x, uint16 y, uint16 z) { + for (uint i = 1; i <= 48; i++) + data.push(uint16(i)); + for (uint j = 1; j <= 10; j++) + data.pop(); + x = data[data.length - 1]; + for (uint k = 1; k <= 10; k++) + data.pop(); + y = data[data.length - 1]; + for (uint l = 1; l <= 10; l++) + data.pop(); + z = data[data.length - 1]; + for (uint m = 1; m <= 18; m++) + data.pop(); + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("test()"), encodeArgs(38, 28, 18)); + BOOST_CHECK(storageEmpty(m_contractAddress)); +} + +BOOST_AUTO_TEST_CASE(array_pop_uint24_transition) +{ + char const* sourceCode = R"( + contract c { + uint24[] data; + function test() public returns (uint24 x, uint24 y) { + for (uint i = 1; i <= 30; i++) + data.push(uint24(i)); + for (uint j = 1; j <= 10; j++) + data.pop(); + x = data[data.length - 1]; + for (uint k = 1; k <= 10; k++) + data.pop(); + y = data[data.length - 1]; + for (uint l = 1; l <= 10; l++) + data.pop(); + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("test()"), encodeArgs(20, 10)); + BOOST_CHECK(storageEmpty(m_contractAddress)); +} + +BOOST_AUTO_TEST_CASE(array_pop_array_transition) +{ + char const* sourceCode = R"( + contract c { + uint16[] inner = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]; + uint16[][] data; + function test() public returns (uint x, uint y, uint z) { + for (uint i = 1; i <= 48; i++) + data.push(inner); + for (uint j = 1; j <= 10; j++) + data.pop(); + x = data[data.length - 1][0]; + for (uint k = 1; k <= 10; k++) + data.pop(); + y = data[data.length - 1][1]; + for (uint l = 1; l <= 10; l++) + data.pop(); + z = data[data.length - 1][2]; + for (uint m = 1; m <= 18; m++) + data.pop(); + delete inner; + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("test()"), encodeArgs(1, 2, 3)); + BOOST_CHECK(storageEmpty(m_contractAddress)); +} + BOOST_AUTO_TEST_CASE(array_pop_empty_exception) { char const* sourceCode = R"( @@ -5252,6 +5331,50 @@ BOOST_AUTO_TEST_CASE(byte_array_pop_storage_empty_long) BOOST_CHECK(storageEmpty(m_contractAddress)); } +BOOST_AUTO_TEST_CASE(byte_array_pop_masking_long) +{ + char const* sourceCode = R"( + contract c { + bytes data; + function test() public returns (bytes) { + for (uint i = 0; i < 34; i++) + data.push(3); + data.pop(); + return data; + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("test()"), encodeArgs( + u256(0x20), + u256(33), + asString(fromHex("0303030303030303030303030303030303030303030303030303030303030303")), + asString(fromHex("03")) + )); +} + +BOOST_AUTO_TEST_CASE(byte_array_pop_copy_long) +{ + char const* sourceCode = R"( + contract c { + bytes data; + function test() public returns (bytes) { + for (uint i = 0; i < 33; i++) + data.push(3); + for (uint j = 0; j < 4; j++) + data.pop(); + return data; + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("test()"), encodeArgs( + u256(0x20), + u256(29), + asString(fromHex("0303030303030303030303030303030303030303030303030303030303")) + )); +} + BOOST_AUTO_TEST_CASE(external_array_args) { char const* sourceCode = R"( From 98d52beba3f989b3a5eeaba2d257de8de5df60a7 Mon Sep 17 00:00:00 2001 From: Erik Kundt Date: Wed, 16 May 2018 17:18:30 +0200 Subject: [PATCH 4/5] Adds syntax tests, documentation and changelog entry. Refines comment for array utility function. --- Changelog.md | 2 ++ docs/types.rst | 4 +++- libsolidity/codegen/ArrayUtils.cpp | 2 +- libsolidity/codegen/ArrayUtils.h | 2 +- test/libsolidity/syntaxTests/array/array_pop.sol | 7 +++++++ test/libsolidity/syntaxTests/array/array_pop_arg.sol | 8 ++++++++ test/libsolidity/syntaxTests/array/bytes_pop.sol | 7 +++++++ .../syntaxTests/array/dynamic_memory_array_pop.sol | 8 ++++++++ ...array_length_cannot_be_constant_function_parameter.sol | 0 .../length}/can_be_constant_in_function.sol | 0 .../length}/can_be_constant_in_struct.sol | 0 .../length}/can_be_recursive_constant.sol | 0 .../{arrayLength => array/length}/cannot_be_function.sol | 0 .../length}/cannot_be_function_call.sol | 0 .../length}/complex_cyclic_constant.sol | 0 .../length}/const_cannot_be_fractional.sol | 0 .../{arrayLength => array/length}/constant_var.sol | 0 .../{arrayLength => array/length}/cyclic_constant.sol | 0 .../{arrayLength => array/length}/inline_array.sol | 0 .../length}/invalid_expression_1.sol | 0 .../length}/invalid_expression_2.sol | 0 .../length}/invalid_expression_3.sol | 0 .../length}/invalid_expression_4.sol | 0 .../length}/invalid_expression_5.sol | 0 .../length}/non_integer_constant_var.sol | 0 .../length}/not_convertible_to_integer.sol | 0 .../{arrayLength => array/length}/parentheses.sol | 0 .../{arrayLength => array/length}/pure_functions.sol | 0 .../{arrayLength => array/length}/too_large.sol | 0 .../syntaxTests/{arrayLength => array/length}/tuples.sol | 0 test/libsolidity/syntaxTests/array/no_array_pop.sol | 8 ++++++++ .../syntaxTests/array/static_storage_array_pop.sol | 8 ++++++++ test/libsolidity/syntaxTests/array/string_pop.sol | 8 ++++++++ 33 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 test/libsolidity/syntaxTests/array/array_pop.sol create mode 100644 test/libsolidity/syntaxTests/array/array_pop_arg.sol create mode 100644 test/libsolidity/syntaxTests/array/bytes_pop.sol create mode 100644 test/libsolidity/syntaxTests/array/dynamic_memory_array_pop.sol rename test/libsolidity/syntaxTests/{arrayLength => array/length}/array_length_cannot_be_constant_function_parameter.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/can_be_constant_in_function.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/can_be_constant_in_struct.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/can_be_recursive_constant.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/cannot_be_function.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/cannot_be_function_call.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/complex_cyclic_constant.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/const_cannot_be_fractional.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/constant_var.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/cyclic_constant.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/inline_array.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/invalid_expression_1.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/invalid_expression_2.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/invalid_expression_3.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/invalid_expression_4.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/invalid_expression_5.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/non_integer_constant_var.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/not_convertible_to_integer.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/parentheses.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/pure_functions.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/too_large.sol (100%) rename test/libsolidity/syntaxTests/{arrayLength => array/length}/tuples.sol (100%) create mode 100644 test/libsolidity/syntaxTests/array/no_array_pop.sol create mode 100644 test/libsolidity/syntaxTests/array/static_storage_array_pop.sol create mode 100644 test/libsolidity/syntaxTests/array/string_pop.sol diff --git a/Changelog.md b/Changelog.md index c31aced2a..a1248868f 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,7 @@ ### 0.5.0 (unreleased) +Language Features: + * General: Support ``pop()`` for storage arrays. Breaking Changes: * Disallow conversions between bytesX and uintY of different size. diff --git a/docs/types.rst b/docs/types.rst index 794a70de0..b3631f747 100644 --- a/docs/types.rst +++ b/docs/types.rst @@ -682,7 +682,7 @@ possible: It is planned to remove this restriction in the future but currently creates some complications because of how arrays are passed in the ABI. -.. index:: ! array;length, length, push, !array;push +.. index:: ! array;length, length, push, pop, !array;push, !array;pop Members ^^^^^^^ @@ -693,6 +693,8 @@ Members ``.length`` member. This does not happen automatically when attempting to access elements outside the current length. The size of memory arrays is fixed (but dynamic, i.e. it can depend on runtime parameters) once they are created. **push**: Dynamic storage arrays and ``bytes`` (not ``string``) have a member function called ``push`` that can be used to append an element at the end of the array. The function returns the new length. +**pop**: + Dynamic storage arrays and ``bytes`` (not ``string``) have a member function called ``pop`` that can be used to remove an element from the end of the array. .. warning:: It is not yet possible to use arrays of arrays in external functions. diff --git a/libsolidity/codegen/ArrayUtils.cpp b/libsolidity/codegen/ArrayUtils.cpp index d78e64a92..6bb9a9612 100644 --- a/libsolidity/codegen/ArrayUtils.cpp +++ b/libsolidity/codegen/ArrayUtils.cpp @@ -840,7 +840,7 @@ void ArrayUtils::popStorageArrayElement(ArrayType const& _type) const let length := and(div(slot_value, 2), 0x1f) if iszero(length) { invalid() } - // Zero-out the suffix inlcluding the least significant byte. + // Zero-out the suffix including the least significant byte. let mask := sub(exp(0x100, sub(33, length)), 1) length := sub(length, 1) slot_value := or(and(not(mask), slot_value), mul(length, 2)) diff --git a/libsolidity/codegen/ArrayUtils.h b/libsolidity/codegen/ArrayUtils.h index 84d591d7e..daf50bf52 100644 --- a/libsolidity/codegen/ArrayUtils.h +++ b/libsolidity/codegen/ArrayUtils.h @@ -73,7 +73,7 @@ public: /// Stack pre: reference (excludes byte offset) /// Stack post: new_length void incrementDynamicArraySize(ArrayType const& _type) const; - /// Decrements the size of a dynamic array by one if length is nonzero. Returns otherwise. + /// Decrements the size of a dynamic array by one if length is nonzero. Causes an invalid instruction otherwise. /// Clears the removed data element. In case of a byte array, this might move the data. /// Stack pre: reference /// Stack post: diff --git a/test/libsolidity/syntaxTests/array/array_pop.sol b/test/libsolidity/syntaxTests/array/array_pop.sol new file mode 100644 index 000000000..3804f911f --- /dev/null +++ b/test/libsolidity/syntaxTests/array/array_pop.sol @@ -0,0 +1,7 @@ +contract C { + uint[] data; + function test() public { + data.pop(); + } +} +// ---- diff --git a/test/libsolidity/syntaxTests/array/array_pop_arg.sol b/test/libsolidity/syntaxTests/array/array_pop_arg.sol new file mode 100644 index 000000000..bb7803e28 --- /dev/null +++ b/test/libsolidity/syntaxTests/array/array_pop_arg.sol @@ -0,0 +1,8 @@ +contract C { + uint[] data; + function test() public { + data.pop(5); + } +} +// ---- +// TypeError: (65-76): Wrong argument count for function call: 1 arguments given but expected 0. diff --git a/test/libsolidity/syntaxTests/array/bytes_pop.sol b/test/libsolidity/syntaxTests/array/bytes_pop.sol new file mode 100644 index 000000000..cd5aa0eb6 --- /dev/null +++ b/test/libsolidity/syntaxTests/array/bytes_pop.sol @@ -0,0 +1,7 @@ +contract C { + bytes data; + function test() public { + data.pop(); + } +} +// ---- diff --git a/test/libsolidity/syntaxTests/array/dynamic_memory_array_pop.sol b/test/libsolidity/syntaxTests/array/dynamic_memory_array_pop.sol new file mode 100644 index 000000000..5a79afc93 --- /dev/null +++ b/test/libsolidity/syntaxTests/array/dynamic_memory_array_pop.sol @@ -0,0 +1,8 @@ +contract C { + function test() public { + uint[] memory data; + data.pop(); + } +} +// ---- +// TypeError: (74-82): Member "pop" is not available in uint256[] memory outside of storage. diff --git a/test/libsolidity/syntaxTests/arrayLength/array_length_cannot_be_constant_function_parameter.sol b/test/libsolidity/syntaxTests/array/length/array_length_cannot_be_constant_function_parameter.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/array_length_cannot_be_constant_function_parameter.sol rename to test/libsolidity/syntaxTests/array/length/array_length_cannot_be_constant_function_parameter.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/can_be_constant_in_function.sol b/test/libsolidity/syntaxTests/array/length/can_be_constant_in_function.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/can_be_constant_in_function.sol rename to test/libsolidity/syntaxTests/array/length/can_be_constant_in_function.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/can_be_constant_in_struct.sol b/test/libsolidity/syntaxTests/array/length/can_be_constant_in_struct.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/can_be_constant_in_struct.sol rename to test/libsolidity/syntaxTests/array/length/can_be_constant_in_struct.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/can_be_recursive_constant.sol b/test/libsolidity/syntaxTests/array/length/can_be_recursive_constant.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/can_be_recursive_constant.sol rename to test/libsolidity/syntaxTests/array/length/can_be_recursive_constant.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/cannot_be_function.sol b/test/libsolidity/syntaxTests/array/length/cannot_be_function.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/cannot_be_function.sol rename to test/libsolidity/syntaxTests/array/length/cannot_be_function.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/cannot_be_function_call.sol b/test/libsolidity/syntaxTests/array/length/cannot_be_function_call.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/cannot_be_function_call.sol rename to test/libsolidity/syntaxTests/array/length/cannot_be_function_call.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/complex_cyclic_constant.sol b/test/libsolidity/syntaxTests/array/length/complex_cyclic_constant.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/complex_cyclic_constant.sol rename to test/libsolidity/syntaxTests/array/length/complex_cyclic_constant.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/const_cannot_be_fractional.sol b/test/libsolidity/syntaxTests/array/length/const_cannot_be_fractional.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/const_cannot_be_fractional.sol rename to test/libsolidity/syntaxTests/array/length/const_cannot_be_fractional.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/constant_var.sol b/test/libsolidity/syntaxTests/array/length/constant_var.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/constant_var.sol rename to test/libsolidity/syntaxTests/array/length/constant_var.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/cyclic_constant.sol b/test/libsolidity/syntaxTests/array/length/cyclic_constant.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/cyclic_constant.sol rename to test/libsolidity/syntaxTests/array/length/cyclic_constant.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/inline_array.sol b/test/libsolidity/syntaxTests/array/length/inline_array.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/inline_array.sol rename to test/libsolidity/syntaxTests/array/length/inline_array.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/invalid_expression_1.sol b/test/libsolidity/syntaxTests/array/length/invalid_expression_1.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/invalid_expression_1.sol rename to test/libsolidity/syntaxTests/array/length/invalid_expression_1.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/invalid_expression_2.sol b/test/libsolidity/syntaxTests/array/length/invalid_expression_2.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/invalid_expression_2.sol rename to test/libsolidity/syntaxTests/array/length/invalid_expression_2.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/invalid_expression_3.sol b/test/libsolidity/syntaxTests/array/length/invalid_expression_3.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/invalid_expression_3.sol rename to test/libsolidity/syntaxTests/array/length/invalid_expression_3.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/invalid_expression_4.sol b/test/libsolidity/syntaxTests/array/length/invalid_expression_4.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/invalid_expression_4.sol rename to test/libsolidity/syntaxTests/array/length/invalid_expression_4.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/invalid_expression_5.sol b/test/libsolidity/syntaxTests/array/length/invalid_expression_5.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/invalid_expression_5.sol rename to test/libsolidity/syntaxTests/array/length/invalid_expression_5.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/non_integer_constant_var.sol b/test/libsolidity/syntaxTests/array/length/non_integer_constant_var.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/non_integer_constant_var.sol rename to test/libsolidity/syntaxTests/array/length/non_integer_constant_var.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/not_convertible_to_integer.sol b/test/libsolidity/syntaxTests/array/length/not_convertible_to_integer.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/not_convertible_to_integer.sol rename to test/libsolidity/syntaxTests/array/length/not_convertible_to_integer.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/parentheses.sol b/test/libsolidity/syntaxTests/array/length/parentheses.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/parentheses.sol rename to test/libsolidity/syntaxTests/array/length/parentheses.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/pure_functions.sol b/test/libsolidity/syntaxTests/array/length/pure_functions.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/pure_functions.sol rename to test/libsolidity/syntaxTests/array/length/pure_functions.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/too_large.sol b/test/libsolidity/syntaxTests/array/length/too_large.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/too_large.sol rename to test/libsolidity/syntaxTests/array/length/too_large.sol diff --git a/test/libsolidity/syntaxTests/arrayLength/tuples.sol b/test/libsolidity/syntaxTests/array/length/tuples.sol similarity index 100% rename from test/libsolidity/syntaxTests/arrayLength/tuples.sol rename to test/libsolidity/syntaxTests/array/length/tuples.sol diff --git a/test/libsolidity/syntaxTests/array/no_array_pop.sol b/test/libsolidity/syntaxTests/array/no_array_pop.sol new file mode 100644 index 000000000..44e54ad2f --- /dev/null +++ b/test/libsolidity/syntaxTests/array/no_array_pop.sol @@ -0,0 +1,8 @@ +contract C { + uint data; + function test() public { + data.pop(); + } +} +// ---- +// TypeError: (63-71): Member "pop" not found or not visible after argument-dependent lookup in uint256 diff --git a/test/libsolidity/syntaxTests/array/static_storage_array_pop.sol b/test/libsolidity/syntaxTests/array/static_storage_array_pop.sol new file mode 100644 index 000000000..0af171ad5 --- /dev/null +++ b/test/libsolidity/syntaxTests/array/static_storage_array_pop.sol @@ -0,0 +1,8 @@ +contract C { + uint[3] data; + function test() public { + data.pop(); + } +} +// ---- +// TypeError: (66-74): Member "pop" not found or not visible after argument-dependent lookup in uint256[3] storage ref diff --git a/test/libsolidity/syntaxTests/array/string_pop.sol b/test/libsolidity/syntaxTests/array/string_pop.sol new file mode 100644 index 000000000..2a46d0c3e --- /dev/null +++ b/test/libsolidity/syntaxTests/array/string_pop.sol @@ -0,0 +1,8 @@ +contract C { + string data; + function test() public { + data.pop(); + } +} +// ---- +// TypeError: (65-73): Member "pop" not found or not visible after argument-dependent lookup in string storage ref From fea0d116f7d95e9a39f0c80c5156cb3656b03ce0 Mon Sep 17 00:00:00 2001 From: Erik Kundt Date: Tue, 29 May 2018 11:25:13 +0200 Subject: [PATCH 5/5] Fixes assembly bug and adds tests to cover it. --- libsolidity/codegen/ArrayUtils.cpp | 9 ++- test/libsolidity/SolidityEndToEndTest.cpp | 70 +++++++++++++++-------- 2 files changed, 50 insertions(+), 29 deletions(-) diff --git a/libsolidity/codegen/ArrayUtils.cpp b/libsolidity/codegen/ArrayUtils.cpp index 6bb9a9612..14c887c3e 100644 --- a/libsolidity/codegen/ArrayUtils.cpp +++ b/libsolidity/codegen/ArrayUtils.cpp @@ -848,9 +848,9 @@ void ArrayUtils::popStorageArrayElement(ArrayType const& _type) const } case 1 { // long byte array + mstore(0, ref) let length := div(slot_value, 2) let slot := keccak256(0, 0x20) - mstore(0, ref) switch length case 32 { @@ -861,14 +861,13 @@ void ArrayUtils::popStorageArrayElement(ArrayType const& _type) const } default { - let slot_offset := div(sub(length, 1), 32) - let length_offset := and(sub(length, 1), 0x1f) - slot := add(slot, slot_offset) + let offset_inside_slot := and(sub(length, 1), 0x1f) + slot := add(slot, div(sub(length, 1), 32)) let data := sload(slot) // Zero-out the suffix of the byte array by masking it. // ((1<<(8 * (32 - offset))) - 1) - let mask := sub(exp(0x100, sub(32, length_offset)), 1) + let mask := sub(exp(0x100, sub(32, offset_inside_slot)), 1) data := and(not(mask), data) sstore(slot, data) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index b0ab15c8e..f1fac3963 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -5161,6 +5161,9 @@ BOOST_AUTO_TEST_CASE(array_pop_uint24_transition) { char const* sourceCode = R"( contract c { + uint256 a; + uint256 b; + uint256 c; uint24[] data; function test() public returns (uint24 x, uint24 y) { for (uint i = 1; i <= 30; i++) @@ -5185,6 +5188,9 @@ BOOST_AUTO_TEST_CASE(array_pop_array_transition) { char const* sourceCode = R"( contract c { + uint256 a; + uint256 b; + uint256 c; uint16[] inner = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]; uint16[][] data; function test() public returns (uint x, uint y, uint z) { @@ -5260,27 +5266,13 @@ BOOST_AUTO_TEST_CASE(byte_array_pop) ABI_CHECK(callContractFunction("test()"), encodeArgs(2, 1, 1)); } -BOOST_AUTO_TEST_CASE(byte_array_pop_long) -{ - char const* sourceCode = R"( - contract c { - bytes data; - function test() public returns (uint l) { - for (uint i = 0; i < 33; i++) - data.push(byte(i)); - data.pop(); - l = data.length; - } - } - )"; - compileAndRun(sourceCode); - ABI_CHECK(callContractFunction("test()"), encodeArgs(32)); -} - BOOST_AUTO_TEST_CASE(byte_array_pop_empty_exception) { char const* sourceCode = R"( contract c { + uint256 a; + uint256 b; + uint256 c; bytes data; function test() public returns (bool) { data.pop(); @@ -5312,22 +5304,52 @@ BOOST_AUTO_TEST_CASE(byte_array_pop_storage_empty) BOOST_CHECK(storageEmpty(m_contractAddress)); } -BOOST_AUTO_TEST_CASE(byte_array_pop_storage_empty_long) +BOOST_AUTO_TEST_CASE(byte_array_pop_long_storage_empty) { char const* sourceCode = R"( contract c { + uint256 a; + uint256 b; + uint256 c; bytes data; - function test() public returns (uint l) { - for (uint i = 0; i < 33; i++) - data.push(3); - for (uint j = 0; j < 33; j++) + function test() public returns (bool) { + for (uint8 i = 0; i <= 40; i++) + data.push(byte(i+1)); + for (int8 j = 40; j >= 0; j--) { + require(data[uint8(j)] == byte(j+1)); + require(data.length == uint8(j+1)); data.pop(); - l = data.length; + } + return true; } } )"; compileAndRun(sourceCode); - ABI_CHECK(callContractFunction("test()"), encodeArgs(0)); + ABI_CHECK(callContractFunction("test()"), encodeArgs(true)); + BOOST_CHECK(storageEmpty(m_contractAddress)); +} + +BOOST_AUTO_TEST_CASE(byte_array_pop_long_storage_empty_garbage_ref) +{ + char const* sourceCode = R"( + contract c { + uint256 a; + uint256 b; + bytes data; + function test() public { + for (uint8 i = 0; i <= 40; i++) + data.push(3); + for (uint8 j = 0; j <= 40; j++) { + assembly { + mstore(0, "garbage") + } + data.pop(); + } + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("test()"), encodeArgs()); BOOST_CHECK(storageEmpty(m_contractAddress)); }