From a27b063c10be48f303b499a90bf988c3ce5bc4ee Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Thu, 19 May 2022 20:50:41 +0200 Subject: [PATCH 1/6] Test for buggy behaviour. --- .../byte_array_to_storage_cleanup.sol | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 test/libsolidity/semanticTests/byte_array_to_storage_cleanup.sol diff --git a/test/libsolidity/semanticTests/byte_array_to_storage_cleanup.sol b/test/libsolidity/semanticTests/byte_array_to_storage_cleanup.sol new file mode 100644 index 000000000..ed9b234ff --- /dev/null +++ b/test/libsolidity/semanticTests/byte_array_to_storage_cleanup.sol @@ -0,0 +1,36 @@ +contract C { + event ev0(uint[] i0, uint); + bytes public s2; + function h() external returns (bytes memory) { + uint[] memory x = new uint[](2); + emit ev0(x, 0x21); + bytes memory m = new bytes(63); + s2 = m; + s2.push(); + return s2; + } + function g() external returns (bytes memory) { + bytes memory m = new bytes(63); + assembly { + mstore8(add(m, add(32, 63)), 0x42) + } + s2 = m; + s2.push(); + return s2; + } + function f(bytes calldata c) external returns (bytes memory) { + s2 = c; + s2.push(); + return s2; + } +} +// ==== +// compileViaYul: false +// ---- +// constructor() -> +// gas legacy: 616124 +// gas legacyOptimized: 450817 +// h() -> 0x20, 0x40, 0x00, 2 +// ~ emit ev0(uint256[],uint256): 0x40, 0x21, 0x02, 0x00, 0x00 +// g() -> 0x20, 0x40, 0, 0x42 +// f(bytes): 0x20, 33, 0, -1 -> 0x20, 0x22, 0, 0xffff000000000000000000000000000000000000000000000000000000000000 From 7a84e9c875f4abfb201bb35c956aa87550f08bb2 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Thu, 19 May 2022 20:50:50 +0200 Subject: [PATCH 2/6] Bugfix. --- libsolidity/codegen/ArrayUtils.cpp | 91 ++++++++----------- .../byte_array_to_storage_cleanup.sol | 13 +-- 2 files changed, 46 insertions(+), 58 deletions(-) diff --git a/libsolidity/codegen/ArrayUtils.cpp b/libsolidity/codegen/ArrayUtils.cpp index 725727869..9b95463ba 100644 --- a/libsolidity/codegen/ArrayUtils.cpp +++ b/libsolidity/codegen/ArrayUtils.cpp @@ -50,9 +50,8 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons // stack layout: [source_ref] [source length] target_ref (top) solAssert(_targetType.location() == DataLocation::Storage, ""); - Type const* uint256 = TypeProvider::uint256(); - Type const* targetBaseType = _targetType.isByteArrayOrString() ? uint256 : _targetType.baseType(); - Type const* sourceBaseType = _sourceType.isByteArrayOrString() ? uint256 : _sourceType.baseType(); + Type const* targetBaseType = _targetType.baseType(); + Type const* sourceBaseType = _sourceType.baseType(); // TODO unroll loop for small sizes @@ -68,7 +67,38 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons for (unsigned i = _sourceType.sizeOnStack(); i > 0; --i) m_context << swapInstruction(i); // stack: target_ref source_ref [source_length] - // stack: target_ref source_ref [source_length] + + if (_targetType.isByteArrayOrString()) + { + // stack: target_ref source_ref [source_length] + if (fromCalldata && _sourceType.isDynamicallySized()) + { + // stack: target_ref source_ref source_length + m_context << Instruction::SWAP1; + // stack: target_ref source_length source_ref + m_context << Instruction::DUP3; + // stack: target_ref source_length source_ref target_ref + m_context.callYulFunction( + m_context.utilFunctions().copyByteArrayToStorageFunction(_sourceType, _targetType), + 3, + 0 + ); + } + else + { + // stack: target_ref source_ref + m_context << Instruction::DUP2; + // stack: target_ref source_ref target_ref + m_context.callYulFunction( + m_context.utilFunctions().copyByteArrayToStorageFunction(_sourceType, _targetType), + 2, + 0 + ); + } + // stack: target_ref + return; + } + // retrieve source length if (_sourceType.location() != DataLocation::CallData || !_sourceType.isDynamicallySized()) retrieveLength(_sourceType); // otherwise, length is already there @@ -97,10 +127,11 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons utils.retrieveLength(_targetType); // stack: target_ref source_ref source_length target_ref target_length if (_targetType.isDynamicallySized()) + { // store new target length - if (!_targetType.isByteArrayOrString()) - // Otherwise, length will be stored below. - _context << Instruction::DUP3 << Instruction::DUP3 << Instruction::SSTORE; + solAssert(!_targetType.isByteArrayOrString()); + _context << Instruction::DUP3 << Instruction::DUP3 << Instruction::SSTORE; + } if (sourceBaseType->category() == Type::Category::Mapping) { solAssert(targetBaseType->category() == Type::Category::Mapping, ""); @@ -125,51 +156,7 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons // stack: target_ref target_data_end source_length target_data_pos source_ref evmasm::AssemblyItem copyLoopEndWithoutByteOffset = _context.newTag(); - - // special case for short byte arrays: Store them together with their length. - if (_targetType.isByteArrayOrString()) - { - // stack: target_ref target_data_end source_length target_data_pos source_ref - _context << Instruction::DUP3; - evmasm::AssemblyItem nonEmptyByteArray = _context.appendConditionalJump(); - // Empty source, just zero out the main slot. - _context << u256(0) << Instruction::DUP6 << Instruction::SSTORE; - _context.appendJumpTo(copyLoopEndWithoutByteOffset); - - _context << nonEmptyByteArray; - // Non-empty source. - // stack: target_ref target_data_end source_length target_data_pos source_ref - _context << Instruction::DUP3 << u256(31) << Instruction::LT; - evmasm::AssemblyItem longByteArray = _context.appendConditionalJump(); - // store the short byte array - solAssert(_sourceType.isByteArrayOrString(), ""); - if (_sourceType.location() == DataLocation::Storage) - { - // just copy the slot, it contains length and data - _context << Instruction::DUP1 << Instruction::SLOAD; - _context << Instruction::DUP6 << Instruction::SSTORE; - } - else - { - _context << Instruction::DUP1; - CompilerUtils(_context).loadFromMemoryDynamic(*sourceBaseType, fromCalldata, true, false); - // stack: target_ref target_data_end source_length target_data_pos source_ref value - // clear the lower-order byte - which will hold the length - _context << u256(0xff) << Instruction::NOT << Instruction::AND; - // fetch the length and shift it left by one - _context << Instruction::DUP4 << Instruction::DUP1 << Instruction::ADD; - // combine value and length and store them - _context << Instruction::OR << Instruction::DUP6 << Instruction::SSTORE; - } - // end of special case, jump right into cleaning target data area - _context.appendJumpTo(copyLoopEndWithoutByteOffset); - _context << longByteArray; - // Store length (2*length+1) - _context << Instruction::DUP3 << Instruction::DUP1 << Instruction::ADD; - _context << u256(1) << Instruction::ADD; - _context << Instruction::DUP6 << Instruction::SSTORE; - } - + solAssert(!_targetType.isByteArrayOrString()); // skip copying if source length is zero _context << Instruction::DUP3 << Instruction::ISZERO; _context.appendConditionalJumpTo(copyLoopEndWithoutByteOffset); diff --git a/test/libsolidity/semanticTests/byte_array_to_storage_cleanup.sol b/test/libsolidity/semanticTests/byte_array_to_storage_cleanup.sol index ed9b234ff..f4e5f62e2 100644 --- a/test/libsolidity/semanticTests/byte_array_to_storage_cleanup.sol +++ b/test/libsolidity/semanticTests/byte_array_to_storage_cleanup.sol @@ -25,12 +25,13 @@ contract C { } } // ==== -// compileViaYul: false +// compileViaYul: also // ---- // constructor() -> -// gas legacy: 616124 -// gas legacyOptimized: 450817 -// h() -> 0x20, 0x40, 0x00, 2 +// gas irOptimized: 520903 +// gas legacy: 731840 +// gas legacyOptimized: 494859 +// h() -> 0x20, 0x40, 0x00, 0 // ~ emit ev0(uint256[],uint256): 0x40, 0x21, 0x02, 0x00, 0x00 -// g() -> 0x20, 0x40, 0, 0x42 -// f(bytes): 0x20, 33, 0, -1 -> 0x20, 0x22, 0, 0xffff000000000000000000000000000000000000000000000000000000000000 +// g() -> 0x20, 0x40, 0, 0x00 +// f(bytes): 0x20, 33, 0, -1 -> 0x20, 0x22, 0, 0xff00000000000000000000000000000000000000000000000000000000000000 From 5989f45e9eba2dbea26499b1ef670d70d9311d38 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Tue, 7 Jun 2022 14:45:10 +0200 Subject: [PATCH 3/6] Test updates. --- .../abiEncoderV1/abi_decode_v2_storage.sol | 4 ++-- .../abi_decode_simple_storage.sol | 4 ++-- .../array/bytes_length_member.sol | 4 ++-- .../array/copying/bytes_inside_mappings.sol | 8 ++++---- .../copying/bytes_storage_to_storage.sol | 20 +++++++++---------- .../copy_byte_array_in_struct_to_storage.sol | 8 ++++---- .../copying/copy_byte_array_to_storage.sol | 4 ++-- .../array/copying/copy_removes_bytes_data.sol | 4 ++-- .../copying/copying_bytes_multiassign.sol | 4 ++-- .../dirty_memory_bytes_to_storage_copy.sol | 2 +- .../memory_dyn_2d_bytes_to_storage.sol | 4 ++-- .../copying/storage_memory_nested_bytes.sol | 4 ++-- .../array/delete/bytes_delete_element.sol | 4 ++-- .../array/push/nested_bytes_push.sol | 4 ++-- .../byte_array_to_storage_cleanup.sol | 2 +- .../copy_from_calldata_removes_bytes_data.sol | 4 ++-- .../bytes_in_constructors_packer.sol | 4 ++-- .../bytes_in_constructors_unpacker.sol | 4 ++-- ...truct_containing_bytes_copy_and_delete.sol | 4 ++-- .../semanticTests/various/address_code.sol | 4 ++-- .../various/destructuring_assignment.sol | 4 ++-- .../skip_dynamic_types_for_structs.sol | 4 ++-- 22 files changed, 54 insertions(+), 54 deletions(-) diff --git a/test/libsolidity/semanticTests/abiEncoderV1/abi_decode_v2_storage.sol b/test/libsolidity/semanticTests/abiEncoderV1/abi_decode_v2_storage.sol index a2b855b85..fd7bc2083 100644 --- a/test/libsolidity/semanticTests/abiEncoderV1/abi_decode_v2_storage.sol +++ b/test/libsolidity/semanticTests/abiEncoderV1/abi_decode_v2_storage.sol @@ -23,5 +23,5 @@ contract C { // ---- // f() -> 0x20, 0x8, 0x40, 0x3, 0x9, 0xa, 0xb // gas irOptimized: 203172 -// gas legacy: 206075 -// gas legacyOptimized: 203059 +// gas legacy: 206343 +// gas legacyOptimized: 203162 diff --git a/test/libsolidity/semanticTests/abiencodedecode/abi_decode_simple_storage.sol b/test/libsolidity/semanticTests/abiencodedecode/abi_decode_simple_storage.sol index 524394ff8..e877e627b 100644 --- a/test/libsolidity/semanticTests/abiencodedecode/abi_decode_simple_storage.sol +++ b/test/libsolidity/semanticTests/abiencodedecode/abi_decode_simple_storage.sol @@ -10,5 +10,5 @@ contract C { // ---- // f(bytes): 0x20, 0x80, 0x21, 0x40, 0x7, "abcdefg" -> 0x21, 0x40, 0x7, "abcdefg" // gas irOptimized: 135787 -// gas legacy: 137181 -// gas legacyOptimized: 136073 +// gas legacy: 137377 +// gas legacyOptimized: 136125 diff --git a/test/libsolidity/semanticTests/array/bytes_length_member.sol b/test/libsolidity/semanticTests/array/bytes_length_member.sol index 8e7b526cf..387ef0930 100644 --- a/test/libsolidity/semanticTests/array/bytes_length_member.sol +++ b/test/libsolidity/semanticTests/array/bytes_length_member.sol @@ -14,6 +14,6 @@ contract c { // getLength() -> 0 // set(): 1, 2 -> true // gas irOptimized: 110402 -// gas legacy: 110723 -// gas legacyOptimized: 110564 +// gas legacy: 110968 +// gas legacyOptimized: 110585 // getLength() -> 68 diff --git a/test/libsolidity/semanticTests/array/copying/bytes_inside_mappings.sol b/test/libsolidity/semanticTests/array/copying/bytes_inside_mappings.sol index 7358e1506..99ee19d7a 100644 --- a/test/libsolidity/semanticTests/array/copying/bytes_inside_mappings.sol +++ b/test/libsolidity/semanticTests/array/copying/bytes_inside_mappings.sol @@ -6,12 +6,12 @@ contract c { // ---- // set(uint256): 1, 2 -> true // gas irOptimized: 110576 -// gas legacy: 111088 -// gas legacyOptimized: 110733 +// gas legacy: 111333 +// gas legacyOptimized: 110750 // set(uint256): 2, 2, 3, 4, 5 -> true // gas irOptimized: 177527 -// gas legacy: 178018 -// gas legacyOptimized: 177663 +// gas legacy: 178335 +// gas legacyOptimized: 177725 // storageEmpty -> 0 // copy(uint256,uint256): 1, 2 -> true // storageEmpty -> 0 diff --git a/test/libsolidity/semanticTests/array/copying/bytes_storage_to_storage.sol b/test/libsolidity/semanticTests/array/copying/bytes_storage_to_storage.sol index 9e1063097..0f0b4083f 100644 --- a/test/libsolidity/semanticTests/array/copying/bytes_storage_to_storage.sol +++ b/test/libsolidity/semanticTests/array/copying/bytes_storage_to_storage.sol @@ -18,24 +18,24 @@ contract c { // f(uint256): 0 -> 0x20, 0x00 // f(uint256): 31 -> 0x20, 0x1f, 0x0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e00 // gas irOptimized: 108395 -// gas legacy: 123884 -// gas legacyOptimized: 119139 +// gas legacy: 124322 +// gas legacyOptimized: 119141 // f(uint256): 32 -> 0x20, 0x20, 1780731860627700044960722568376592200742329637303199754547598369979440671 // gas irOptimized: 117480 -// gas legacy: 134936 -// gas legacyOptimized: 130046 +// gas legacy: 135259 +// gas legacyOptimized: 130089 // f(uint256): 33 -> 0x20, 33, 1780731860627700044960722568376592200742329637303199754547598369979440671, 0x2000000000000000000000000000000000000000000000000000000000000000 // gas irOptimized: 124117 -// gas legacy: 141728 -// gas legacyOptimized: 136711 +// gas legacy: 142861 +// gas legacyOptimized: 137030 // f(uint256): 63 -> 0x20, 0x3f, 1780731860627700044960722568376592200742329637303199754547598369979440671, 14532552714582660066924456880521368950258152170031413196862950297402215316992 // gas irOptimized: 126467 -// gas legacy: 159768 -// gas legacyOptimized: 150641 +// gas legacy: 160901 +// gas legacyOptimized: 150960 // f(uint256): 12 -> 0x20, 0x0c, 0x0102030405060708090a0b0000000000000000000000000000000000000000 // gas legacy: 59345 // gas legacyOptimized: 57279 // f(uint256): 129 -> 0x20, 0x81, 1780731860627700044960722568376592200742329637303199754547598369979440671, 0x202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f, 29063324697304692433803953038474361308315562010425523193971352996434451193439, 0x606162636465666768696a6b6c6d6e6f707172737475767778797a7b7c7d7e7f, -57896044618658097711785492504343953926634992332820282019728792003956564819968 // gas irOptimized: 353326 -// gas legacy: 421067 -// gas legacyOptimized: 402910 +// gas legacy: 421700 +// gas legacyOptimized: 402999 diff --git a/test/libsolidity/semanticTests/array/copying/copy_byte_array_in_struct_to_storage.sol b/test/libsolidity/semanticTests/array/copying/copy_byte_array_in_struct_to_storage.sol index 308e756ae..2a3e2044f 100644 --- a/test/libsolidity/semanticTests/array/copying/copy_byte_array_in_struct_to_storage.sol +++ b/test/libsolidity/semanticTests/array/copying/copy_byte_array_in_struct_to_storage.sol @@ -36,11 +36,11 @@ contract C { // ---- // f() -> 0x40, 0x80, 6, 0x6162636465660000000000000000000000000000000000000000000000000000, 0x49, 0x3132333435363738393031323334353637383930313233343536373839303120, 0x3132333435363738393031323334353637383930313233343536373839303120, 0x3132333435363738390000000000000000000000000000000000000000000000 // gas irOptimized: 179880 -// gas legacy: 180676 -// gas legacyOptimized: 180070 +// gas legacy: 181099 +// gas legacyOptimized: 180073 // g() -> 0x40, 0xc0, 0x49, 0x3132333435363738393031323334353637383930313233343536373839303120, 0x3132333435363738393031323334353637383930313233343536373839303120, 0x3132333435363738390000000000000000000000000000000000000000000000, 0x11, 0x3132333435363738393233343536373839000000000000000000000000000000 // gas irOptimized: 107211 -// gas legacy: 107877 -// gas legacyOptimized: 107236 +// gas legacy: 110253 +// gas legacyOptimized: 107397 // h() -> 0x40, 0x60, 0x00, 0x00 // storageEmpty -> 1 diff --git a/test/libsolidity/semanticTests/array/copying/copy_byte_array_to_storage.sol b/test/libsolidity/semanticTests/array/copying/copy_byte_array_to_storage.sol index 33a70fa36..92f2a1f2e 100644 --- a/test/libsolidity/semanticTests/array/copying/copy_byte_array_to_storage.sol +++ b/test/libsolidity/semanticTests/array/copying/copy_byte_array_to_storage.sol @@ -47,5 +47,5 @@ contract C { // ---- // f() -> 0xff // gas irOptimized: 119584 -// gas legacy: 128005 -// gas legacyOptimized: 123446 +// gas legacy: 132274 +// gas legacyOptimized: 123756 diff --git a/test/libsolidity/semanticTests/array/copying/copy_removes_bytes_data.sol b/test/libsolidity/semanticTests/array/copying/copy_removes_bytes_data.sol index c111c7b89..1c7266611 100644 --- a/test/libsolidity/semanticTests/array/copying/copy_removes_bytes_data.sol +++ b/test/libsolidity/semanticTests/array/copying/copy_removes_bytes_data.sol @@ -8,8 +8,8 @@ contract c { // ---- // set(): 1, 2, 3, 4, 5 -> true // gas irOptimized: 177386 -// gas legacy: 177653 -// gas legacyOptimized: 177493 +// gas legacy: 177970 +// gas legacyOptimized: 177559 // storageEmpty -> 0 // reset() -> true // storageEmpty -> 1 diff --git a/test/libsolidity/semanticTests/array/copying/copying_bytes_multiassign.sol b/test/libsolidity/semanticTests/array/copying/copying_bytes_multiassign.sol index 9b308a010..cc61509d4 100644 --- a/test/libsolidity/semanticTests/array/copying/copying_bytes_multiassign.sol +++ b/test/libsolidity/semanticTests/array/copying/copying_bytes_multiassign.sol @@ -21,8 +21,8 @@ contract sender { // ---- // (): 7 -> // gas irOptimized: 110826 -// gas legacy: 111071 -// gas legacyOptimized: 111016 +// gas legacy: 111408 +// gas legacyOptimized: 111078 // val() -> 0 // forward(bool): true -> true // val() -> 0x80 diff --git a/test/libsolidity/semanticTests/array/copying/dirty_memory_bytes_to_storage_copy.sol b/test/libsolidity/semanticTests/array/copying/dirty_memory_bytes_to_storage_copy.sol index 767e26856..ca154eee9 100644 --- a/test/libsolidity/semanticTests/array/copying/dirty_memory_bytes_to_storage_copy.sol +++ b/test/libsolidity/semanticTests/array/copying/dirty_memory_bytes_to_storage_copy.sol @@ -16,4 +16,4 @@ contract C { // ==== // compileViaYul: false // ---- -// f() -> 0x6465616462656566313564656164000000000000000000000000000000000010 +// f() -> 0x6465616462656566000000000000000000000000000000000000000000000010 diff --git a/test/libsolidity/semanticTests/array/copying/memory_dyn_2d_bytes_to_storage.sol b/test/libsolidity/semanticTests/array/copying/memory_dyn_2d_bytes_to_storage.sol index 38c20b8fc..16d10a36d 100644 --- a/test/libsolidity/semanticTests/array/copying/memory_dyn_2d_bytes_to_storage.sol +++ b/test/libsolidity/semanticTests/array/copying/memory_dyn_2d_bytes_to_storage.sol @@ -19,5 +19,5 @@ contract C { // ---- // f() -> 3 // gas irOptimized: 128172 -// gas legacy: 130181 -// gas legacyOptimized: 129198 +// gas legacy: 130757 +// gas legacyOptimized: 129144 diff --git a/test/libsolidity/semanticTests/array/copying/storage_memory_nested_bytes.sol b/test/libsolidity/semanticTests/array/copying/storage_memory_nested_bytes.sol index fe0cf9583..349bfd935 100644 --- a/test/libsolidity/semanticTests/array/copying/storage_memory_nested_bytes.sol +++ b/test/libsolidity/semanticTests/array/copying/storage_memory_nested_bytes.sol @@ -12,5 +12,5 @@ contract C { // ---- // f() -> 0x20, 0x02, 0x40, 0x80, 3, 0x6162630000000000000000000000000000000000000000000000000000000000, 0x99, 44048183304486788312148433451363384677562265908331949128489393215789685032262, 32241931068525137014058842823026578386641954854143559838526554899205067598957, 49951309422467613961193228765530489307475214998374779756599339590522149884499, 0x54555658595a6162636465666768696a6b6c6d6e6f707172737475767778797a, 0x4142434445464748494a4b4c4d4e4f5051525354555658595a00000000000000 // gas irOptimized: 202952 -// gas legacy: 204441 -// gas legacyOptimized: 203419 +// gas legacy: 204912 +// gas legacyOptimized: 203437 diff --git a/test/libsolidity/semanticTests/array/delete/bytes_delete_element.sol b/test/libsolidity/semanticTests/array/delete/bytes_delete_element.sol index b9e2aa839..652854cb8 100644 --- a/test/libsolidity/semanticTests/array/delete/bytes_delete_element.sol +++ b/test/libsolidity/semanticTests/array/delete/bytes_delete_element.sol @@ -17,5 +17,5 @@ contract c { // ---- // test1() -> true // gas irOptimized: 207928 -// gas legacy: 254650 -// gas legacyOptimized: 247384 +// gas legacy: 254905 +// gas legacyOptimized: 247415 diff --git a/test/libsolidity/semanticTests/array/push/nested_bytes_push.sol b/test/libsolidity/semanticTests/array/push/nested_bytes_push.sol index 6c006ffd6..8a962ad43 100644 --- a/test/libsolidity/semanticTests/array/push/nested_bytes_push.sol +++ b/test/libsolidity/semanticTests/array/push/nested_bytes_push.sol @@ -14,5 +14,5 @@ contract C { // ---- // f() -> // gas irOptimized: 179173 -// gas legacy: 180602 -// gas legacyOptimized: 180385 +// gas legacy: 181066 +// gas legacyOptimized: 180435 diff --git a/test/libsolidity/semanticTests/byte_array_to_storage_cleanup.sol b/test/libsolidity/semanticTests/byte_array_to_storage_cleanup.sol index f4e5f62e2..f43166e78 100644 --- a/test/libsolidity/semanticTests/byte_array_to_storage_cleanup.sol +++ b/test/libsolidity/semanticTests/byte_array_to_storage_cleanup.sol @@ -28,7 +28,7 @@ contract C { // compileViaYul: also // ---- // constructor() -> -// gas irOptimized: 520903 +// gas irOptimized: 521983 // gas legacy: 731840 // gas legacyOptimized: 494859 // h() -> 0x20, 0x40, 0x00, 0 diff --git a/test/libsolidity/semanticTests/calldata/copy_from_calldata_removes_bytes_data.sol b/test/libsolidity/semanticTests/calldata/copy_from_calldata_removes_bytes_data.sol index f567d752d..9fc432e8c 100644 --- a/test/libsolidity/semanticTests/calldata/copy_from_calldata_removes_bytes_data.sol +++ b/test/libsolidity/semanticTests/calldata/copy_from_calldata_removes_bytes_data.sol @@ -11,8 +11,8 @@ contract c { // ---- // (): 1, 2, 3, 4, 5 -> // gas irOptimized: 155158 -// gas legacy: 155249 -// gas legacyOptimized: 155212 +// gas legacy: 155483 +// gas legacyOptimized: 155303 // checkIfDataIsEmpty() -> false // sendMessage() -> true, 0x40, 0 // checkIfDataIsEmpty() -> true diff --git a/test/libsolidity/semanticTests/constructor/bytes_in_constructors_packer.sol b/test/libsolidity/semanticTests/constructor/bytes_in_constructors_packer.sol index b855e898c..5c57963b2 100644 --- a/test/libsolidity/semanticTests/constructor/bytes_in_constructors_packer.sol +++ b/test/libsolidity/semanticTests/constructor/bytes_in_constructors_packer.sol @@ -25,5 +25,5 @@ contract Creator { // ---- // f(uint256,bytes): 7, 0x40, 78, "abcdefghijklmnopqrstuvwxyzabcdef", "ghijklmnopqrstuvwxyzabcdefghijkl", "mnopqrstuvwxyz" -> 7, "h" // gas irOptimized: 282398 -// gas legacy: 428711 -// gas legacyOptimized: 297922 +// gas legacy: 429047 +// gas legacyOptimized: 297958 diff --git a/test/libsolidity/semanticTests/constructor/bytes_in_constructors_unpacker.sol b/test/libsolidity/semanticTests/constructor/bytes_in_constructors_unpacker.sol index a160fafcc..867a85a12 100644 --- a/test/libsolidity/semanticTests/constructor/bytes_in_constructors_unpacker.sol +++ b/test/libsolidity/semanticTests/constructor/bytes_in_constructors_unpacker.sol @@ -9,7 +9,7 @@ contract Test { // ---- // constructor(): 7, 0x40, 78, "abcdefghijklmnopqrstuvwxyzabcdef", "ghijklmnopqrstuvwxyzabcdefghijkl", "mnopqrstuvwxyz" -> // gas irOptimized: 273340 -// gas legacy: 309607 -// gas legacyOptimized: 260566 +// gas legacy: 317746 +// gas legacyOptimized: 262368 // m_x() -> 7 // m_s() -> 0x20, 78, "abcdefghijklmnopqrstuvwxyzabcdef", "ghijklmnopqrstuvwxyzabcdefghijkl", "mnopqrstuvwxyz" diff --git a/test/libsolidity/semanticTests/structs/struct_containing_bytes_copy_and_delete.sol b/test/libsolidity/semanticTests/structs/struct_containing_bytes_copy_and_delete.sol index 34806a35b..998007572 100644 --- a/test/libsolidity/semanticTests/structs/struct_containing_bytes_copy_and_delete.sol +++ b/test/libsolidity/semanticTests/structs/struct_containing_bytes_copy_and_delete.sol @@ -24,8 +24,8 @@ contract c { // storageEmpty -> 1 // set(uint256,bytes,uint256): 12, 0x60, 13, 33, "12345678901234567890123456789012", "3" -> true // gas irOptimized: 133599 -// gas legacy: 134433 -// gas legacyOptimized: 133876 +// gas legacy: 134654 +// gas legacyOptimized: 133882 // test(uint256): 32 -> "3" // storageEmpty -> 0 // copy() -> true diff --git a/test/libsolidity/semanticTests/various/address_code.sol b/test/libsolidity/semanticTests/various/address_code.sol index 73720fa5f..c44fd410a 100644 --- a/test/libsolidity/semanticTests/various/address_code.sol +++ b/test/libsolidity/semanticTests/various/address_code.sol @@ -17,8 +17,8 @@ contract C { // ---- // constructor() -> // gas irOptimized: 177507 -// gas legacy: 240889 -// gas legacyOptimized: 155314 +// gas legacy: 249207 +// gas legacyOptimized: 157489 // initCode() -> 0x20, 0 // f() -> true // g() -> 0 diff --git a/test/libsolidity/semanticTests/various/destructuring_assignment.sol b/test/libsolidity/semanticTests/various/destructuring_assignment.sol index 37c0d98ea..6be5ffd47 100644 --- a/test/libsolidity/semanticTests/various/destructuring_assignment.sol +++ b/test/libsolidity/semanticTests/various/destructuring_assignment.sol @@ -35,5 +35,5 @@ contract C { // ---- // f(bytes): 0x20, 0x5, "abcde" -> 0 // gas irOptimized: 239194 -// gas legacy: 240349 -// gas legacyOptimized: 239673 +// gas legacy: 240541 +// gas legacyOptimized: 239654 diff --git a/test/libsolidity/semanticTests/various/skip_dynamic_types_for_structs.sol b/test/libsolidity/semanticTests/various/skip_dynamic_types_for_structs.sol index 1cc1bb039..3e29dc270 100644 --- a/test/libsolidity/semanticTests/various/skip_dynamic_types_for_structs.sol +++ b/test/libsolidity/semanticTests/various/skip_dynamic_types_for_structs.sol @@ -21,5 +21,5 @@ contract C { // ---- // g() -> 2, 6 // gas irOptimized: 178677 -// gas legacy: 180753 -// gas legacyOptimized: 179472 +// gas legacy: 180945 +// gas legacyOptimized: 179460 From 91ff02b988e99f319ecce24eecb923d89d25327e Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Mon, 13 Jun 2022 12:35:21 +0200 Subject: [PATCH 4/6] Changelog and buglist entries. --- Changelog.md | 1 + docs/bugs.json | 10 ++++ docs/bugs_by_version.json | 98 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+) diff --git a/Changelog.md b/Changelog.md index ba091f266..f85404b9d 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,6 +1,7 @@ ### 0.8.15 (unreleased) Important Bugfixes: + * Code Generation: Avoid writing dirty bytes to storage when copying ``bytes`` arrays. * Yul Optimizer: Keep all memory side-effects of inline assembly blocks. diff --git a/docs/bugs.json b/docs/bugs.json index bc1bc622e..979f4264f 100644 --- a/docs/bugs.json +++ b/docs/bugs.json @@ -1,4 +1,14 @@ [ + { + "uid": "SOL-2022-5", + "name": "DirtyBytesArrayToStorage", + "summary": "Copying ``bytes`` arrays from memory or calldata to storage may result in dirty storage values.", + "description": "Copying ``bytes`` arrays from memory or calldata to storage is done in chunks of 32 bytes. Thereby, dirty values in calldata or memory can be written to storage, which may then become observable after a ``.push()`` on the bytes array in storage.", + "link": "https://blog.soliditylang.org/2022/06/15/dirty-bytes-array-to-storage-bug/", + "introduced": "0.0.1", + "fixed": "0.8.15", + "severity": "low" + }, { "uid": "SOL-2022-4", "name": "InlineAssemblyMemorySideEffects", diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json index 1b0f7d1ca..82eaa8141 100644 --- a/docs/bugs_by_version.json +++ b/docs/bugs_by_version.json @@ -1,6 +1,7 @@ { "0.1.0": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -22,6 +23,7 @@ }, "0.1.1": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -43,6 +45,7 @@ }, "0.1.2": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -64,6 +67,7 @@ }, "0.1.3": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -85,6 +89,7 @@ }, "0.1.4": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -107,6 +112,7 @@ }, "0.1.5": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -129,6 +135,7 @@ }, "0.1.6": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -153,6 +160,7 @@ }, "0.1.7": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -177,6 +185,7 @@ }, "0.2.0": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -202,6 +211,7 @@ }, "0.2.1": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -227,6 +237,7 @@ }, "0.2.2": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -252,6 +263,7 @@ }, "0.3.0": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -279,6 +291,7 @@ }, "0.3.1": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -305,6 +318,7 @@ }, "0.3.2": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -331,6 +345,7 @@ }, "0.3.3": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -356,6 +371,7 @@ }, "0.3.4": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -381,6 +397,7 @@ }, "0.3.5": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -406,6 +423,7 @@ }, "0.3.6": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -429,6 +447,7 @@ }, "0.4.0": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -452,6 +471,7 @@ }, "0.4.1": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -475,6 +495,7 @@ }, "0.4.10": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -497,6 +518,7 @@ }, "0.4.11": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -518,6 +540,7 @@ }, "0.4.12": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -538,6 +561,7 @@ }, "0.4.13": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -558,6 +582,7 @@ }, "0.4.14": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -577,6 +602,7 @@ }, "0.4.15": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -595,6 +621,7 @@ }, "0.4.16": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -616,6 +643,7 @@ }, "0.4.17": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -638,6 +666,7 @@ }, "0.4.18": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -659,6 +688,7 @@ }, "0.4.19": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -681,6 +711,7 @@ }, "0.4.2": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -703,6 +734,7 @@ }, "0.4.20": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -725,6 +757,7 @@ }, "0.4.21": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -747,6 +780,7 @@ }, "0.4.22": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -769,6 +803,7 @@ }, "0.4.23": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -790,6 +825,7 @@ }, "0.4.24": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -811,6 +847,7 @@ }, "0.4.25": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -830,6 +867,7 @@ }, "0.4.26": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -846,6 +884,7 @@ }, "0.4.3": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -867,6 +906,7 @@ }, "0.4.4": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -887,6 +927,7 @@ }, "0.4.5": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -910,6 +951,7 @@ }, "0.4.6": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -932,6 +974,7 @@ }, "0.4.7": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -954,6 +997,7 @@ }, "0.4.8": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -976,6 +1020,7 @@ }, "0.4.9": { "bugs": [ + "DirtyBytesArrayToStorage", "KeccakCaching", "EmptyByteArrayCopy", "DynamicArrayCleanup", @@ -998,6 +1043,7 @@ }, "0.5.0": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -1017,6 +1063,7 @@ }, "0.5.1": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -1036,6 +1083,7 @@ }, "0.5.10": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", @@ -1052,6 +1100,7 @@ }, "0.5.11": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", @@ -1067,6 +1116,7 @@ }, "0.5.12": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", @@ -1082,6 +1132,7 @@ }, "0.5.13": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", @@ -1097,6 +1148,7 @@ }, "0.5.14": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", @@ -1114,6 +1166,7 @@ }, "0.5.15": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", @@ -1130,6 +1183,7 @@ }, "0.5.16": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", @@ -1145,6 +1199,7 @@ }, "0.5.17": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", @@ -1159,6 +1214,7 @@ }, "0.5.2": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -1178,6 +1234,7 @@ }, "0.5.3": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -1197,6 +1254,7 @@ }, "0.5.4": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -1216,6 +1274,7 @@ }, "0.5.5": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -1237,6 +1296,7 @@ }, "0.5.6": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -1258,6 +1318,7 @@ }, "0.5.7": { "bugs": [ + "DirtyBytesArrayToStorage", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", "EmptyByteArrayCopy", @@ -1277,6 +1338,7 @@ }, "0.5.8": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", @@ -1296,6 +1358,7 @@ }, "0.5.9": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", @@ -1314,6 +1377,7 @@ }, "0.6.0": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", @@ -1330,6 +1394,7 @@ }, "0.6.1": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", @@ -1345,6 +1410,7 @@ }, "0.6.10": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", @@ -1357,6 +1423,7 @@ }, "0.6.11": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", @@ -1369,6 +1436,7 @@ }, "0.6.12": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", @@ -1381,6 +1449,7 @@ }, "0.6.2": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", @@ -1396,6 +1465,7 @@ }, "0.6.3": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", @@ -1411,6 +1481,7 @@ }, "0.6.4": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "ABIDecodeTwoDimensionalArrayMemory", "KeccakCaching", @@ -1426,6 +1497,7 @@ }, "0.6.5": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1441,6 +1513,7 @@ }, "0.6.6": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1455,6 +1528,7 @@ }, "0.6.7": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1469,6 +1543,7 @@ }, "0.6.8": { "bugs": [ + "DirtyBytesArrayToStorage", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1480,6 +1555,7 @@ }, "0.6.9": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", @@ -1493,6 +1569,7 @@ }, "0.7.0": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", @@ -1505,6 +1582,7 @@ }, "0.7.1": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", @@ -1518,6 +1596,7 @@ }, "0.7.2": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", @@ -1530,6 +1609,7 @@ }, "0.7.3": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", @@ -1541,6 +1621,7 @@ }, "0.7.4": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", @@ -1551,6 +1632,7 @@ }, "0.7.5": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", @@ -1561,6 +1643,7 @@ }, "0.7.6": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", @@ -1571,6 +1654,7 @@ }, "0.8.0": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", @@ -1581,6 +1665,7 @@ }, "0.8.1": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", @@ -1591,6 +1676,7 @@ }, "0.8.10": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation" ], @@ -1598,6 +1684,7 @@ }, "0.8.11": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "AbiEncodeCallLiteralAsFixedBytesBug" @@ -1606,6 +1693,7 @@ }, "0.8.12": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "AbiEncodeCallLiteralAsFixedBytesBug" @@ -1614,6 +1702,7 @@ }, "0.8.13": { "bugs": [ + "DirtyBytesArrayToStorage", "InlineAssemblyMemorySideEffects", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation" @@ -1622,12 +1711,14 @@ }, "0.8.14": { "bugs": [ + "DirtyBytesArrayToStorage", "InlineAssemblyMemorySideEffects" ], "released": "2022-05-17" }, "0.8.2": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", @@ -1638,6 +1729,7 @@ }, "0.8.3": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", @@ -1647,6 +1739,7 @@ }, "0.8.4": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables" @@ -1655,6 +1748,7 @@ }, "0.8.5": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables" @@ -1663,6 +1757,7 @@ }, "0.8.6": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables" @@ -1671,6 +1766,7 @@ }, "0.8.7": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables" @@ -1679,6 +1775,7 @@ }, "0.8.8": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "UserDefinedValueTypesBug", @@ -1688,6 +1785,7 @@ }, "0.8.9": { "bugs": [ + "DirtyBytesArrayToStorage", "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation" ], From 187f0f070d51700006dddb7dc833d8f384f10eda Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Tue, 14 Jun 2022 17:30:14 +0200 Subject: [PATCH 5/6] Some review suggestions. --- docs/bugs.json | 2 +- .../byte_array_to_storage_cleanup.sol | 48 +++++++++---------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/docs/bugs.json b/docs/bugs.json index 979f4264f..b7108fac1 100644 --- a/docs/bugs.json +++ b/docs/bugs.json @@ -3,7 +3,7 @@ "uid": "SOL-2022-5", "name": "DirtyBytesArrayToStorage", "summary": "Copying ``bytes`` arrays from memory or calldata to storage may result in dirty storage values.", - "description": "Copying ``bytes`` arrays from memory or calldata to storage is done in chunks of 32 bytes. Thereby, dirty values in calldata or memory can be written to storage, which may then become observable after a ``.push()`` on the bytes array in storage.", + "description": "Copying ``bytes`` arrays from memory or calldata to storage is done in chunks of 32 bytes even if the length is not a multiple of 32. Thereby, extra bytes past the end of the array may be copied from calldata or memory to storage. These dirty bytes may then become observable after a ``.push()`` without arguments to the bytes array in storage, i.e. such a push will not result in a zero value at the end of the array as expected. This bug only affects the legacy code generation pipeline, the new code generation pipeline via IR is not affected.", "link": "https://blog.soliditylang.org/2022/06/15/dirty-bytes-array-to-storage-bug/", "introduced": "0.0.1", "fixed": "0.8.15", diff --git a/test/libsolidity/semanticTests/byte_array_to_storage_cleanup.sol b/test/libsolidity/semanticTests/byte_array_to_storage_cleanup.sol index f43166e78..f275590e4 100644 --- a/test/libsolidity/semanticTests/byte_array_to_storage_cleanup.sol +++ b/test/libsolidity/semanticTests/byte_array_to_storage_cleanup.sol @@ -1,28 +1,28 @@ contract C { - event ev0(uint[] i0, uint); - bytes public s2; - function h() external returns (bytes memory) { - uint[] memory x = new uint[](2); - emit ev0(x, 0x21); - bytes memory m = new bytes(63); - s2 = m; - s2.push(); - return s2; - } - function g() external returns (bytes memory) { - bytes memory m = new bytes(63); - assembly { - mstore8(add(m, add(32, 63)), 0x42) + event ev(uint[], uint); + bytes public s; + function h() external returns (bytes memory) { + uint[] memory x = new uint[](2); + emit ev(x, 0x21); + bytes memory m = new bytes(63); + s = m; + s.push(); + return s; + } + function g() external returns (bytes memory) { + bytes memory m = new bytes(63); + assembly { + mstore8(add(m, add(32, 63)), 0x42) + } + s = m; + s.push(); + return s; + } + function f(bytes calldata c) external returns (bytes memory) { + s = c; + s.push(); + return s; } - s2 = m; - s2.push(); - return s2; - } - function f(bytes calldata c) external returns (bytes memory) { - s2 = c; - s2.push(); - return s2; - } } // ==== // compileViaYul: also @@ -32,6 +32,6 @@ contract C { // gas legacy: 731840 // gas legacyOptimized: 494859 // h() -> 0x20, 0x40, 0x00, 0 -// ~ emit ev0(uint256[],uint256): 0x40, 0x21, 0x02, 0x00, 0x00 +// ~ emit ev(uint256[],uint256): 0x40, 0x21, 0x02, 0x00, 0x00 // g() -> 0x20, 0x40, 0, 0x00 // f(bytes): 0x20, 33, 0, -1 -> 0x20, 0x22, 0, 0xff00000000000000000000000000000000000000000000000000000000000000 From 27822dbca728b29e2314471fd0fffb121435a443 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Tue, 14 Jun 2022 17:30:24 +0200 Subject: [PATCH 6/6] Remove entry in IR breaking changes. --- docs/ir-breaking-changes.rst | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/docs/ir-breaking-changes.rst b/docs/ir-breaking-changes.rst index 2b5905a96..7e4efce68 100644 --- a/docs/ir-breaking-changes.rst +++ b/docs/ir-breaking-changes.rst @@ -154,37 +154,6 @@ hiding new and different behavior in existing code. - New code generator: ``0`` as all parameters, including return parameters, will be re-initialized before each ``_;`` evaluation. -- Copying ``bytes`` arrays from memory to storage is implemented in a different way. - The old code generator always copies full words, while the new one cuts the byte - array after its end. The old behaviour can lead to dirty data being copied after - the end of the array (but still in the same storage slot). - This causes differences in some contracts, for example: - - .. code-block:: solidity - - // SPDX-License-Identifier: GPL-3.0 - pragma solidity >=0.8.1; - - contract C { - bytes x; - function f() public returns (uint r) { - bytes memory m = "tmp"; - assembly { - mstore(m, 8) - mstore(add(m, 32), "deadbeef15dead") - } - x = m; - assembly { - r := sload(x.slot) - } - } - } - - Previously ``f()`` would return ``0x6465616462656566313564656164000000000000000000000000000000000010`` - (it has correct length, and correct first 8 elements, but then it contains dirty data which was set via assembly). - Now it is returning ``0x6465616462656566000000000000000000000000000000000000000000000010`` (it has - correct length, and correct elements, but does not contain superfluous data). - .. index:: ! evaluation order; expression - For the old code generator, the evaluation order of expressions is unspecified.