From 0be325dc0dcf5b18c5246271bb55c058cd76a9df Mon Sep 17 00:00:00 2001 From: Martin Blicha Date: Fri, 11 Dec 2020 18:59:03 +0100 Subject: [PATCH 01/10] [SMTChecker] Fix handling of function calls where the function identifier is nested in a tuple. --- libsolidity/formal/SMTEncoder.cpp | 5 ++-- .../functions_identifier_nested_tuple_1.sol | 15 ++++++++++++ .../functions_identifier_nested_tuple_2.sol | 23 +++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 test/libsolidity/smtCheckerTests/functions/functions_identifier_nested_tuple_1.sol create mode 100644 test/libsolidity/smtCheckerTests/functions/functions_identifier_nested_tuple_2.sol diff --git a/libsolidity/formal/SMTEncoder.cpp b/libsolidity/formal/SMTEncoder.cpp index be67326c3..97268560d 100644 --- a/libsolidity/formal/SMTEncoder.cpp +++ b/libsolidity/formal/SMTEncoder.cpp @@ -2547,10 +2547,10 @@ FunctionDefinition const* SMTEncoder::functionCallToDefinition(FunctionCall cons FunctionDefinition const* funDef = nullptr; Expression const* calledExpr = &_funCall.expression(); - if (TupleExpression const* fun = dynamic_cast(&_funCall.expression())) + if (TupleExpression const* fun = dynamic_cast(calledExpr)) { solAssert(fun->components().size() == 1, ""); - calledExpr = fun->components().front().get(); + calledExpr = innermostTuple(*calledExpr); } if (Identifier const* fun = dynamic_cast(calledExpr)) @@ -2689,6 +2689,7 @@ vector SMTEncoder::symbolicArguments(FunctionCall const& _f unsigned firstParam = 0; if (funType->bound()) { + calledExpr = innermostTuple(*calledExpr); auto const& boundFunction = dynamic_cast(calledExpr); solAssert(boundFunction, ""); args.push_back(expr(boundFunction->expression(), functionParams.front()->type())); diff --git a/test/libsolidity/smtCheckerTests/functions/functions_identifier_nested_tuple_1.sol b/test/libsolidity/smtCheckerTests/functions/functions_identifier_nested_tuple_1.sol new file mode 100644 index 000000000..a329120cc --- /dev/null +++ b/test/libsolidity/smtCheckerTests/functions/functions_identifier_nested_tuple_1.sol @@ -0,0 +1,15 @@ +pragma experimental SMTChecker; + +contract C { + uint x; + function f() public { + x = 0; + ((inc))(); + assert(x == 1); // should hold + } + + function inc() internal returns (uint) { + require(x < 100); + return ++x; + } +} diff --git a/test/libsolidity/smtCheckerTests/functions/functions_identifier_nested_tuple_2.sol b/test/libsolidity/smtCheckerTests/functions/functions_identifier_nested_tuple_2.sol new file mode 100644 index 000000000..9664722ee --- /dev/null +++ b/test/libsolidity/smtCheckerTests/functions/functions_identifier_nested_tuple_2.sol @@ -0,0 +1,23 @@ +pragma experimental SMTChecker; + +library L { + struct S { + uint256[] data; + } + function f(S memory _s) internal pure returns (uint256) { + require(_s.data.length > 0); + return 42; + } +} + +contract C { + using L for L.S; + function f() public pure returns (uint256 y) { + L.S memory x; + y = (x.f)(); + assert(y == 42); // should hold + } +} +// ---- +// Warning 6031: (289-292): Internal error: Expression undefined for SMT solver. +// Warning 6031: (289-292): Internal error: Expression undefined for SMT solver. From 27402781c4bab8d7cffbbfb64a62001dc568df5e Mon Sep 17 00:00:00 2001 From: Martin Blicha Date: Mon, 14 Dec 2020 11:54:54 +0100 Subject: [PATCH 02/10] [SMTChecker] Fixed crash on push to bytes on lhs of an assignment --- Changelog.md | 1 + libsolidity/formal/SMTEncoder.cpp | 5 +++- .../push_as_lhs_and_rhs_bytes.sol | 16 ++++++++++++ .../array_members/push_as_lhs_bytes.sol | 22 ++++++++++++++++ .../array_members/push_as_lhs_bytes_2d.sol | 26 +++++++++++++++++++ 5 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 test/libsolidity/smtCheckerTests/array_members/push_as_lhs_and_rhs_bytes.sol create mode 100644 test/libsolidity/smtCheckerTests/array_members/push_as_lhs_bytes.sol create mode 100644 test/libsolidity/smtCheckerTests/array_members/push_as_lhs_bytes_2d.sol diff --git a/Changelog.md b/Changelog.md index c9f4d2318..983597589 100644 --- a/Changelog.md +++ b/Changelog.md @@ -25,6 +25,7 @@ Bugfixes: * SMTChecker: Fix internal compiler error when doing bitwise compound assignment with string literals. * SMTChecker: Fix internal error when trying to generate counterexamples with old z3. * SMTChecker: Fix segmentation fault that could occur on certain SMT-enabled sources when no SMT solver was available. + * SMTChecker: Fix internal error when ``bytes.push()`` is used as the LHS of an assignment. * Type Checker: ``super`` is not available in libraries. * Type Checker: Disallow leading zeroes in sized-types (e.g. ``bytes000032``), but allow them to be treated as identifiers. * Yul Optimizer: Fix a bug in NameSimplifier where a new name created by NameSimplifier could also be created by NameDispenser. diff --git a/libsolidity/formal/SMTEncoder.cpp b/libsolidity/formal/SMTEncoder.cpp index 97268560d..e39827723 100644 --- a/libsolidity/formal/SMTEncoder.cpp +++ b/libsolidity/formal/SMTEncoder.cpp @@ -709,6 +709,7 @@ void SMTEncoder::endVisit(FunctionCall const& _funCall) break; } case FunctionType::Kind::ArrayPush: + case FunctionType::Kind::ByteArrayPush: arrayPush(_funCall); break; case FunctionType::Kind::ArrayPop: @@ -1572,6 +1573,8 @@ void SMTEncoder::arrayPushPopAssign(Expression const& _expr, smtutil::Expression else if (auto const* funCall = dynamic_cast(expr)) { FunctionType const& funType = dynamic_cast(*funCall->expression().annotation().type); + // Push cannot occur on an expression that is itself a ByteArrayPush, i.e., bytes.push().push() is not possible. + solAssert(funType.kind() != FunctionType::Kind::ByteArrayPush, ""); if (funType.kind() == FunctionType::Kind::ArrayPush) { auto memberAccess = dynamic_cast(&funCall->expression()); @@ -2499,7 +2502,7 @@ MemberAccess const* SMTEncoder::isEmptyPush(Expression const& _expr) const ) { auto const& funType = dynamic_cast(*funCall->expression().annotation().type); - if (funType.kind() == FunctionType::Kind::ArrayPush) + if (funType.kind() == FunctionType::Kind::ArrayPush || funType.kind() == FunctionType::Kind::ByteArrayPush) return &dynamic_cast(funCall->expression()); } return nullptr; diff --git a/test/libsolidity/smtCheckerTests/array_members/push_as_lhs_and_rhs_bytes.sol b/test/libsolidity/smtCheckerTests/array_members/push_as_lhs_and_rhs_bytes.sol new file mode 100644 index 000000000..b56761611 --- /dev/null +++ b/test/libsolidity/smtCheckerTests/array_members/push_as_lhs_and_rhs_bytes.sol @@ -0,0 +1,16 @@ +pragma experimental SMTChecker; + +contract C { + bytes b; + function f() public { + b.push() = b.push(); + uint length = b.length; + assert(length >= 2); + assert(b[length - 1] == 0); + assert(b[length - 1] == b[length - 2]); + // Fails + assert(b[length - 1] == byte(uint8(1))); + } +} +// ---- +// Warning 6328: (236-275): CHC: Assertion violation happens here.\nCounterexample:\nb = [0, 0]\n\n\n\nTransaction trace:\nconstructor()\nState: b = []\nf() diff --git a/test/libsolidity/smtCheckerTests/array_members/push_as_lhs_bytes.sol b/test/libsolidity/smtCheckerTests/array_members/push_as_lhs_bytes.sol new file mode 100644 index 000000000..fc0dcb3fc --- /dev/null +++ b/test/libsolidity/smtCheckerTests/array_members/push_as_lhs_bytes.sol @@ -0,0 +1,22 @@ +pragma experimental SMTChecker; + +contract C { + bytes b; + + function f() public { + require(b.length == 0); + b.push() = byte(uint8(1)); + assert(b[0] == byte(uint8(1))); + } + + function g() public { + byte one = byte(uint8(1)); + b.push() = one; + assert(b[b.length - 1] == one); + // Fails + assert(b[b.length - 1] == byte(uint8(100))); + } + +} +// ---- +// Warning 6328: (290-333): CHC: Assertion violation happens here.\nCounterexample:\nb = [1]\n\n\n\nTransaction trace:\nconstructor()\nState: b = []\ng() diff --git a/test/libsolidity/smtCheckerTests/array_members/push_as_lhs_bytes_2d.sol b/test/libsolidity/smtCheckerTests/array_members/push_as_lhs_bytes_2d.sol new file mode 100644 index 000000000..22bc28401 --- /dev/null +++ b/test/libsolidity/smtCheckerTests/array_members/push_as_lhs_bytes_2d.sol @@ -0,0 +1,26 @@ +pragma experimental SMTChecker; + +contract C { + bytes[] c; + + function f() public { + bytes1 val = bytes1(uint8(2)); + require(c.length == 0); + c.push().push() = val; + assert(c.length == 1); + assert(c[0].length == 1); + assert(c[0][0] == val); + } + + function g() public { + bytes1 val = bytes1(uint8(2)); + c.push().push() = val; + assert(c.length > 0); + assert(c[c.length - 1].length == 1); + assert(c[c.length - 1][c[c.length - 1].length - 1] == val); + // Fails + assert(c[c.length - 1][c[c.length - 1].length - 1] == bytes1(uint8(100))); + } +} +// ---- +// Warning 6328: (468-541): CHC: Assertion violation happens here.\nCounterexample:\nc = [[2]]\n\n\n\nTransaction trace:\nconstructor()\nState: c = []\ng() From 103fa3b7eb90c521eb91bd01e1c8f45c6be2bb33 Mon Sep 17 00:00:00 2001 From: Martin Blicha Date: Mon, 14 Dec 2020 18:23:07 +0100 Subject: [PATCH 03/10] [SMTChecker] Fix internal error on abstract modifier --- libsolidity/formal/SMTEncoder.cpp | 7 +++++-- .../smtCheckerTests/modifiers/modifier_abstract.sol | 6 ++++++ 2 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 test/libsolidity/smtCheckerTests/modifiers/modifier_abstract.sol diff --git a/libsolidity/formal/SMTEncoder.cpp b/libsolidity/formal/SMTEncoder.cpp index e39827723..b44230674 100644 --- a/libsolidity/formal/SMTEncoder.cpp +++ b/libsolidity/formal/SMTEncoder.cpp @@ -2604,8 +2604,11 @@ vector SMTEncoder::modifiersVariables(FunctionDefini continue; visited.insert(modifier); - vars += applyMap(modifier->parameters(), [](auto _var) { return _var.get(); }); - vars += BlockVars(modifier->body()).vars; + if (modifier->isImplemented()) + { + vars += applyMap(modifier->parameters(), [](auto _var) { return _var.get(); }); + vars += BlockVars(modifier->body()).vars; + } } return vars; } diff --git a/test/libsolidity/smtCheckerTests/modifiers/modifier_abstract.sol b/test/libsolidity/smtCheckerTests/modifiers/modifier_abstract.sol new file mode 100644 index 000000000..c02aa5c66 --- /dev/null +++ b/test/libsolidity/smtCheckerTests/modifiers/modifier_abstract.sol @@ -0,0 +1,6 @@ +pragma experimental SMTChecker; + +abstract contract A { + function f() public mod {} + modifier mod virtual; +} From 0efd52a38e01be049e0eed67a4dcc4421da895cd Mon Sep 17 00:00:00 2001 From: Djordje Mijovic Date: Fri, 11 Dec 2020 17:33:13 +0100 Subject: [PATCH 04/10] Allowing implicit conversion from calldata slice to memory and storage array types. --- libsolidity/ast/Types.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index ed3686b36..d32650383 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -2074,7 +2074,11 @@ std::unique_ptr ArrayType::copyForLocation(DataLocation _location BoolResult ArraySliceType::isImplicitlyConvertibleTo(Type const& _other) const { - if (m_arrayType.location() == DataLocation::CallData && m_arrayType.isDynamicallySized() && m_arrayType == _other) + if ( + m_arrayType.dataStoredIn(DataLocation::CallData) && + m_arrayType.isDynamicallySized() && + m_arrayType.isImplicitlyConvertibleTo(_other) + ) return true; return (*this) == _other; } From b99a74fb343adac02e0003ae170b4050da40e30b Mon Sep 17 00:00:00 2001 From: Djordje Mijovic Date: Fri, 11 Dec 2020 11:06:30 +0100 Subject: [PATCH 05/10] Implementing conversion from calldata slices to memory arrays. --- Changelog.md | 1 + libsolidity/codegen/CompilerUtils.cpp | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Changelog.md b/Changelog.md index c9f4d2318..def607f3a 100644 --- a/Changelog.md +++ b/Changelog.md @@ -3,6 +3,7 @@ Language Features: * Code generator: Support copying dynamically encoded structs from calldata to memory. * Code generator: Support copying of nested arrays from calldata to memory. + * Code generator: Support conversion from calldata slices to memory and storage arrays. * The fallback function can now also have a single ``calldata`` argument (equaling ``msg.data``) and return ``bytes memory`` (which will not be ABI-encoded but returned as-is). * Wasm backend: Add ``i32.select`` and ``i64.select`` instructions. diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp index ea6a84d1d..407c1037a 100644 --- a/libsolidity/codegen/CompilerUtils.cpp +++ b/libsolidity/codegen/CompilerUtils.cpp @@ -1051,11 +1051,11 @@ void CompilerUtils::convertType( case Type::Category::ArraySlice: { auto& typeOnStack = dynamic_cast(_typeOnStack); - solUnimplementedAssert( - _targetType.dataStoredIn(DataLocation::CallData), - "Conversion from calldata slices to memory not yet implemented." - ); - solAssert(_targetType == typeOnStack.arrayType(), ""); + solAssert(_targetType.category() == Type::Category::Array, ""); + auto const& targetArrayType = dynamic_cast(_targetType); + solAssert(targetArrayType == *typeOnStack.arrayType().copyForLocation(targetArrayType.location(), false), ""); + if (!_targetType.dataStoredIn(DataLocation::CallData)) + return convertType(typeOnStack.arrayType(), _targetType); solUnimplementedAssert( typeOnStack.arrayType().location() == DataLocation::CallData && typeOnStack.arrayType().isDynamicallySized() && From 8aa4568b10e6bde427fd5e84dfa21c01e74c78d9 Mon Sep 17 00:00:00 2001 From: Djordje Mijovic Date: Fri, 11 Dec 2020 11:07:22 +0100 Subject: [PATCH 06/10] [Sol->Yul] Implementing conversion from calldata slices to memory arrays. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Kamil Śliwak --- libsolidity/ast/Types.cpp | 14 +++++++------- libsolidity/codegen/YulUtilFunctions.cpp | 7 +++++-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index d32650383..5c7c91126 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -2074,13 +2074,13 @@ std::unique_ptr ArrayType::copyForLocation(DataLocation _location BoolResult ArraySliceType::isImplicitlyConvertibleTo(Type const& _other) const { - if ( - m_arrayType.dataStoredIn(DataLocation::CallData) && - m_arrayType.isDynamicallySized() && - m_arrayType.isImplicitlyConvertibleTo(_other) - ) - return true; - return (*this) == _other; + return + (*this) == _other || + ( + m_arrayType.dataStoredIn(DataLocation::CallData) && + m_arrayType.isDynamicallySized() && + m_arrayType.isImplicitlyConvertibleTo(_other) + ); } string ArraySliceType::richIdentifier() const diff --git a/libsolidity/codegen/YulUtilFunctions.cpp b/libsolidity/codegen/YulUtilFunctions.cpp index db92a7a0a..6c1784551 100644 --- a/libsolidity/codegen/YulUtilFunctions.cpp +++ b/libsolidity/codegen/YulUtilFunctions.cpp @@ -2852,8 +2852,8 @@ string YulUtilFunctions::conversionFunction(Type const& _from, Type const& _to) solAssert(_from.dataStoredIn(DataLocation::CallData), ""); solAssert(_to.category() == Type::Category::Array, ""); - ArraySliceType const& fromType = dynamic_cast(_from); - ArrayType const& targetType = dynamic_cast(_to); + auto const& fromType = dynamic_cast(_from); + auto const& targetType = dynamic_cast(_to); solAssert(!fromType.arrayType().baseType()->isDynamicallyEncoded(), ""); solAssert( @@ -2861,6 +2861,9 @@ string YulUtilFunctions::conversionFunction(Type const& _from, Type const& _to) "Converting arrays of different type is not possible" ); + if (!targetType.dataStoredIn(DataLocation::CallData)) + return arrayConversionFunction(fromType.arrayType(), targetType); + string const functionName = "convert_" + _from.identifier() + From 71f835b71b1fd04cb9cde77d0f07f05d0fcd6198 Mon Sep 17 00:00:00 2001 From: Martin Blicha Date: Mon, 14 Dec 2020 14:11:54 +0100 Subject: [PATCH 07/10] [SMTChecker] Fixed internal error when increment/decrement is applied on a result of push(). --- libsolidity/formal/SMTEncoder.cpp | 32 +++++++++++-------- .../operators/unary_add_array_push_1.sol | 13 ++++++++ .../operators/unary_add_array_push_2.sol | 11 +++++++ 3 files changed, 42 insertions(+), 14 deletions(-) create mode 100644 test/libsolidity/smtCheckerTests/operators/unary_add_array_push_1.sol create mode 100644 test/libsolidity/smtCheckerTests/operators/unary_add_array_push_2.sol diff --git a/libsolidity/formal/SMTEncoder.cpp b/libsolidity/formal/SMTEncoder.cpp index b44230674..cf02ca77e 100644 --- a/libsolidity/formal/SMTEncoder.cpp +++ b/libsolidity/formal/SMTEncoder.cpp @@ -507,18 +507,21 @@ void SMTEncoder::endVisit(UnaryOperation const& _op) { solAssert(smt::isInteger(*type) || smt::isFixedPoint(*type), ""); solAssert(subExpr->annotation().willBeWrittenTo, ""); + auto computeNewValue = [&](auto currentValue) { + return arithmeticOperation( + _op.getOperator() == Token::Inc ? Token::Add : Token::Sub, + currentValue, + smtutil::Expression(size_t(1)), + _op.annotation().type, + _op + ).first; + }; if (auto identifier = dynamic_cast(subExpr)) { auto decl = identifierToVariable(*identifier); solAssert(decl, ""); auto innerValue = currentValue(*decl); - auto newValue = arithmeticOperation( - _op.getOperator() == Token::Inc ? Token::Add : Token::Sub, - innerValue, - smtutil::Expression(size_t(1)), - _op.annotation().type, - _op - ).first; + auto newValue = computeNewValue(innerValue); defineExpr(_op, _op.isPrefixOperation() ? newValue : innerValue); assignment(*decl, newValue); } @@ -528,16 +531,17 @@ void SMTEncoder::endVisit(UnaryOperation const& _op) ) { auto innerValue = expr(*subExpr); - auto newValue = arithmeticOperation( - _op.getOperator() == Token::Inc ? Token::Add : Token::Sub, - innerValue, - smtutil::Expression(size_t(1)), - _op.annotation().type, - _op - ).first; + auto newValue = computeNewValue(innerValue); defineExpr(_op, _op.isPrefixOperation() ? newValue : innerValue); indexOrMemberAssignment(*subExpr, newValue); } + else if (isEmptyPush(*subExpr)) + { + auto innerValue = expr(*subExpr); + auto newValue = computeNewValue(innerValue); + defineExpr(_op, _op.isPrefixOperation() ? newValue : innerValue); + arrayPushPopAssign(*subExpr, newValue); + } else solAssert(false, ""); diff --git a/test/libsolidity/smtCheckerTests/operators/unary_add_array_push_1.sol b/test/libsolidity/smtCheckerTests/operators/unary_add_array_push_1.sol new file mode 100644 index 000000000..db823c29e --- /dev/null +++ b/test/libsolidity/smtCheckerTests/operators/unary_add_array_push_1.sol @@ -0,0 +1,13 @@ +pragma experimental SMTChecker; +contract C { + uint[] x; + function f() public { + require(x.length == 0); + ++x.push(); + assert(x.length == 1); + assert(x[0] == 1); // should hold + assert(x[0] == 42); // should fail + } +} +// ---- +// Warning 6328: (182-200): CHC: Assertion violation happens here.\nCounterexample:\nx = [1]\n\n\n\nTransaction trace:\nconstructor()\nState: x = []\nf() diff --git a/test/libsolidity/smtCheckerTests/operators/unary_add_array_push_2.sol b/test/libsolidity/smtCheckerTests/operators/unary_add_array_push_2.sol new file mode 100644 index 000000000..1aaa8a80f --- /dev/null +++ b/test/libsolidity/smtCheckerTests/operators/unary_add_array_push_2.sol @@ -0,0 +1,11 @@ +pragma experimental SMTChecker; + +contract C { + struct S { + int[][] d; + } + S[] data; + function f() public { + ++data[1].d[3].push(); + } +} From d479c98920ae526857a6f638be70ae6c4bc3f520 Mon Sep 17 00:00:00 2001 From: Djordje Mijovic Date: Fri, 11 Dec 2020 11:07:55 +0100 Subject: [PATCH 08/10] Adding tests for conversion of calldata slices to memory and storage and fixing syntax tests. --- ...ta_array_as_argument_internal_function.sol | 19 +++++++++ ...calldata_as_argument_of_external_calls.sol | 39 +++++++++++++++++++ .../slices/array_slice_calldata_to_memory.sol | 29 ++++++++++++++ .../array_slice_calldata_to_storage.sol | 16 ++++++++ .../array/slice/assign_to_storage.sol | 1 - .../calldata_dynamic_convert_to_memory.sol | 8 ++-- .../array/slice/calldata_dynamic_forward.sol | 1 - 7 files changed, 106 insertions(+), 7 deletions(-) create mode 100644 test/libsolidity/semanticTests/array/calldata_array_as_argument_internal_function.sol create mode 100644 test/libsolidity/semanticTests/array/slices/array_slice_calldata_as_argument_of_external_calls.sol create mode 100644 test/libsolidity/semanticTests/array/slices/array_slice_calldata_to_memory.sol create mode 100644 test/libsolidity/semanticTests/array/slices/array_slice_calldata_to_storage.sol diff --git a/test/libsolidity/semanticTests/array/calldata_array_as_argument_internal_function.sol b/test/libsolidity/semanticTests/array/calldata_array_as_argument_internal_function.sol new file mode 100644 index 000000000..21085a7d0 --- /dev/null +++ b/test/libsolidity/semanticTests/array/calldata_array_as_argument_internal_function.sol @@ -0,0 +1,19 @@ +pragma abicoder v2; +contract Test { + function f(uint256[] calldata c) internal returns (uint a, uint b) { + return (c.length, c[0]); + } + + function g(uint256[] calldata c) external returns (uint a, uint b) { + return f(c); + } + + function h(uint256[] calldata c, uint start, uint end) external returns (uint a, uint b) { + return f(c[start: end]); + } +} +// ==== +// compileViaYul: also +// ---- +// g(uint256[]): 0x20, 4, 1, 2, 3, 4 -> 4, 1 +// h(uint256[], uint256, uint256): 0x60, 1, 3, 4, 1, 2, 3, 4 -> 2, 2 diff --git a/test/libsolidity/semanticTests/array/slices/array_slice_calldata_as_argument_of_external_calls.sol b/test/libsolidity/semanticTests/array/slices/array_slice_calldata_as_argument_of_external_calls.sol new file mode 100644 index 000000000..1f5e929d4 --- /dev/null +++ b/test/libsolidity/semanticTests/array/slices/array_slice_calldata_as_argument_of_external_calls.sol @@ -0,0 +1,39 @@ +contract C { + function f1(bytes calldata c1, uint256 s, uint256 e, bytes calldata c2) public returns (bool) { + return keccak256(c1[s:e]) == keccak256(c2); + } + + function f2(bytes calldata c, uint256 s) public returns (uint256, bytes memory) { + return abi.decode(c[s:], (uint256, bytes)); + } + + function f3(bytes calldata c1, uint256 s, uint256 e, bytes calldata c2) public returns (bool) { + bytes memory a = abi.encode(c1[s:e]); + bytes memory b = abi.encode(c2); + if (a.length != b.length) { return false; } + for (uint256 i = 0; i < a.length; i++) { + if (a[i] != b[i]) { return false; } + } + return true; + } + + function f4(bytes calldata c1, uint256 s, uint256 e, bytes calldata c2) public returns (bool) { + bytes memory a = abi.encodePacked(c1[s:e]); + bytes memory b = abi.encodePacked(c2); + if (a.length != b.length) { return false; } + for (uint256 i = 0; i < a.length; i++) { + if (a[i] != b[i]) { return false; } + } + return true; + } +} +// ==== +// compileViaYul: also +// ---- +// f1(bytes,uint256,uint256,bytes): 0x80, 1, 5, 0xC0, 8, "abcdefgh", 4, "bcde" -> true +// f1(bytes,uint256,uint256,bytes): 0x80, 1, 5, 0xC0, 8, "abcdefgh", 4, "bcdf" -> false +// f2(bytes,uint256): 0x40, 0, 0x80, 0x21, 0x40, 0x7, "abcdefg" -> 0x21, 0x40, 0x7, "abcdefg" +// f3(bytes,uint256,uint256,bytes): 0x80, 1, 5, 0xC0, 8, "abcdefgh", 4, "bcde" -> true +// f3(bytes,uint256,uint256,bytes): 0x80, 1, 5, 0xC0, 8, "abcdefgh", 4, "bcdf" -> false +// f4(bytes,uint256,uint256,bytes): 0x80, 1, 5, 0xC0, 8, "abcdefgh", 4, "bcde" -> true +// f4(bytes,uint256,uint256,bytes): 0x80, 1, 5, 0xC0, 8, "abcdefgh", 4, "bcdf" -> false diff --git a/test/libsolidity/semanticTests/array/slices/array_slice_calldata_to_memory.sol b/test/libsolidity/semanticTests/array/slices/array_slice_calldata_to_memory.sol new file mode 100644 index 000000000..6ec8c7900 --- /dev/null +++ b/test/libsolidity/semanticTests/array/slices/array_slice_calldata_to_memory.sol @@ -0,0 +1,29 @@ +contract C { + function f(int[] calldata b, uint256 start, uint256 end) public returns (int) { + int[] memory m = b[start:end]; + uint len = end - start; + assert(len == m.length); + for (uint i = 0; i < len; i++) { + assert(b[start:end][i] == m[i]); + } + return [b[start:end]][0][0]; + } + + function g(int[] calldata b, uint256 start, uint256 end) public returns (int[] memory) { + return b[start:end]; + } + + function h1(int[] memory b) internal returns (int[] memory) { + return b; + } + + function h(int[] calldata b, uint256 start, uint256 end) public returns (int[] memory) { + return h1(b[start:end]); + } +} +// ==== +// compileViaYul: also +// ---- +// f(int256[], uint256, uint256): 0x60, 1, 3, 4, 1, 2, 3, 4 -> 2 +// g(int256[], uint256, uint256): 0x60, 1, 3, 4, 1, 2, 3, 4 -> 0x20, 2, 2, 3 +// h(int256[], uint256, uint256): 0x60, 1, 3, 4, 1, 2, 3, 4 -> 0x20, 2, 2, 3 diff --git a/test/libsolidity/semanticTests/array/slices/array_slice_calldata_to_storage.sol b/test/libsolidity/semanticTests/array/slices/array_slice_calldata_to_storage.sol new file mode 100644 index 000000000..88ad84604 --- /dev/null +++ b/test/libsolidity/semanticTests/array/slices/array_slice_calldata_to_storage.sol @@ -0,0 +1,16 @@ +contract C { + int[] s; + function f(int[] calldata b, uint256 start, uint256 end) public returns (int) { + s = b[start:end]; + uint len = end - start; + assert(len == s.length); + for (uint i = 0; i < len; i++) { + assert(b[start:end][i] == s[i]); + } + return s[0]; + } +} +// ==== +// compileViaYul: also +// ---- +// f(int256[], uint256, uint256): 0x60, 1, 3, 4, 1, 2, 3, 4 -> 2 diff --git a/test/libsolidity/syntaxTests/array/slice/assign_to_storage.sol b/test/libsolidity/syntaxTests/array/slice/assign_to_storage.sol index 12fd82710..df259b294 100644 --- a/test/libsolidity/syntaxTests/array/slice/assign_to_storage.sol +++ b/test/libsolidity/syntaxTests/array/slice/assign_to_storage.sol @@ -5,4 +5,3 @@ contract c { } } // ---- -// TypeError 7407: (63-74): Type bytes calldata slice is not implicitly convertible to expected type bytes storage ref. diff --git a/test/libsolidity/syntaxTests/array/slice/calldata_dynamic_convert_to_memory.sol b/test/libsolidity/syntaxTests/array/slice/calldata_dynamic_convert_to_memory.sol index 76a360754..62cc33c65 100644 --- a/test/libsolidity/syntaxTests/array/slice/calldata_dynamic_convert_to_memory.sol +++ b/test/libsolidity/syntaxTests/array/slice/calldata_dynamic_convert_to_memory.sol @@ -1,7 +1,5 @@ contract C { - function f(bytes calldata x) external { - bytes memory y = x[1:2]; + function f(bytes calldata x) external pure returns (bytes memory) { + return x[1:2]; } -} -// ---- -// TypeError 9574: (65-88): Type bytes calldata slice is not implicitly convertible to expected type bytes memory. +} \ No newline at end of file diff --git a/test/libsolidity/syntaxTests/array/slice/calldata_dynamic_forward.sol b/test/libsolidity/syntaxTests/array/slice/calldata_dynamic_forward.sol index ccb70bbbd..75c42cd4e 100644 --- a/test/libsolidity/syntaxTests/array/slice/calldata_dynamic_forward.sol +++ b/test/libsolidity/syntaxTests/array/slice/calldata_dynamic_forward.sol @@ -4,4 +4,3 @@ contract C { } } // ---- -// TypeError 9553: (79-85): Invalid type for argument in function call. Invalid implicit conversion from bytes calldata slice to bytes memory requested. From 64f0120622ca32177a90e70fd95ff92daeeabe96 Mon Sep 17 00:00:00 2001 From: Djordje Mijovic Date: Mon, 14 Dec 2020 11:48:51 +0100 Subject: [PATCH 09/10] Change assertions to align to TypeChecker. --- libsolidity/codegen/CompilerUtils.cpp | 12 ++++----- libsolidity/codegen/YulUtilFunctions.cpp | 11 ++++---- .../array_slice_calldata_to_calldata.sol | 27 +++++++++++++++++++ 3 files changed, 38 insertions(+), 12 deletions(-) create mode 100644 test/libsolidity/semanticTests/array/slices/array_slice_calldata_to_calldata.sol diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp index 407c1037a..63d169d47 100644 --- a/libsolidity/codegen/CompilerUtils.cpp +++ b/libsolidity/codegen/CompilerUtils.cpp @@ -1050,18 +1050,18 @@ void CompilerUtils::convertType( } case Type::Category::ArraySlice: { - auto& typeOnStack = dynamic_cast(_typeOnStack); solAssert(_targetType.category() == Type::Category::Array, ""); + auto& typeOnStack = dynamic_cast(_typeOnStack); auto const& targetArrayType = dynamic_cast(_targetType); - solAssert(targetArrayType == *typeOnStack.arrayType().copyForLocation(targetArrayType.location(), false), ""); - if (!_targetType.dataStoredIn(DataLocation::CallData)) - return convertType(typeOnStack.arrayType(), _targetType); - solUnimplementedAssert( - typeOnStack.arrayType().location() == DataLocation::CallData && + solAssert(typeOnStack.arrayType().isImplicitlyConvertibleTo(targetArrayType), ""); + solAssert( + typeOnStack.arrayType().dataStoredIn(DataLocation::CallData) && typeOnStack.arrayType().isDynamicallySized() && !typeOnStack.arrayType().baseType()->isDynamicallyEncoded(), "" ); + if (!_targetType.dataStoredIn(DataLocation::CallData)) + return convertType(typeOnStack.arrayType(), _targetType); break; } case Type::Category::Struct: diff --git a/libsolidity/codegen/YulUtilFunctions.cpp b/libsolidity/codegen/YulUtilFunctions.cpp index 6c1784551..95a04adf0 100644 --- a/libsolidity/codegen/YulUtilFunctions.cpp +++ b/libsolidity/codegen/YulUtilFunctions.cpp @@ -2848,17 +2848,16 @@ string YulUtilFunctions::conversionFunction(Type const& _from, Type const& _to) } else if (_from.category() == Type::Category::ArraySlice) { - solAssert(_from.isDynamicallySized(), ""); - solAssert(_from.dataStoredIn(DataLocation::CallData), ""); solAssert(_to.category() == Type::Category::Array, ""); - auto const& fromType = dynamic_cast(_from); auto const& targetType = dynamic_cast(_to); - solAssert(!fromType.arrayType().baseType()->isDynamicallyEncoded(), ""); + solAssert(fromType.arrayType().isImplicitlyConvertibleTo(targetType), ""); solAssert( - *fromType.arrayType().baseType() == *targetType.baseType(), - "Converting arrays of different type is not possible" + fromType.arrayType().dataStoredIn(DataLocation::CallData) && + fromType.arrayType().isDynamicallySized() && + !fromType.arrayType().baseType()->isDynamicallyEncoded(), + "" ); if (!targetType.dataStoredIn(DataLocation::CallData)) diff --git a/test/libsolidity/semanticTests/array/slices/array_slice_calldata_to_calldata.sol b/test/libsolidity/semanticTests/array/slices/array_slice_calldata_to_calldata.sol new file mode 100644 index 000000000..2003dc47f --- /dev/null +++ b/test/libsolidity/semanticTests/array/slices/array_slice_calldata_to_calldata.sol @@ -0,0 +1,27 @@ +pragma abicoder v2; + +contract C { + struct S { + uint128 p1; + uint256[3] a; + uint32 p2; + } + function f(S[] calldata c) internal returns (S[] memory) { + return c; + } + function g(S[] calldata c, uint256 s, uint256 e) public returns (S[] memory) { + return f(c[s:e]); + } + + function f1(uint256[3][] calldata c) internal returns (uint256[3][] memory) { + return c; + } + function g1(uint256[3][] calldata c, uint256 s, uint256 e) public returns (uint256[3][] memory) { + return f1(c[s:e]); + } +} +// ==== +// compileViaYul: also +// ---- +// g((uint128, uint256[3], uint32)[], uint256, uint256): 0x60, 1, 3, 4, 55, 1, 2, 3, 66, 66, 2, 3, 4, 77, 77, 3, 4, 5, 88, 88, 4, 5, 6, 99 -> 0x20, 2, 66, 2, 3, 4, 77, 77, 3, 4, 5, 88 +// g1(uint256[3][], uint256, uint256): 0x60, 1, 3, 4, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12 -> 0x20, 2, 4, 5, 6, 7, 8, 9 From e2c27b8ea412c443037ff1ba1f68e7bcea18ee05 Mon Sep 17 00:00:00 2001 From: Martin Blicha Date: Tue, 15 Dec 2020 09:15:59 +0100 Subject: [PATCH 10/10] [SMTChecker] Fix internal error on constructor of a recursive struct --- libsolidity/formal/SMTEncoder.cpp | 8 +++++-- .../struct/struct_constructor_recursive_1.sol | 13 ++++++++++++ .../struct/struct_constructor_recursive_2.sol | 21 +++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 test/libsolidity/smtCheckerTests/types/struct/struct_constructor_recursive_1.sol create mode 100644 test/libsolidity/smtCheckerTests/types/struct/struct_constructor_recursive_2.sol diff --git a/libsolidity/formal/SMTEncoder.cpp b/libsolidity/formal/SMTEncoder.cpp index cf02ca77e..bd0746550 100644 --- a/libsolidity/formal/SMTEncoder.cpp +++ b/libsolidity/formal/SMTEncoder.cpp @@ -1144,8 +1144,12 @@ void SMTEncoder::visitFunctionIdentifier(Identifier const& _identifier) void SMTEncoder::visitStructConstructorCall(FunctionCall const& _funCall) { solAssert(*_funCall.annotation().kind == FunctionCallKind::StructConstructorCall, ""); - auto& structSymbolicVar = dynamic_cast(*m_context.expression(_funCall)); - structSymbolicVar.assignAllMembers(applyMap(_funCall.sortedArguments(), [this](auto const& arg) { return expr(*arg); })); + if (smt::isNonRecursiveStruct(*_funCall.annotation().type)) + { + auto& structSymbolicVar = dynamic_cast(*m_context.expression(_funCall)); + structSymbolicVar.assignAllMembers(applyMap(_funCall.sortedArguments(), [this](auto const& arg) { return expr(*arg); })); + } + } void SMTEncoder::endVisit(Literal const& _literal) diff --git a/test/libsolidity/smtCheckerTests/types/struct/struct_constructor_recursive_1.sol b/test/libsolidity/smtCheckerTests/types/struct/struct_constructor_recursive_1.sol new file mode 100644 index 000000000..1265adbb9 --- /dev/null +++ b/test/libsolidity/smtCheckerTests/types/struct/struct_constructor_recursive_1.sol @@ -0,0 +1,13 @@ +pragma experimental SMTChecker; +contract Test { + struct RecursiveStruct { + RecursiveStruct[] vals; + } + function func() public pure { + RecursiveStruct[1] memory val = [ RecursiveStruct(new RecursiveStruct[](42)) ]; + } +} +// ---- +// Warning 2072: (136-165): Unused local variable. +// Warning 8364: (170-212): Assertion checker does not yet implement type struct Test.RecursiveStruct memory +// Warning 8364: (170-212): Assertion checker does not yet implement type struct Test.RecursiveStruct memory diff --git a/test/libsolidity/smtCheckerTests/types/struct/struct_constructor_recursive_2.sol b/test/libsolidity/smtCheckerTests/types/struct/struct_constructor_recursive_2.sol new file mode 100644 index 000000000..08194e68b --- /dev/null +++ b/test/libsolidity/smtCheckerTests/types/struct/struct_constructor_recursive_2.sol @@ -0,0 +1,21 @@ +pragma experimental SMTChecker; +contract Test { + struct RecursiveStruct { + uint x; + RecursiveStruct[] vals; + } + function func() public pure { + RecursiveStruct memory val = RecursiveStruct(1, new RecursiveStruct[](42)); + assert(val.x == 1); + } +} +// ---- +// Warning 8115: (146-172): Assertion checker does not yet support the type of this variable. +// Warning 8364: (175-220): Assertion checker does not yet implement type struct Test.RecursiveStruct memory +// Warning 7650: (231-236): Assertion checker does not yet support this expression. +// Warning 8364: (231-234): Assertion checker does not yet implement type struct Test.RecursiveStruct memory +// Warning 6328: (224-242): CHC: Assertion violation happens here.\nCounterexample:\n\n\n\n\nTransaction trace:\nconstructor()\nfunc() +// Warning 8115: (146-172): Assertion checker does not yet support the type of this variable. +// Warning 8364: (175-220): Assertion checker does not yet implement type struct Test.RecursiveStruct memory +// Warning 7650: (231-236): Assertion checker does not yet support this expression. +// Warning 8364: (231-234): Assertion checker does not yet implement type struct Test.RecursiveStruct memory