Skip to content

Commit

Permalink
fix off-by-one in dynamic array length calculation (#2821)
Browse files Browse the repository at this point in the history
one slot too few was allocated to dynamic arrays, allowing a write to
the end of the dynamic array to clobber the storage variable allocated
immediately after it.
  • Loading branch information
charles-cooper authored Apr 21, 2022
1 parent 981ec17 commit e991b1f
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 3 deletions.
11 changes: 9 additions & 2 deletions tests/cli/outputs/test_storage_layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ def public_foo2():
def _bar():
pass
arr: DynArray[uint256, 3]
# mix it up a little
baz: Bytes[65]
bar: uint256
Expand Down Expand Up @@ -49,6 +51,11 @@ def public_foo3():
"location": "storage",
"slot": 2,
},
"baz": {"type": "Bytes[65]", "location": "storage", "slot": 3},
"bar": {"type": "uint256", "location": "storage", "slot": 7},
"arr": {
"type": "DynArray[uint256, 3]",
"location": "storage",
"slot": 3,
},
"baz": {"type": "Bytes[65]", "location": "storage", "slot": 7},
"bar": {"type": "uint256", "location": "storage", "slot": 11},
}
10 changes: 10 additions & 0 deletions tests/functional/context/types/test_size_in_bytes.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ def test_array_value_types(build_node, type_str, location, length, size):
assert type_definition.size_in_bytes == size


@pytest.mark.parametrize("type_str", BASE_TYPES)
@pytest.mark.parametrize("location", LOCATIONS)
@pytest.mark.parametrize("length", range(1, 4))
def test_dynamic_array_lengths(build_node, type_str, location, length):
node = build_node(f"DynArray[{type_str}, {length}]")
type_definition = get_type_from_annotation(node, location)

assert type_definition.size_in_bytes == 32 + length * 32


@pytest.mark.parametrize("type_str", BASE_TYPES)
@pytest.mark.parametrize("location", LOCATIONS)
@pytest.mark.parametrize("length", range(1, 4))
Expand Down
2 changes: 2 additions & 0 deletions tests/functional/test_storage_slots.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
c: public(Bytes[32])
d: public(int128[4])
foo: public(HashMap[uint256, uint256[3]])
dyn_array: DynArray[uint256, 3]
e: public(String[47])
f: public(int256[1])
g: public(StructTwo[2])
Expand All @@ -36,6 +37,7 @@ def __init__():
c: "whatifthisstringtakesuptheentirelengthwouldthatbesobadidothinkso"
})
]
self.dyn_array = [1, 2, 3]
self.h = [123456789]
self.foo[0] = [987, 654, 321]
self.foo[1] = [123, 456, 789]
Expand Down
3 changes: 2 additions & 1 deletion vyper/semantics/types/indexable/sequence.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ def is_dynamic_size(self):
# TODO rename me to memory_bytes_required
@property
def size_in_bytes(self):
return self.value_type.size_in_bytes * self.length
# one length word + size of the array items
return 32 + self.value_type.size_in_bytes * self.length

def validate_index_type(self, node):
if isinstance(node, vy_ast.Int):
Expand Down

0 comments on commit e991b1f

Please sign in to comment.