From ff5be1799088ca51fb51e9c86031bd2d88fe3bce Mon Sep 17 00:00:00 2001 From: Erik Kundt Date: Fri, 21 Sep 2018 18:26:19 +0200 Subject: [PATCH 1/6] Disallows fixed-size multidim. arrays with zero-length. --- libsolidity/analysis/TypeChecker.cpp | 5 +++++ .../libsolidity/syntaxTests/array/multi_dim_zero_length.sol | 6 ++++++ 2 files changed, 11 insertions(+) create mode 100644 test/libsolidity/syntaxTests/array/multi_dim_zero_length.sol diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 3056561b0..3b5e7b11d 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -822,12 +822,17 @@ bool TypeChecker::visit(VariableDeclaration const& _variable) { case Type::Category::Array: if (auto arrayType = dynamic_cast(varType.get())) + { if ( ((arrayType->location() == DataLocation::Memory) || (arrayType->location() == DataLocation::CallData)) && !arrayType->validForCalldata() ) m_errorReporter.typeError(_variable.location(), "Array is too large to be encoded."); + if (auto baseType = dynamic_cast(arrayType->baseType().get())) + if (!baseType->isDynamicallySized() && baseType->length() == 0) + m_errorReporter.typeError(_variable.location(), "Fixed-size multidimensional arrays are not allowed to have zero length."); + } break; case Type::Category::Mapping: if (auto mappingType = dynamic_cast(varType.get())) diff --git a/test/libsolidity/syntaxTests/array/multi_dim_zero_length.sol b/test/libsolidity/syntaxTests/array/multi_dim_zero_length.sol new file mode 100644 index 000000000..2f487cde7 --- /dev/null +++ b/test/libsolidity/syntaxTests/array/multi_dim_zero_length.sol @@ -0,0 +1,6 @@ +contract C { + bytes[0] a; + function f() public pure returns(bytes32[0][500] memory) {} +} +// ---- +// TypeError: (62-84): Fixed-size multidimensional arrays are not allowed to have zero length. From d821cbdff5a483b3f7a7bd07758bf5e11a7cd762 Mon Sep 17 00:00:00 2001 From: Erik Kundt Date: Sat, 22 Sep 2018 00:18:22 +0200 Subject: [PATCH 2/6] Moves length check to reference resolver. --- libsolidity/analysis/ReferencesResolver.cpp | 4 ++++ libsolidity/analysis/TypeChecker.cpp | 5 ----- .../length/fixed_size_multidim_zero_length.sol | 15 +++++++++++++++ .../array/length/fixed_size_zero_length.sol | 15 +++++++++++++++ .../syntaxTests/array/length/parentheses.sol | 2 +- .../syntaxTests/array/multi_dim_zero_length.sol | 6 ------ .../syntaxTests/parsing/arrays_in_storage.sol | 2 +- 7 files changed, 36 insertions(+), 13 deletions(-) create mode 100644 test/libsolidity/syntaxTests/array/length/fixed_size_multidim_zero_length.sol create mode 100644 test/libsolidity/syntaxTests/array/length/fixed_size_zero_length.sol delete mode 100644 test/libsolidity/syntaxTests/array/multi_dim_zero_length.sol diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp index 8a576e2ed..30aa3d498 100644 --- a/libsolidity/analysis/ReferencesResolver.cpp +++ b/libsolidity/analysis/ReferencesResolver.cpp @@ -231,6 +231,8 @@ void ReferencesResolver::endVisit(Mapping const& _typeName) void ReferencesResolver::endVisit(ArrayTypeName const& _typeName) { TypePointer baseType = _typeName.baseType().annotation().type; + auto arrayType = dynamic_cast(baseType.get()); + bool isFixedSizeArray = (arrayType && arrayType->isByteArray()) || !baseType->isDynamicallySized(); if (!baseType) { solAssert(!m_errorReporter.errors().empty(), ""); @@ -246,6 +248,8 @@ void ReferencesResolver::endVisit(ArrayTypeName const& _typeName) RationalNumberType const* lengthType = dynamic_cast(lengthTypeGeneric.get()); if (!lengthType || !lengthType->mobileType()) fatalTypeError(length->location(), "Invalid array length, expected integer literal or constant expression."); + else if (lengthType->isZero() && isFixedSizeArray) + fatalTypeError(length->location(), "Array with zero length specified."); else if (lengthType->isFractional()) fatalTypeError(length->location(), "Array with fractional length specified."); else if (lengthType->isNegative()) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 3b5e7b11d..3056561b0 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -822,17 +822,12 @@ bool TypeChecker::visit(VariableDeclaration const& _variable) { case Type::Category::Array: if (auto arrayType = dynamic_cast(varType.get())) - { if ( ((arrayType->location() == DataLocation::Memory) || (arrayType->location() == DataLocation::CallData)) && !arrayType->validForCalldata() ) m_errorReporter.typeError(_variable.location(), "Array is too large to be encoded."); - if (auto baseType = dynamic_cast(arrayType->baseType().get())) - if (!baseType->isDynamicallySized() && baseType->length() == 0) - m_errorReporter.typeError(_variable.location(), "Fixed-size multidimensional arrays are not allowed to have zero length."); - } break; case Type::Category::Mapping: if (auto mappingType = dynamic_cast(varType.get())) diff --git a/test/libsolidity/syntaxTests/array/length/fixed_size_multidim_zero_length.sol b/test/libsolidity/syntaxTests/array/length/fixed_size_multidim_zero_length.sol new file mode 100644 index 000000000..fd8f30782 --- /dev/null +++ b/test/libsolidity/syntaxTests/array/length/fixed_size_multidim_zero_length.sol @@ -0,0 +1,15 @@ +contract C { + function a() public pure returns(int[0][500] memory) {} + function b() public pure returns(uint[0][500] memory) {} + function c() public pure returns(byte[0][500] memory) {} + function d() public pure returns(bytes32[0][500] memory) {} + function e() public pure returns(bytes[0][500] memory) {} + function e() public pure returns(string[0][500] memory) {} +} +// ---- +// TypeError: (52-53): Array with zero length specified. +// TypeError: (111-112): Array with zero length specified. +// TypeError: (170-171): Array with zero length specified. +// TypeError: (232-233): Array with zero length specified. +// TypeError: (292-293): Array with zero length specified. +// TypeError: (353-354): Array with zero length specified. diff --git a/test/libsolidity/syntaxTests/array/length/fixed_size_zero_length.sol b/test/libsolidity/syntaxTests/array/length/fixed_size_zero_length.sol new file mode 100644 index 000000000..b38939e39 --- /dev/null +++ b/test/libsolidity/syntaxTests/array/length/fixed_size_zero_length.sol @@ -0,0 +1,15 @@ +contract C { + int[0] a; + uint[0] b; + byte[0] c; + bytes32[0] d; + bytes[0] e; + string[0] f; +} +// ---- +// TypeError: (19-20): Array with zero length specified. +// TypeError: (32-33): Array with zero length specified. +// TypeError: (45-46): Array with zero length specified. +// TypeError: (61-62): Array with zero length specified. +// TypeError: (75-76): Array with zero length specified. +// TypeError: (90-91): Array with zero length specified. diff --git a/test/libsolidity/syntaxTests/array/length/parentheses.sol b/test/libsolidity/syntaxTests/array/length/parentheses.sol index 40f55ad63..8dbcc0a46 100644 --- a/test/libsolidity/syntaxTests/array/length/parentheses.sol +++ b/test/libsolidity/syntaxTests/array/length/parentheses.sol @@ -21,5 +21,5 @@ contract C { uint[((2) + 1) + 1] a12; uint[(2 + 1) + ((1))] a13; uint[(((2) + 1)) + (((1)))] a14; - uint[((((2) + 1)) + (((1))))%1] a15; + uint[((((3) + 1)) + (((1))))%2] a15; } diff --git a/test/libsolidity/syntaxTests/array/multi_dim_zero_length.sol b/test/libsolidity/syntaxTests/array/multi_dim_zero_length.sol deleted file mode 100644 index 2f487cde7..000000000 --- a/test/libsolidity/syntaxTests/array/multi_dim_zero_length.sol +++ /dev/null @@ -1,6 +0,0 @@ -contract C { - bytes[0] a; - function f() public pure returns(bytes32[0][500] memory) {} -} -// ---- -// TypeError: (62-84): Fixed-size multidimensional arrays are not allowed to have zero length. diff --git a/test/libsolidity/syntaxTests/parsing/arrays_in_storage.sol b/test/libsolidity/syntaxTests/parsing/arrays_in_storage.sol index 9181222e4..9adf6f125 100644 --- a/test/libsolidity/syntaxTests/parsing/arrays_in_storage.sol +++ b/test/libsolidity/syntaxTests/parsing/arrays_in_storage.sol @@ -1,6 +1,6 @@ contract c { uint[10] a; uint[] a2; - struct x { uint[2**20] b; y[0] c; } + struct x { uint[2**20] b; y[1] c; } struct y { uint d; mapping(uint=>x)[] e; } } From 13a5890cc36d3ca5e9b4cd53c21dd4616c4eb55b Mon Sep 17 00:00:00 2001 From: Erik Kundt Date: Sat, 22 Sep 2018 00:35:44 +0200 Subject: [PATCH 3/6] Updates changelog. --- Changelog.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Changelog.md b/Changelog.md index 7443a0c6d..60a683687 100644 --- a/Changelog.md +++ b/Changelog.md @@ -83,6 +83,7 @@ Language Features: * General: Allow ``mapping`` storage pointers as arguments and return values in all internal functions. * General: Allow ``struct``s in interfaces. * General: Provide access to the ABI decoder through ``abi.decode(bytes memory data, (...))``. + * General: Disallow zero length for fixed-size arrays. * Parser: Accept the ``address payable`` type during parsing. Compiler Features: @@ -100,6 +101,7 @@ Bugfixes: * Tests: Fix chain parameters to make ipc tests work with newer versions of cpp-ethereum. * Code Generator: Fix allocation of byte arrays (zeroed out too much memory). * Code Generator: Properly handle negative number literals in ABIEncoderV2. + * Code Generator: Do not crash on using a length of zero for multidimensional fixed-size arrays. * Commandline Interface: Correctly handle paths with backslashes on windows. * Fix NatSpec json output for `@notice` and `@dev` tags on contract definitions. * Optimizer: Correctly estimate gas costs of constants for special cases. From e6d87e54c8cdf7c9111707770e995b486ec82460 Mon Sep 17 00:00:00 2001 From: Erik Kundt Date: Sat, 22 Sep 2018 03:07:12 +0200 Subject: [PATCH 4/6] Simplifies zero-length check for fixed-size arrays. --- libsolidity/analysis/ReferencesResolver.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp index 30aa3d498..6217c5d15 100644 --- a/libsolidity/analysis/ReferencesResolver.cpp +++ b/libsolidity/analysis/ReferencesResolver.cpp @@ -231,8 +231,6 @@ void ReferencesResolver::endVisit(Mapping const& _typeName) void ReferencesResolver::endVisit(ArrayTypeName const& _typeName) { TypePointer baseType = _typeName.baseType().annotation().type; - auto arrayType = dynamic_cast(baseType.get()); - bool isFixedSizeArray = (arrayType && arrayType->isByteArray()) || !baseType->isDynamicallySized(); if (!baseType) { solAssert(!m_errorReporter.errors().empty(), ""); @@ -243,12 +241,14 @@ void ReferencesResolver::endVisit(ArrayTypeName const& _typeName) if (Expression const* length = _typeName.length()) { TypePointer lengthTypeGeneric = length->annotation().type; + auto arrayType = dynamic_cast(baseType.get()); + bool isByteOrFixedSizeArray = (arrayType && arrayType->isByteArray()) || !baseType->isDynamicallySized(); if (!lengthTypeGeneric) lengthTypeGeneric = ConstantEvaluator(m_errorReporter).evaluate(*length); RationalNumberType const* lengthType = dynamic_cast(lengthTypeGeneric.get()); if (!lengthType || !lengthType->mobileType()) fatalTypeError(length->location(), "Invalid array length, expected integer literal or constant expression."); - else if (lengthType->isZero() && isFixedSizeArray) + else if (lengthType->isZero() && isByteOrFixedSizeArray) fatalTypeError(length->location(), "Array with zero length specified."); else if (lengthType->isFractional()) fatalTypeError(length->location(), "Array with fractional length specified."); From 466e8f56e6fa7d816cc28028871b43f3a52fcdaa Mon Sep 17 00:00:00 2001 From: Erik Kundt Date: Wed, 26 Sep 2018 12:46:08 +0200 Subject: [PATCH 5/6] Removes unnecessary check of array type. --- libsolidity/analysis/ReferencesResolver.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp index 6217c5d15..81de3c437 100644 --- a/libsolidity/analysis/ReferencesResolver.cpp +++ b/libsolidity/analysis/ReferencesResolver.cpp @@ -241,14 +241,12 @@ void ReferencesResolver::endVisit(ArrayTypeName const& _typeName) if (Expression const* length = _typeName.length()) { TypePointer lengthTypeGeneric = length->annotation().type; - auto arrayType = dynamic_cast(baseType.get()); - bool isByteOrFixedSizeArray = (arrayType && arrayType->isByteArray()) || !baseType->isDynamicallySized(); if (!lengthTypeGeneric) lengthTypeGeneric = ConstantEvaluator(m_errorReporter).evaluate(*length); RationalNumberType const* lengthType = dynamic_cast(lengthTypeGeneric.get()); if (!lengthType || !lengthType->mobileType()) fatalTypeError(length->location(), "Invalid array length, expected integer literal or constant expression."); - else if (lengthType->isZero() && isByteOrFixedSizeArray) + else if (lengthType->isZero()) fatalTypeError(length->location(), "Array with zero length specified."); else if (lengthType->isFractional()) fatalTypeError(length->location(), "Array with fractional length specified."); From 79307059e502f1383e1ac50cd7af783aec6588a2 Mon Sep 17 00:00:00 2001 From: Erik Kundt Date: Wed, 26 Sep 2018 13:47:59 +0200 Subject: [PATCH 6/6] Updates breaking changes documentation. --- docs/050-breaking-changes.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/050-breaking-changes.rst b/docs/050-breaking-changes.rst index 7b5acd89b..e0c335385 100644 --- a/docs/050-breaking-changes.rst +++ b/docs/050-breaking-changes.rst @@ -220,6 +220,8 @@ Variables * Detecting cyclic dependencies in variables and structs is limited in recursion to 256. +* Fixed-size arrays with a length of zero are now disallowed. + Syntax ------