Passing arrays to private and public functions

[robinsierra]

• vyper Version: 0.1.0b9
• Tested with Remix IDE

I'm trying to understand how arrays work in Vyper. It seems that they always get copied on assignment. However, I found the following:

@public
def change_arr(arr: int128[5]):
    a: int128[5] = arr
    a[0] = 24

@public
def call_arr() -> int128:
    a: int128[5]
    a[0] = 42
    self.change_arr(a)
    return a[0]

As expected, calling the function call_arr returns 42. However, if I make the function change_arr private the execution ends in an exception (because of a[0] = 24).

[charles-cooper]

What version of vyper are you on?

[robinsierra]

I tested it with the Vyper plugin in the Remix IDE which seems to be using version 0.1.0b9.

[charles-cooper]

Thanks - you mentioned your execution ends in an exception, what specific exception are you seeing?

[robinsierra]

On closer inspection I found the following: On instruction no 522 (which is an MSTORE) there is only a single value on the operand stack, instead of the required 2.

[charles-cooper]

This seems like a bug. Trying to reproduce it on my end, in the meantime would it be possible for you to post the code which produces this? Or the IR (vyper -f ir) with some context surrounding the MSTORE in question - I just want to be able to correlate the source code with the IR.

[robinsierra]

The code above (with private instead of public) is all that is needed:

@private
def change_arr(arr: int128[5]):
    a: int128[5] = arr
    a[0] = 24

@public
def call_arr() -> int128:
    a: int128[5]
    a[0] = 42
    self.change_arr(a)
    return a[0]

Calling call_arr() triggers the bug. I think the bottom most [mstore, 416, pass] right at the end of line 10 is the one that makes it fail.

IR:

[seq,
  [return,
    0,
    [lll,
      [seq,
        [mstore, 28, [calldataload, 0]],
        [mstore, 32, 1461501637330902918203684832716283019655932542976],
        [mstore, 64, 170141183460469231731687303715884105727],
        [mstore, 96, -170141183460469231731687303715884105728],
        [mstore, 128, 1701411834604692317316873037158841057270000000000],
        [mstore, 160, -1701411834604692317316873037158841057280000000000],
        # Line 1
        [if,
          0,
          [seq,
            /* change_arr(int128[5]) */ [label, priv_3996084614],
            /* pop callback pointer */ [mstore, 480, pass],
            [mstore, 320, pass],
            [mstore, 352, pass],
            [mstore, 384, pass],
            [mstore, 416, pass],
            [mstore, 448, pass],
            [with,
              _L,
              512,
              [with,
                _R,
                '320' <arr>,
                [seq,
                  [mstore,
                    [add, [mul, 32, [uclamplt, 0, 5]], _L],
                    [mload, [add, [mul, 32, [uclamplt, 0, 5]], _R]]],
                  [mstore,
                    [add, [mul, 32, [uclamplt, 1, 5]], _L],
                    [mload, [add, [mul, 32, [uclamplt, 1, 5]], _R]]],
                  [mstore,
                    [add, [mul, 32, [uclamplt, 2, 5]], _L],
                    [mload, [add, [mul, 32, [uclamplt, 2, 5]], _R]]],
                  [mstore,
                    [add, [mul, 32, [uclamplt, 3, 5]], _L],
                    [mload, [add, [mul, 32, [uclamplt, 3, 5]], _R]]],
                  [mstore,
                    [add, [mul, 32, [uclamplt, 4, 5]], _L],
                    [mload, [add, [mul, 32, [uclamplt, 4, 5]], _R]]]]]],
            # Line 4
            [mstore, [add, [mul, 32, [uclamplt, 0, 5]], '512' <a>], 24],
            # Line 1
            [jump, [mload, 480]]]],
        # Line 6
        [if,
          [eq, [mload, 0], '3032955471' <call_arr()>],
          [seq,
            [assert, [iszero, callvalue]],
            # Line 9
            [mstore, [add, [mul, 32, [uclamplt, 0, 5]], '320' <a>], 42],
            # Line 10
            /* Internal Call: change_arr */ 
            [pop,
              [seq_unchecked,
                [mload, 320],
                [mload, 352],
                [mload, 384],
                [mload, 416],
                [mload, 448],
                [seq,
                  [mstore, 480, 3996084614],
                  [with,
                    _L,
                    512,
                    [with,
                      _R,
                      '320' <a>,
                      [seq,
                        [mstore,
                          [add, [mul, 32, [uclamplt, 0, 5]], _L],
                          [mload, [add, [mul, 32, [uclamplt, 0, 5]], _R]]],
                        [mstore,
                          [add, [mul, 32, [uclamplt, 1, 5]], _L],
                          [mload, [add, [mul, 32, [uclamplt, 1, 5]], _R]]],
                        [mstore,
                          [add, [mul, 32, [uclamplt, 2, 5]], _L],
                          [mload, [add, [mul, 32, [uclamplt, 2, 5]], _R]]],
                        [mstore,
                          [add, [mul, 32, [uclamplt, 3, 5]], _L],
                          [mload, [add, [mul, 32, [uclamplt, 3, 5]], _R]]],
                        [mstore,
                          [add, [mul, 32, [uclamplt, 4, 5]], _L],
                          [mload, [add, [mul, 32, [uclamplt, 4, 5]], _R]]]]]]],
                [mstore, 736, 672],
                [label, push_args_3996084614_10_4_start],
                [if, [lt, [mload, 736], 544], [goto, push_args_3996084614_10_4_end]],
                [if_unchecked, [ne, [mload, [mload, 736]], 0], [mload, [mload, 736]]],
                [mstore, 736, [sub, [mload, 736], 32]],
                [goto, push_args_3996084614_10_4_start],
                [label, push_args_3996084614_10_4_end],
                [mload, 512],
                [add, pc, 6],
                [goto, priv_3996084614],
                jumpdest,
                [mstore, 448, pass],
                [mstore, 416, pass],                                                                  
                [mstore, 384, pass],
                [mstore, 352, pass],
                [mstore, 320, pass],
                0]],
            # Line 11
            [mstore, 0, [mload, [add, [mul, 32, [uclamplt, 0, 5]], '320' <a>]]],
            [return, 0, 32],
            # Line 6
            stop]],
        /* Default function */ [revert, 0, 0]],
      0]]]

[charles-cooper]

This seems to be a bug in argument packing (the inner assignment is not needed to trip the issue). The following is a minimal difference between the non-error and error cases

@private
def change_arr(arr: int128[2]):
    pass
@public
def call_arr() -> int128:
-    a: int128[2] = [42,1] # pass
+    a: int128[2]          # fail
    self.change_arr(a)
    return 42

[charles-cooper]

An even more subtle test case:

@private
def change_arr(arr: int128[2]):
    pass
@public
def call_arr() -> int128:
    a: int128[2]
+    a[0] = 37 # fail
-    a[1] = 37 # pass
    self.change_arr(a)
    return 42

Difference in IR:

<             [mstore, [add, [mul, 32, [uclamplt, 1, 2]], '320' <a>], 37],
---
>             [mstore, [add, [mul, 32, [uclamplt, 0, 2]], '320' <a>], 37],

[fubuloubu]

Security Alert: if you use arrays as parameters to a private function, on released versions starting at v0.1.0-beta.4 (introduced by #1012) it could possibly cause a 'out of stack items' reversion. This could be used in a Denial of Service attack, under certain scenarios where the attacker is able to manipulate the stack items. More severe versions of this vulnerability can be used by an attacker to access arbitrary memory if the right conditions make that possible.

You should upgrade to v0.1.0-beta.10 to fix this issue.