Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Sol2Yul] Fixed an ICE on struct member copy #12579

Merged
merged 1 commit into from Jan 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Changelog.md
Expand Up @@ -18,6 +18,7 @@ Bugfixes:
* Immutables: Fix wrong error when the constructor of a base contract uses ``return`` and the parent contract contains immutable variables.
* IR Generator: Add missing cleanup during the conversion of fixed bytes types to smaller fixed bytes types.
* IR Generator: Add missing cleanup for indexed event arguments of value type.
* IR Generator: Fix internal error when copying reference types in calldata and storage to struct or array members in memory.
* IR Generator: Fix IR syntax error when copying storage arrays of structs containing functions.
* Natspec: Fix ICE when overriding a struct getter with a Natspec-documented return value and the name in the struct is different.
* TypeChecker: Fix ICE when a constant variable declaration forward references a struct.
Expand Down
7 changes: 5 additions & 2 deletions libsolidity/codegen/ir/IRGeneratorForStatements.cpp
Expand Up @@ -2985,8 +2985,11 @@ void IRGeneratorForStatements::writeToLValue(IRLValue const& _lvalue, IRVariable
{
solAssert(_lvalue.type.sizeOnStack() == 1);
auto const* valueReferenceType = dynamic_cast<ReferenceType const*>(&_value.type());
solAssert(valueReferenceType && valueReferenceType->dataStoredIn(DataLocation::Memory));
appendCode() << "mstore(" + _memory.address + ", " + _value.part("mpos").name() + ")\n";
solAssert(valueReferenceType);
if (valueReferenceType->dataStoredIn(DataLocation::Memory))
appendCode() << "mstore(" + _memory.address + ", " + _value.part("mpos").name() + ")\n";
else
appendCode() << "mstore(" + _memory.address + ", " + m_utils.conversionFunction(_value.type(), _lvalue.type) + "(" + _value.commaSeparatedList() + "))\n";
bshastry marked this conversation as resolved.
Show resolved Hide resolved
}
},
[&](IRLValue::Stack const& _stack) { assign(_stack.variable, _value); },
Expand Down
38 changes: 38 additions & 0 deletions test/libsolidity/semanticTests/structs/copy_from_calldata.sol
@@ -0,0 +1,38 @@
// Example from https://github.com/ethereum/solidity/issues/12558
pragma abicoder v2;
contract C {
function f(uint[] calldata a) external returns (uint[][] memory) {
uint[][] memory m = new uint[][](2);
m[0] = a;

return m;
}
}
contract Test {
C immutable c = new C();

function test() external returns (bool) {
uint[] memory arr = new uint[](4);

arr[0] = 13;
arr[1] = 14;
arr[2] = 15;
arr[3] = 16;

uint[][] memory ret = c.f(arr);
assert(ret.length == 2);
assert(ret[0].length == 4);
assert(ret[0][0] == 13);
assert(ret[0][1] == 14);
assert(ret[0][2] == 15);
assert(ret[0][3] == 16);
assert(ret[1].length == 0);

return true;
}
}
// ====
// EVMVersion: >homestead
// compileViaYul: also
// ----
// test() -> true
24 changes: 24 additions & 0 deletions test/libsolidity/semanticTests/structs/copy_from_storage.sol
@@ -0,0 +1,24 @@
pragma abicoder v2;
// Example from https://github.com/ethereum/solidity/issues/12558
struct S {
uint x;
}

contract C {
S sStorage;
constructor() {
sStorage.x = 13;
}

function f() external returns (S[] memory) {
S[] memory sMemory = new S[](1);

sMemory[0] = sStorage;

return sMemory;
}
}
// ====
// compileViaYul: also
// ----
// f() -> 0x20, 1, 13
@@ -0,0 +1,96 @@
pragma abicoder v2;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this example: @ekpyron

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this one surprisingly also compiles in other versions. So independent of the fix.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... I'm pretty sure that there were cases that didn't work in other versions... let me see...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

contract C {
        uint256[] x;

        function f() public {
                uint256[][] memory y = new uint256[][](1);
                y[0] = x;
        }
}

crashes on 0.8.11 with --experimental-via-ir --bin for me...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, this test case you added also does, so it's fine - but it's not independent of the fix :-).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, my bad. Forgot to add --ir flag to the test. So yeah, it does crash!

struct S { uint value; }

