Merge pull request #6362 from ethereum/fixABIEncoderV2StorageRead

Fix abi encoder v2 storage read

Changelog.md

@@ -1,6 +1,8 @@
 ### 0.5.7 (unreleased)
 
 Important Bugfixes:
+ * ABIEncoderV2: Fix bugs related to loading short value types from storage when encoding a packed array or struct from storage.
+ * ABIEncoderV2: Fix buffer overflow problem when encoding packed array from storage.
  * Optimizer: Fix wrong ordering of arguments in byte optimization rule for constants.
 
 Language Features:

docs/bugs.json

@@ -1,4 +1,15 @@
 [
+    {
+        "name": "ABIEncoderV2PackedStorage",
+        "summary": "Storage structs and arrays with types shorter than 32 bytes can cause data corruption if encoded directly from storage using the experimental ABIEncoderV2.",
+        "description": "Elements of structs and arrays that are shorter than 32 bytes are not properly decoded from storage when encoded directly (i.e. not via a memory type) using ABIEncoderV2. This can cause corruption in the values themselves but can also overwrite other parts of the encoded data.",
+        "introduced": "0.4.19",
+        "fixed": "0.5.7",
+        "severity": "low",
+        "conditions": {
+            "ABIEncoderV2": true
+        }
+    },
     {
         "name": "IncorrectByteInstructionOptimization",
 	"summary": "The optimizer incorrectly handles byte opcodes whose second argument is 31 or a constant expression that evaluates to 31. This can result in unexpected values.",

docs/bugs_by_version.json

@@ -456,6 +456,7 @@
     },
     "0.4.19": {
         "bugs": [
+            "ABIEncoderV2PackedStorage",
             "ExpExponentCleanup",
             "EventStructWrongData",
             "NestedArrayFunctionCallDecoder"
@@ -479,6 +480,7 @@
     },
     "0.4.20": {
         "bugs": [
+            "ABIEncoderV2PackedStorage",
             "ExpExponentCleanup",
             "EventStructWrongData",
             "NestedArrayFunctionCallDecoder"
@@ -487,6 +489,7 @@
     },
     "0.4.21": {
         "bugs": [
+            "ABIEncoderV2PackedStorage",
             "ExpExponentCleanup",
             "EventStructWrongData",
             "NestedArrayFunctionCallDecoder"
@@ -495,6 +498,7 @@
     },
     "0.4.22": {
         "bugs": [
+            "ABIEncoderV2PackedStorage",
             "ExpExponentCleanup",
             "EventStructWrongData",
             "OneOfTwoConstructorsSkipped"
@@ -503,6 +507,7 @@
     },
     "0.4.23": {
         "bugs": [
+            "ABIEncoderV2PackedStorage",
             "ExpExponentCleanup",
             "EventStructWrongData"
         ],
@@ -510,13 +515,16 @@
     },
     "0.4.24": {
         "bugs": [
+            "ABIEncoderV2PackedStorage",
             "ExpExponentCleanup",
             "EventStructWrongData"
         ],
         "released": "2018-05-16"
     },
     "0.4.25": {
-        "bugs": [],
+        "bugs": [
+            "ABIEncoderV2PackedStorage"
+        ],
         "released": "2018-09-12"
     },
     "0.4.3": {
@@ -610,27 +618,38 @@
         "released": "2017-01-31"
     },
     "0.5.0": {
-        "bugs": [],
+        "bugs": [
+            "ABIEncoderV2PackedStorage"
+        ],
         "released": "2018-11-13"
     },
     "0.5.1": {
-        "bugs": [],
+        "bugs": [
+            "ABIEncoderV2PackedStorage"
+        ],
         "released": "2018-12-03"
     },
     "0.5.2": {
-        "bugs": [],
+        "bugs": [
+            "ABIEncoderV2PackedStorage"
+        ],
         "released": "2018-12-19"
     },
     "0.5.3": {
-        "bugs": [],
+        "bugs": [
+            "ABIEncoderV2PackedStorage"
+        ],
         "released": "2019-01-22"
     },
     "0.5.4": {
-        "bugs": [],
+        "bugs": [
+            "ABIEncoderV2PackedStorage"
+        ],
         "released": "2019-02-12"
     },
     "0.5.5": {
         "bugs": [
+            "ABIEncoderV2PackedStorage",
             "IncorrectByteInstructionOptimization",
             "DoubleShiftSizeOverflow"
         ],
@@ -638,6 +657,7 @@
     },
     "0.5.6": {
         "bugs": [
+            "ABIEncoderV2PackedStorage",
             "IncorrectByteInstructionOptimization"
         ],
         "released": "2019-03-13"

libsolidity/ast/Types.cpp

@@ -2842,6 +2842,14 @@ u256 FunctionType::storageSize() const
 		solAssert(false, "Storage size of non-storable function type requested.");
 }
 
+bool FunctionType::leftAligned() const
+{
+	if (m_kind == Kind::External)
+		return true;
+	else
+		solAssert(false, "Alignment property of non-exportable function type requested.");
+}
+
 unsigned FunctionType::storageBytes() const
 {
 	if (m_kind == Kind::External)

libsolidity/ast/Types.h

@@ -236,6 +236,13 @@ public:
 	/// In order to avoid computation at runtime of whether such moving is necessary, structs and
 	/// array data (not each element) always start a new slot.
 	virtual unsigned storageBytes() const { return 32; }
+	/// Returns true if the type is a value type that is left-aligned on the stack with a size of
+	/// storageBytes() bytes. Returns false if the type is a value type that is right-aligned on
+	/// the stack with a size of storageBytes() bytes. Asserts if it is not a value type or the
+	/// encoding is more complicated.
+	/// Signed integers are not considered "more complicated" even though they need to be
+	/// sign-extended.
+	virtual bool leftAligned() const { solAssert(false, "Alignment property of non-value type requested."); }
 	/// Returns true if the type can be stored in storage.
 	virtual bool canBeStored() const { return true; }
 	/// Returns false if the type cannot live outside the storage, i.e. if it includes some mapping.
@@ -345,6 +352,7 @@ public:
 
 	unsigned calldataEncodedSize(bool _padded = true) const override { return _padded ? 32 : 160 / 8; }
 	unsigned storageBytes() const override { return 160 / 8; }
+	bool leftAligned() const override { return false; }
 	bool isValueType() const override { return true; }
 
 	MemberList::MemberMap nativeMembers(ContractDefinition const*) const override;
@@ -390,6 +398,7 @@ public:
 
 	unsigned calldataEncodedSize(bool _padded = true) const override { return _padded ? 32 : m_bits / 8; }
 	unsigned storageBytes() const override { return m_bits / 8; }
+	bool leftAligned() const override { return false; }
 	bool isValueType() const override { return true; }
 
 	std::string toString(bool _short) const override;
@@ -432,6 +441,7 @@ public:
 
 	unsigned calldataEncodedSize(bool _padded = true) const override { return _padded ? 32 : m_totalBits / 8; }
 	unsigned storageBytes() const override { return m_totalBits / 8; }
+	bool leftAligned() const override { return false; }
 	bool isValueType() const override { return true; }
 
 	std::string toString(bool _short) const override;
@@ -578,6 +588,7 @@ public:
 
 	unsigned calldataEncodedSize(bool _padded) const override { return _padded && m_bytes > 0 ? 32 : m_bytes; }
 	unsigned storageBytes() const override { return m_bytes; }
+	bool leftAligned() const override { return true; }
 	bool isValueType() const override { return true; }
 
 	std::string toString(bool) const override { return "bytes" + dev::toString(m_bytes); }
@@ -604,6 +615,7 @@ public:
 
 	unsigned calldataEncodedSize(bool _padded) const override{ return _padded ? 32 : 1; }
 	unsigned storageBytes() const override { return 1; }
+	bool leftAligned() const override { return false; }
 	bool isValueType() const override { return true; }
 
 	std::string toString(bool) const override { return "bool"; }
@@ -775,6 +787,7 @@ public:
 		return encodingType()->calldataEncodedSize(_padded);
 	}
 	unsigned storageBytes() const override { solAssert(!isSuper(), ""); return 20; }
+	bool leftAligned() const override { solAssert(!isSuper(), ""); return false; }
 	bool canLiveOutsideStorage() const override { return !isSuper(); }
 	unsigned sizeOnStack() const override { return m_super ? 0 : 1; }
 	bool isValueType() const override { return !isSuper(); }
@@ -897,6 +910,7 @@ public:
 		return encodingType()->calldataEncodedSize(_padded);
 	}
 	unsigned storageBytes() const override;
+	bool leftAligned() const override { return false; }
 	bool canLiveOutsideStorage() const override { return true; }
 	std::string toString(bool _short) const override;
 	std::string canonicalName() const override;
@@ -1096,6 +1110,7 @@ public:
 	unsigned calldataEncodedSize(bool _padded) const override;
 	bool canBeStored() const override { return m_kind == Kind::Internal || m_kind == Kind::External; }
 	u256 storageSize() const override;
+	bool leftAligned() const override;
 	unsigned storageBytes() const override;
 	bool isValueType() const override { return true; }
 	bool canLiveOutsideStorage() const override { return m_kind == Kind::Internal || m_kind == Kind::External; }

libsolidity/codegen/ABIFunctions.cpp

@@ -346,6 +346,39 @@ string ABIFunctions::cleanupFunction(Type const& _type, bool _revertOnFailure)
 	});
 }
 
+string ABIFunctions::cleanupFromStorageFunction(Type const& _type, bool _splitFunctionTypes)
+{
+	solAssert(_type.isValueType(), "");
+	solUnimplementedAssert(!_splitFunctionTypes, "");
+
+	string functionName = string("cleanup_from_storage_") + (_splitFunctionTypes ? "split_" : "") + _type.identifier();
+	return createFunction(functionName, [&] {
+		Whiskers templ(R"(
+			function <functionName>(value) -> cleaned {
+				<body>
+			}
+		)");
+		templ("functionName", functionName);
+
+		unsigned storageBytes = _type.storageBytes();
+		if (IntegerType const* type = dynamic_cast<IntegerType const*>(&_type))
+			if (type->isSigned() && storageBytes != 32)
+			{
+				templ("body", "cleaned := signextend(" + to_string(storageBytes - 1) + ", value)");
+				return templ.render();
+			}
+
+		if (storageBytes == 32)
+			templ("body", "cleaned := value");
+		else if (_type.leftAligned())
+			templ("body", "cleaned := " + m_utils.shiftLeftFunction(256 - 8 * storageBytes) + "(value)");
+		else
+			templ("body", "cleaned := and(value, " + toCompactHexWithPrefix((u256(1) << (8 * storageBytes)) - 1) + ")");
+
+		return templ.render();
+	});
+}
+
 string ABIFunctions::conversionFunction(Type const& _from, Type const& _to)
 {
 	string functionName =
@@ -778,7 +811,15 @@ string ABIFunctions::abiEncodingFunctionSimpleArray(
 		subOptions.encodeFunctionFromStack = false;
 		subOptions.padded = true;
 		templ("encodeToMemoryFun", abiEncodeAndReturnUpdatedPosFunction(*_from.baseType(), *_to.baseType(), subOptions));
-		templ("arrayElementAccess", inMemory ? "mload(srcPtr)" : _from.baseType()->isValueType() ? "sload(srcPtr)" : "srcPtr" );
+		if (inMemory)
+			templ("arrayElementAccess", "mload(srcPtr)");
+		else if (_from.baseType()->isValueType())
+		{
+			solAssert(_from.dataStoredIn(DataLocation::Storage), "");
+			templ("arrayElementAccess", readFromStorage(*_from.baseType(), 0, false) + "(srcPtr)");
+		}
+		else
+			templ("arrayElementAccess", "srcPtr");
 		templ("nextArrayElement", m_utils.nextArrayElementFunction(_from));
 		return templ.render();
 	});
@@ -886,8 +927,9 @@ string ABIFunctions::abiEncodingFunctionCompactStorageArray(
 			bool dynamic = _to.isDynamicallyEncoded();
 			size_t storageBytes = _from.baseType()->storageBytes();
 			size_t itemsPerSlot = 32 / storageBytes;
-			// This always writes full slot contents to memory, which might be
+			solAssert(itemsPerSlot > 0, "");
-			// more than desired, i.e. it always writes beyond the end of memory.
+			// The number of elements we need to handle manually after the loop.
+			size_t spill = size_t(_from.length() % itemsPerSlot);
 			Whiskers templ(
 				R"(
 					// <readableTypeNameFrom> -> <readableTypeNameTo>
@@ -896,16 +938,31 @@ string ABIFunctions::abiEncodingFunctionCompactStorageArray(
 						pos := <storeLength>(pos, length)
 						let originalPos := pos
 						let srcPtr := <dataArea>(value)
-						for { let i := 0 } lt(i, length) { i := add(i, <itemsPerSlot>) }
+						let itemCounter := 0
-						{
+						if <useLoop> {
+							// Run the loop over all full slots
+							for { } lt(add(itemCounter, sub(<itemsPerSlot>, 1)), length)
+										{ itemCounter := add(itemCounter, <itemsPerSlot>) }
+							{
+								let data := sload(srcPtr)
+								<#items>
+									<encodeToMemoryFun>(<extractFromSlot>(data), pos)
+									pos := add(pos, <elementEncodedSize>)
+								</items>
+								srcPtr := add(srcPtr, 1)
+							}
+						}
+						// Handle the last (not necessarily full) slot specially
+						if <useSpill> {
 							let data := sload(srcPtr)
 							<#items>
-								<encodeToMemoryFun>(<shiftRightFun>(data), pos)
+								if <inRange> {
-								pos := add(pos, <elementEncodedSize>)
+									<encodeToMemoryFun>(<extractFromSlot>(data), pos)
+									pos := add(pos, <elementEncodedSize>)
+									itemCounter := add(itemCounter, 1)
+								}
 							</items>
-							srcPtr := add(srcPtr, 1)
 						}
-						pos := add(originalPos, mul(length, <elementEncodedSize>))
 						<assignEnd>
 					}
 				)"
@@ -918,6 +975,15 @@ string ABIFunctions::abiEncodingFunctionCompactStorageArray(
 			templ("lengthFun", m_utils.arrayLengthFunction(_from));
 			templ("storeLength", arrayStoreLengthForEncodingFunction(_to, _options));
 			templ("dataArea", m_utils.arrayDataAreaFunction(_from));
+			// We skip the loop for arrays that fit a single slot.
+			if (_from.isDynamicallySized() || _from.length() >= itemsPerSlot)
+				templ("useLoop", "1");
+			else
+				templ("useLoop", "0");
+			if (_from.isDynamicallySized() || spill != 0)
+				templ("useSpill", "1");
+			else
+				templ("useSpill", "0");
 			templ("itemsPerSlot", to_string(itemsPerSlot));
 			// We use padded size because array elements are always padded.
 			string elementEncodedSize = toCompactHexWithPrefix(_to.baseType()->calldataEncodedSize());
@@ -934,7 +1000,15 @@ string ABIFunctions::abiEncodingFunctionCompactStorageArray(
 			templ("encodeToMemoryFun", encodeToMemoryFun);
 			std::vector<std::map<std::string, std::string>> items(itemsPerSlot);
 			for (size_t i = 0; i < itemsPerSlot; ++i)
-				items[i]["shiftRightFun"] = m_utils.shiftRightFunction(i * storageBytes * 8);
+			{
+				if (_from.isDynamicallySized())
+					items[i]["inRange"] = "lt(itemCounter, length)";
+				else if (i < spill)
+					items[i]["inRange"] = "1";
+				else
+					items[i]["inRange"] = "0";
+				items[i]["extractFromSlot"] = extractFromStorageValue(*_from.baseType(), i * storageBytes, false);
+			}
 			templ("items", items);
 			return templ.render();
 		}
@@ -1020,7 +1094,7 @@ string ABIFunctions::abiEncodingFunctionStruct(
 						members.back()["preprocess"] = "slotValue := sload(add(value, " + toCompactHexWithPrefix(storageSlotOffset) + "))";
 						previousSlotOffset = storageSlotOffset;
 					}
-					members.back()["retrieveValue"] = m_utils.shiftRightFunction(intraSlotOffset * 8) + "(slotValue)";
+					members.back()["retrieveValue"] = extractFromStorageValue(*memberTypeFrom, intraSlotOffset, false) + "(slotValue)";
 				}
 				else
 				{
@@ -1509,6 +1583,52 @@ string ABIFunctions::abiDecodingFunctionFunctionType(FunctionType const& _type,
 	});
 }
 
+
+string ABIFunctions::readFromStorage(Type const& _type, size_t _offset, bool _splitFunctionTypes)
+{
+	solUnimplementedAssert(!_splitFunctionTypes, "");
+	string functionName =
+		"read_from_storage_" +
+		string(_splitFunctionTypes ? "split_" : "") +
+		"offset_" +
+		to_string(_offset) +
+		_type.identifier();
+	return m_functionCollector->createFunction(functionName, [&] {
+		solAssert(_type.sizeOnStack() == 1, "");
+		return Whiskers(R"(
+			function <functionName>(slot) -> value {
+				value := <extract>(sload(slot))
+			}
+		)")
+		("functionName", functionName)
+		("extract", extractFromStorageValue(_type, _offset, false))
+		.render();
+	});
+}
+
+string ABIFunctions::extractFromStorageValue(Type const& _type, size_t _offset, bool _splitFunctionTypes)
+{
+	solUnimplementedAssert(!_splitFunctionTypes, "");
+
+	string functionName =
+		"extract_from_storage_value_" +
+		string(_splitFunctionTypes ? "split_" : "") +
+		"offset_" +
+		to_string(_offset) +
+		_type.identifier();
+	return m_functionCollector->createFunction(functionName, [&] {
+		return Whiskers(R"(
+			function <functionName>(slot_value) -> value {
+				value := <cleanupStorage>(<shr>(slot_value))
+			}
+		)")
+		("functionName", functionName)
+		("shr", m_utils.shiftRightFunction(_offset * 8))
+		("cleanupStorage", cleanupFromStorageFunction(_type, false))
+		.render();
+	});
+}
+
 string ABIFunctions::arrayStoreLengthForEncodingFunction(ArrayType const& _type, EncodingOptions const& _options)
 {
 	string functionName = "array_storeLengthForEncoding_" + _type.identifier() + _options.toFunctionNameSuffix();

libsolidity/codegen/ABIFunctions.h

@@ -134,6 +134,15 @@ private:
 	/// otherwise an assertion failure.
 	std::string cleanupFunction(Type const& _type, bool _revertOnFailure = false);
 
+	/// Performs cleanup after reading from a potentially compressed storage slot.
+	/// The function does not perform any validation, it just masks or sign-extends
+	/// higher order bytes or left-aligns (in case of bytesNN).
+	/// The storage cleanup expects the value to be right-aligned with potentially
+	/// dirty higher order bytes.
+	/// @param _splitFunctionTypes if false, returns the address and function signature in a
+	/// single variable.
+	std::string cleanupFromStorageFunction(Type const& _type, bool _splitFunctionTypes);
+
 	/// @returns the name of the function that converts a value of type @a _from
 	/// to a value of type @a _to. The resulting vale is guaranteed to be in range
 	/// (i.e. "clean"). Asserts on failure.
@@ -233,6 +242,19 @@ private:
 	/// Part of @a abiDecodingFunction for array types.
 	std::string abiDecodingFunctionFunctionType(FunctionType const& _type, bool _fromMemory, bool _forUseOnStack);
 
+	/// @returns a function that reads a value type from storage.
+	/// Performs bit mask/sign extend cleanup and appropriate left / right shift, but not validation.
+	/// @param _splitFunctionTypes if false, returns the address and function signature in a
+	/// single variable.
+	std::string readFromStorage(Type const& _type, size_t _offset, bool _splitFunctionTypes);
+
+	/// @returns a function that extracts a value type from storage slot that has been
+	/// retrieved already.
+	/// Performs bit mask/sign extend cleanup and appropriate left / right shift, but not validation.
+	/// @param _splitFunctionTypes if false, returns the address and function signature in a
+	/// single variable.
+	std::string extractFromStorageValue(Type const& _type, size_t _offset, bool _splitFunctionTypes);
+
 	/// @returns the name of a function used during encoding that stores the length
 	/// if the array is dynamically sized (and the options do not request in-place encoding).
 	/// It returns the new encoding position.

libsolidity/codegen/YulUtilFunctions.h

@@ -68,6 +68,7 @@ public:
 
 	std::string shiftLeftFunction(size_t _numBits);
 	std::string shiftRightFunction(size_t _numBits);
+
 	/// @returns the name of a function that rounds its input to the next multiple
 	/// of 32 or the input if it is a multiple of 32.
 	std::string roundUpFunction();

test/libsolidity/ABIEncoderTests.cpp

@@ -18,15 +18,19 @@
  * Unit tests for Solidity's ABI encoder.
  */
 
-#include <functional>
-#include <string>
-#include <tuple>
-#include <boost/test/unit_test.hpp>
-#include <liblangutil/Exceptions.h>
 #include <test/libsolidity/SolidityExecutionFramework.h>
 
 #include <test/libsolidity/ABITestsCommon.h>
 
+#include <liblangutil/Exceptions.h>
+
+#include <boost/algorithm/string/replace.hpp>
+#include <boost/test/unit_test.hpp>
+
+#include <functional>
+#include <string>
+#include <tuple>
+
 using namespace std;
 using namespace std::placeholders;
 using namespace dev::test;
@@ -510,6 +514,204 @@ BOOST_AUTO_TEST_CASE(structs2)
 	)
 }
 
+BOOST_AUTO_TEST_CASE(bool_arrays)
+{
+	string sourceCode = R"(
+		contract C {
+			bool[] x;
+			bool[4] y;
+			event E(bool[], bool[4]);
+			function f() public returns (bool[] memory, bool[4] memory) {
+				x.length = 4;
+				x[0] = true;
+				x[1] = false;
+				x[2] = true;
+				x[3] = false;
+				y[0] = true;
+				y[1] = false;
+				y[2] = true;
+				y[3] = false;
+				emit E(x, y);
+				return (x, y); // this copies to memory first
+			}
+		}
+	)";
+
+	BOTH_ENCODERS(
+		compileAndRun(sourceCode, 0, "C");
+		bytes encoded = encodeArgs(
+			0xa0, 1, 0, 1, 0,
+			4, 1, 0, 1, 0
+		);
+		ABI_CHECK(callContractFunction("f()"), encoded);
+		REQUIRE_LOG_DATA(encoded);
+	)
+}
+
+BOOST_AUTO_TEST_CASE(bool_arrays_split)
+{
+	string sourceCode = R"(
+		contract C {
+			bool[] x;
+			bool[4] y;
+			event E(bool[], bool[4]);
+			function store() public {
+				x.length = 4;
+				x[0] = true;
+				x[1] = false;
+				x[2] = true;
+				x[3] = false;
+				y[0] = true;
+				y[1] = false;
+				y[2] = true;
+				y[3] = false;
+			}
+			function f() public returns (bool[] memory, bool[4] memory) {
+				emit E(x, y);
+				return (x, y); // this copies to memory first
+			}
+		}
+	)";
+
+	BOTH_ENCODERS(
+		compileAndRun(sourceCode, 0, "C");
+		bytes encoded = encodeArgs(
+			0xa0, 1, 0, 1, 0,
+			4, 1, 0, 1, 0
+		);
+		ABI_CHECK(callContractFunction("store()"), bytes{});
+		ABI_CHECK(callContractFunction("f()"), encoded);
+		REQUIRE_LOG_DATA(encoded);
+	)
+}
+
+BOOST_AUTO_TEST_CASE(bytesNN_arrays)
+{
+	// This tests that encoding packed arrays from storage work correctly.
+	string sourceCode = R"(
+		contract C {
+			bytes8[] x;
+			bytesWIDTH[SIZE] y;
+			event E(bytes8[], bytesWIDTH[SIZE]);
+			function store() public {
+				x.length = 2;
+				x[0] = "abc";
+				x[1] = "def";
+				for (uint i = 0; i < y.length; i ++)
+					y[i] = bytesWIDTH(uintUINTWIDTH(i + 1));
+			}
+			function f() public returns (bytes8[] memory, bytesWIDTH[SIZE] memory) {
+				emit E(x, y);
+				return (x, y); // this copies to memory first
+			}
+		}
+	)";
+
+	BOTH_ENCODERS(
+		for (size_t size = 1; size < 15; size++)
+		{
+			for (size_t width: {1, 2, 4, 5, 7, 15, 16, 17, 31, 32})
+			{
+				string source = boost::algorithm::replace_all_copy(sourceCode, "SIZE", to_string(size));
+				source = boost::algorithm::replace_all_copy(source, "UINTWIDTH", to_string(width * 8));
+				source = boost::algorithm::replace_all_copy(source, "WIDTH", to_string(width));
+				compileAndRun(source, 0, "C");
+				ABI_CHECK(callContractFunction("store()"), bytes{});
+				vector<u256> arr;
+				for (size_t i = 0; i < size; i ++)
+					arr.emplace_back(u256(i + 1) << (8 * (32 - width)));
+				bytes encoded = encodeArgs(
+					0x20 * (1 + size), arr,
+					2, "abc", "def"
+				);
+				ABI_CHECK(callContractFunction("f()"), encoded);
+				REQUIRE_LOG_DATA(encoded);
+			}
+		}
+	)
+}
+
+BOOST_AUTO_TEST_CASE(bytesNN_arrays_dyn)
+{
+	// This tests that encoding packed arrays from storage work correctly.
+	string sourceCode = R"(
+		contract C {
+			bytes8[] x;
+			bytesWIDTH[] y;
+			event E(bytesWIDTH[], bytes8[]);
+			function store() public {
+				x.length = 2;
+				x[0] = "abc";
+				x[1] = "def";
+				for (uint i = 0; i < SIZE; i ++)
+					y.push(bytesWIDTH(uintUINTWIDTH(i + 1)));
+			}
+			function f() public returns (bytesWIDTH[] memory, bytes8[] memory) {
+				emit E(y, x);
+				return (y, x); // this copies to memory first
+			}
+		}
+	)";
+
+	BOTH_ENCODERS(
+		for (size_t size = 0; size < 15; size++)
+		{
+			for (size_t width: {1, 2, 4, 5, 7, 15, 16, 17, 31, 32})
+			{
+				string source = boost::algorithm::replace_all_copy(sourceCode, "SIZE", to_string(size));
+				source = boost::algorithm::replace_all_copy(source, "UINTWIDTH", to_string(width * 8));
+				source = boost::algorithm::replace_all_copy(source, "WIDTH", to_string(width));
+				compileAndRun(source, 0, "C");
+				ABI_CHECK(callContractFunction("store()"), bytes{});
+				vector<u256> arr;
+				for (size_t i = 0; i < size; i ++)
+					arr.emplace_back(u256(i + 1) << (8 * (32 - width)));
+				bytes encoded = encodeArgs(
+					0x20 * 2, 0x20 * (3 + size),
+					size, arr,
+					2, "abc", "def"
+				);
+				ABI_CHECK(callContractFunction("f()"), encoded);
+				REQUIRE_LOG_DATA(encoded);
+			}
+		}
+	)
+}
+
+BOOST_AUTO_TEST_CASE(packed_structs)
+{
+	string sourceCode = R"(
+		contract C {
+			struct S { bool a; int8 b; function() external g; bytes3 d; int8 e; }
+			S s;
+			event E(S);
+			function store() public {
+				s.a = false;
+				s.b = -5;
+				s.g = this.g;
+				s.d = 0x010203;
+				s.e = -3;
+			}
+			function f() public returns (S memory) {
+				emit E(s);
+				return s; // this copies to memory first
+			}
+			function g() public pure {}
+		}
+	)";
+
+	NEW_ENCODER(
+		compileAndRun(sourceCode, 0, "C");
+		ABI_CHECK(callContractFunction("store()"), bytes{});
+		bytes fun = m_contractAddress.asBytes() + fromHex("0xe2179b8e");
+		bytes encoded = encodeArgs(
+			0, u256(-5), asString(fun), "\x01\x02\x03", u256(-3)
+		);
+		ABI_CHECK(callContractFunction("f()"), encoded);
+		REQUIRE_LOG_DATA(encoded);
+	)
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }