From 409e92580f76971b0663cb0ba5d9f428a411efee Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 19 Nov 2020 11:00:18 +0100 Subject: [PATCH 1/2] Do not allocate memory objects if they will be assigned directly. --- Changelog.md | 2 ++ libsolidity/codegen/ContractCompiler.cpp | 17 ++++++++--- libsolidity/codegen/ContractCompiler.h | 5 +++- .../memoryManagement/assembly_access.sol | 14 +++++++++ .../memoryManagement/return_variable.sol | 30 +++++++++++++++++++ .../static_memory_array_allocation.sol | 24 +++++++++++++++ .../memoryManagement/struct_allocation.sol | 24 +++++++++++++++ .../access_in_assignment_dynamic_array.sol | 7 +++++ .../scoping/access_in_assignment_struct.sol | 8 +++++ 9 files changed, 126 insertions(+), 5 deletions(-) create mode 100644 test/libsolidity/semanticTests/memoryManagement/assembly_access.sol create mode 100644 test/libsolidity/semanticTests/memoryManagement/return_variable.sol create mode 100644 test/libsolidity/semanticTests/memoryManagement/static_memory_array_allocation.sol create mode 100644 test/libsolidity/semanticTests/memoryManagement/struct_allocation.sol create mode 100644 test/libsolidity/syntaxTests/scoping/access_in_assignment_dynamic_array.sol create mode 100644 test/libsolidity/syntaxTests/scoping/access_in_assignment_struct.sol diff --git a/Changelog.md b/Changelog.md index e113a9997..c51fc2a54 100644 --- a/Changelog.md +++ b/Changelog.md @@ -4,9 +4,11 @@ Language Features: * 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). Compiler Features: + * Code Generator: Avoid memory allocation for default value if it is not used. * SMTChecker: Support named arguments in function calls. * SMTChecker: Support struct constructor. + ### 0.7.5 (2020-11-18) Language Features: diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 66c67f59a..a73166507 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -612,7 +612,7 @@ bool ContractCompiler::visit(FunctionDefinition const& _function) } for (ASTPointer const& variable: _function.returnParameters()) - appendStackVariableInitialisation(*variable); + appendStackVariableInitialisation(*variable, /* _provideDefaultValue = */ true); if (_function.isConstructor()) if (auto c = dynamic_cast(*_function.scope()).nextConstructor( @@ -1230,7 +1230,7 @@ bool ContractCompiler::visit(VariableDeclarationStatement const& _variableDeclar // and freed in the end of their scope. for (auto decl: _variableDeclarationStatement.declarations()) if (decl) - appendStackVariableInitialisation(*decl); + appendStackVariableInitialisation(*decl, !_variableDeclarationStatement.initialValue()); StackHeightChecker checker(m_context); if (Expression const* expression = _variableDeclarationStatement.initialValue()) @@ -1376,11 +1376,20 @@ void ContractCompiler::appendModifierOrFunctionCode() m_context.setModifierDepth(m_modifierDepth); } -void ContractCompiler::appendStackVariableInitialisation(VariableDeclaration const& _variable) +void ContractCompiler::appendStackVariableInitialisation( + VariableDeclaration const& _variable, + bool _provideDefaultValue +) { CompilerContext::LocationSetter location(m_context, _variable); m_context.addVariable(_variable); - CompilerUtils(m_context).pushZeroValue(*_variable.annotation().type); + if (!_provideDefaultValue && _variable.type()->dataStoredIn(DataLocation::Memory)) + { + solAssert(_variable.type()->sizeOnStack() == 1, ""); + m_context << u256(0); + } + else + CompilerUtils(m_context).pushZeroValue(*_variable.annotation().type); } void ContractCompiler::compileExpression(Expression const& _expression, TypePointer const& _targetType) diff --git a/libsolidity/codegen/ContractCompiler.h b/libsolidity/codegen/ContractCompiler.h index 64c86fc2d..d3290a4c7 100644 --- a/libsolidity/codegen/ContractCompiler.h +++ b/libsolidity/codegen/ContractCompiler.h @@ -130,7 +130,10 @@ private: /// body itself if the last modifier was reached. void appendModifierOrFunctionCode(); - void appendStackVariableInitialisation(VariableDeclaration const& _variable); + /// Creates a stack slot for the given variable and assigns a default value. + /// If the default value is complex (needs memory allocation) and @a _provideDefaultValue + /// is false, this might be skipped. + void appendStackVariableInitialisation(VariableDeclaration const& _variable, bool _provideDefaultValue); void compileExpression(Expression const& _expression, TypePointer const& _targetType = TypePointer()); /// Frees the variables of a certain scope (to be used when leaving). diff --git a/test/libsolidity/semanticTests/memoryManagement/assembly_access.sol b/test/libsolidity/semanticTests/memoryManagement/assembly_access.sol new file mode 100644 index 000000000..449edc304 --- /dev/null +++ b/test/libsolidity/semanticTests/memoryManagement/assembly_access.sol @@ -0,0 +1,14 @@ +contract C { + function f() public pure { + uint[] memory x; + uint y; + assembly { + y := x + } + assert(y != 0); + } +} +// ==== +// compileViaYul: also +// ---- +// f() -> diff --git a/test/libsolidity/semanticTests/memoryManagement/return_variable.sol b/test/libsolidity/semanticTests/memoryManagement/return_variable.sol new file mode 100644 index 000000000..501fea0f1 --- /dev/null +++ b/test/libsolidity/semanticTests/memoryManagement/return_variable.sol @@ -0,0 +1,30 @@ +contract C { + function memorySize() internal pure returns (uint s) { + assembly { s := mload(0x40) } + } + function f() public returns (uint, uint, uint) { + uint a = memorySize(); + g(); + uint b = memorySize(); + h(); + uint c = memorySize(); + i(); + uint d = memorySize(); + return (b - a, c - b, d - c); + } + // In these functions, we do allocate memory in both cases. + // In `i()`, this could be avoided but we would have to check + // that all code paths return explicitly and provide a value. + function g() internal returns (uint[40] memory) { + } + function h() internal returns (uint[40] memory t) { + } + function i() internal returns (uint[40] memory) { + uint[40] memory x; + return x; + } +} +// ==== +// compileViaYul: also +// ---- +// f() -> 0x0500, 0x0500, 0x0a00 diff --git a/test/libsolidity/semanticTests/memoryManagement/static_memory_array_allocation.sol b/test/libsolidity/semanticTests/memoryManagement/static_memory_array_allocation.sol new file mode 100644 index 000000000..868d532b8 --- /dev/null +++ b/test/libsolidity/semanticTests/memoryManagement/static_memory_array_allocation.sol @@ -0,0 +1,24 @@ +contract C { + function memorySize() internal pure returns (uint s) { + assembly { s := mload(0x40) } + } + function withValue() public pure returns (uint) { + uint[20] memory x; + uint memorySizeBefore = memorySize(); + uint[20] memory t = x; + uint memorySizeAfter = memorySize(); + return memorySizeAfter - memorySizeBefore; + } + function withoutValue() public pure returns (uint) { + uint[20] memory x; + uint memorySizeBefore = memorySize(); + uint[20] memory t; + uint memorySizeAfter = memorySize(); + return memorySizeAfter - memorySizeBefore; + } +} +// ==== +// compileViaYul: also +// ---- +// withValue() -> 0x00 +// withoutValue() -> 0x0280 diff --git a/test/libsolidity/semanticTests/memoryManagement/struct_allocation.sol b/test/libsolidity/semanticTests/memoryManagement/struct_allocation.sol new file mode 100644 index 000000000..68a52182b --- /dev/null +++ b/test/libsolidity/semanticTests/memoryManagement/struct_allocation.sol @@ -0,0 +1,24 @@ +contract C { + struct S { uint x; uint y; uint z; } + function memorySize() internal pure returns (uint s) { + assembly { s := mload(0x40) } + } + function withValue() public pure returns (uint) { + S memory x = S(1, 2, 3); + uint memorySizeBefore = memorySize(); + S memory t = x; + uint memorySizeAfter = memorySize(); + return memorySizeAfter - memorySizeBefore; + } + function withoutValue() public pure returns (uint) { + uint memorySizeBefore = memorySize(); + S memory t; + uint memorySizeAfter = memorySize(); + return memorySizeAfter - memorySizeBefore; + } +} +// ==== +// compileViaYul: also +// ---- +// withValue() -> 0x00 +// withoutValue() -> 0x60 diff --git a/test/libsolidity/syntaxTests/scoping/access_in_assignment_dynamic_array.sol b/test/libsolidity/syntaxTests/scoping/access_in_assignment_dynamic_array.sol new file mode 100644 index 000000000..12454b740 --- /dev/null +++ b/test/libsolidity/syntaxTests/scoping/access_in_assignment_dynamic_array.sol @@ -0,0 +1,7 @@ +contract C { + function f() public pure { + uint[] memory x = x[0]; + } +} +// ---- +// DeclarationError 7576: (70-71): Undeclared identifier. "x" is not (or not yet) visible at this point. diff --git a/test/libsolidity/syntaxTests/scoping/access_in_assignment_struct.sol b/test/libsolidity/syntaxTests/scoping/access_in_assignment_struct.sol new file mode 100644 index 000000000..152ee038e --- /dev/null +++ b/test/libsolidity/syntaxTests/scoping/access_in_assignment_struct.sol @@ -0,0 +1,8 @@ +contract C { + struct S { uint y; } + function f() public pure { + S memory x = x.y; + } +} +// ---- +// DeclarationError 7576: (90-91): Undeclared identifier. "x" is not (or not yet) visible at this point. From dbb24484e90484f2f183023a52da5f0d7d0912f4 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 24 Nov 2020 14:39:37 +0100 Subject: [PATCH 2/2] Update test/libsolidity/semanticTests/memoryManagement/assembly_access.sol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Kamil Śliwak --- .../semanticTests/memoryManagement/assembly_access.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/libsolidity/semanticTests/memoryManagement/assembly_access.sol b/test/libsolidity/semanticTests/memoryManagement/assembly_access.sol index 449edc304..15e171dfc 100644 --- a/test/libsolidity/semanticTests/memoryManagement/assembly_access.sol +++ b/test/libsolidity/semanticTests/memoryManagement/assembly_access.sol @@ -5,6 +5,8 @@ contract C { assembly { y := x } + // The value of an uninitialized dynamic array is not zero but rather + // an address of a location in memory that has the value of zero. assert(y != 0); } }