contract Test {
S[][] a;
S[] b;

constructor() {
a.push();
a[0].push(S(1));
a[0].push(S(2));
a[0].push(S(3));

b.push(S(4));
b.push(S(5));
b.push(S(6));
b.push(S(7));
}

function test1() external returns (bool) {
a.push();
a[1] = b;

assert(a.length == 2);
assert(a[0].length == 3);
assert(a[1].length == 4);
assert(a[1][0].value == 4);
assert(a[1][1].value == 5);
assert(a[1][2].value == 6);
assert(a[1][3].value == 7);

return true;
}

function test2() external returns (bool) {
S[][] memory temp = new S[][](2);

temp = a;

assert(temp.length == 2);
assert(temp[0].length == 3);
assert(temp[1].length == 4);
assert(temp[1][0].value == 4);
assert(temp[1][1].value == 5);
assert(temp[1][2].value == 6);
assert(temp[1][3].value == 7);

return true;
}

function test3() external returns (bool) {
S[][] memory temp = new S[][](2);

temp[0] = a[0];
temp[1] = a[1];

assert(temp.length == 2);
assert(temp[0].length == 3);
assert(temp[1].length == 4);
assert(temp[1][0].value == 4);
assert(temp[1][1].value == 5);
assert(temp[1][2].value == 6);
assert(temp[1][3].value == 7);

return true;
}

function test4() external returns (bool) {
S[][] memory temp = new S[][](2);

temp[0] = a[0];
temp[1] = b;

assert(temp.length == 2);
assert(temp[0].length == 3);
assert(temp[1].length == 4);
assert(temp[1][0].value == 4);
assert(temp[1][1].value == 5);
assert(temp[1][2].value == 6);
assert(temp[1][3].value == 7);

return true;
}
}
// ====
// EVMVersion: >homestead
// compileViaYul: also
// ----
// test1() -> true
// gas irOptimized: 150618
// gas legacy: 150266
// gas legacyOptimized: 149875
// test2() -> true
// test3() -> true
// test4() -> true
44 changes: 44 additions & 0 deletions test/libsolidity/semanticTests/structs/function_type_copy.sol
@@ -0,0 +1,44 @@
pragma abicoder v2;
struct S {
function () external[] functions;
}

contract C {
function f(function () external[] calldata functions) external returns (S memory) {
S memory s;
s.functions = functions;
return s;
}
}

contract Test {
C immutable c = new C();

function test() external returns (bool) {
function() external[] memory functions = new function() external[](3);

functions[0] = this.random1;
functions[1] = this.random2;
functions[2] = this.random3;

S memory ret = c.f(functions);

assert(ret.functions.length == 3);
assert(ret.functions[0] == this.random1);
assert(ret.functions[1] == this.random2);
assert(ret.functions[2] == this.random3);

return true;
}
function random1() external {
}
function random2() external {
}
function random3() external {
}
}
// ====
// EVMVersion: >homestead
// compileViaYul: also
// ----
// test() -> true
@@ -0,0 +1,45 @@
pragma abicoder v2;

struct St0 {
bytes el0;
}
contract C {
function f() external returns (St0 memory) {
St0 memory x;
x.el0 = msg.data;
return x;
}

function g() external returns (St0 memory) {
bytes memory temp = msg.data;
St0 memory x;
x.el0 = temp;
return x;
}

function hashes() external returns (bytes4, bytes4) {
return (this.f.selector, this.g.selector);
}

function large(uint256, uint256, uint256, uint256) external returns (St0 memory) {
St0 memory x;
x.el0 = msg.data;
return x;
}

function another_large(uint256, uint256, uint256, uint256) external returns (St0 memory) {
bytes memory temp = msg.data;
St0 memory x;
x.el0 = temp;
return x;
}

}
// ====
// compileViaYul: also
// ----
// f() -> 0x20, 0x20, 4, 0x26121ff000000000000000000000000000000000000000000000000000000000
// g() -> 0x20, 0x20, 4, 0xe2179b8e00000000000000000000000000000000000000000000000000000000
// hashes() -> 0x26121ff000000000000000000000000000000000000000000000000000000000, 0xe2179b8e00000000000000000000000000000000000000000000000000000000
// large(uint256,uint256,uint256,uint256): 1, 2, 3, 4 -> 0x20, 0x20, 0x84, 0xe02492f800000000000000000000000000000000000000000000000000000000, 0x100000000000000000000000000000000000000000000000000000000, 0x200000000000000000000000000000000000000000000000000000000, 0x300000000000000000000000000000000000000000000000000000000, 0x400000000000000000000000000000000000000000000000000000000
// another_large(uint256,uint256,uint256,uint256): 1, 2, 3, 4 -> 0x20, 0x20, 0x84, 0x2a46f85a00000000000000000000000000000000000000000000000000000000, 0x100000000000000000000000000000000000000000000000000000000, 0x200000000000000000000000000000000000000000000000000000000, 0x300000000000000000000000000000000000000000000000000000000, 0x400000000000000000000000000000000000000000000000000000000
@@ -0,0 +1,9 @@
// Example from https://github.com/ethereum/solidity/issues/12558
pragma abicoder v2;
contract C {
function() external[1][] s0;
constructor(function() external[1][] memory i0)
{
i0[0] = s0[1];
}
}