Skip to content

Commit

Permalink
fix: expand memory on reading prev. untouched location (#1887)
Browse files Browse the repository at this point in the history
* fix: expand memory on reading prev. untouched location

Memory is expanded by word when accessing previously untouched memory word ([relevant docs](https://docs.soliditylang.org/en/v0.8.13/introduction-to-smart-contracts.html#storage-memory-and-the-stack)). That applies to read operation on memory too.

* fix: properly auto-expand memory on read/write

* test: add tests for memory expansion on access

Removed a couple of tests for write beyond capacity.
This is because memory is now auto expanded during write.
  • Loading branch information
nvnx7 authored May 23, 2022
1 parent bdfbe37 commit 0b7cc1b
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 14 deletions.
4 changes: 4 additions & 0 deletions packages/vm/src/evm/memory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ export default class Memory {
return
}

this.extend(offset, size)

assert(value.length === size, 'Invalid value size')
assert(offset + size <= this._store.length, 'Value exceeds memory capacity')
assert(Buffer.isBuffer(value), 'Invalid value type')
Expand All @@ -63,6 +65,8 @@ export default class Memory {
* @param size - How many bytes to read
*/
read(offset: number, size: number): Buffer {
this.extend(offset, size)

const returnBuffer = Buffer.allocUnsafe(size)
// Copy the stored "buffer" from memory into the return Buffer

Expand Down
9 changes: 2 additions & 7 deletions packages/vm/src/evm/opcodes/functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,6 @@ export const handlers: Map<number, OpHandler> = new Map([
const data = getDataSlice(runState.eei.getCallData(), dataOffset, dataLength)
const memOffsetNum = memOffset.toNumber()
const dataLengthNum = dataLength.toNumber()
runState.memory.extend(memOffsetNum, dataLengthNum)
runState.memory.write(memOffsetNum, dataLengthNum, data)
}
},
Expand All @@ -480,7 +479,6 @@ export const handlers: Map<number, OpHandler> = new Map([
const data = getDataSlice(runState.eei.getCode(), codeOffset, dataLength)
const memOffsetNum = memOffset.toNumber()
const lengthNum = dataLength.toNumber()
runState.memory.extend(memOffsetNum, lengthNum)
runState.memory.write(memOffsetNum, lengthNum, data)
}
},
Expand All @@ -506,7 +504,6 @@ export const handlers: Map<number, OpHandler> = new Map([
const data = getDataSlice(code, codeOffset, dataLength)
const memOffsetNum = memOffset.toNumber()
const lengthNum = dataLength.toNumber()
runState.memory.extend(memOffsetNum, lengthNum)
runState.memory.write(memOffsetNum, lengthNum, data)
}
},
Expand Down Expand Up @@ -549,7 +546,6 @@ export const handlers: Map<number, OpHandler> = new Map([
const data = getDataSlice(runState.eei.getReturnData(), returnDataOffset, dataLength)
const memOffsetNum = memOffset.toNumber()
const lengthNum = dataLength.toNumber()
runState.memory.extend(memOffsetNum, lengthNum)
runState.memory.write(memOffsetNum, lengthNum, data)
}
},
Expand Down Expand Up @@ -652,7 +648,8 @@ export const handlers: Map<number, OpHandler> = new Map([
0x51,
function (runState) {
const pos = runState.stack.pop()
const word = runState.memory.read(pos.toNumber(), 32)
const posNum = pos.toNumber()
const word = runState.memory.read(posNum, 32)
runState.stack.push(new BN(word))
},
],
Expand All @@ -663,7 +660,6 @@ export const handlers: Map<number, OpHandler> = new Map([
const [offset, word] = runState.stack.popN(2)
const buf = word.toArrayLike(Buffer, 'be', 32)
const offsetNum = offset.toNumber()
runState.memory.extend(offsetNum, 32)
runState.memory.write(offsetNum, 32, buf)
},
],
Expand All @@ -678,7 +674,6 @@ export const handlers: Map<number, OpHandler> = new Map([
// the types are wrong
const buf = Buffer.from([byte.andln(0xff) as unknown as number])
const offsetNum = offset.toNumber()
runState.memory.extend(offsetNum, 1)
runState.memory.write(offsetNum, 1, buf)
},
],
Expand Down
1 change: 0 additions & 1 deletion packages/vm/src/evm/opcodes/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,6 @@ export function writeCallOutput(runState: RunState, outOffset: BN, outLength: BN
dataLength = returnData.length
}
const data = getDataSlice(returnData, new BN(0), new BN(dataLength))
runState.memory.extend(memOffset, dataLength)
runState.memory.write(memOffset, dataLength, data)
}
}
Expand Down
46 changes: 40 additions & 6 deletions packages/vm/tests/api/evm/memory.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ tape('Memory', (t) => {
const m = new Memory()
t.test('should have 0 capacity initially', (st) => {
st.equal(m._store.length, 0)
st.throws(() => m.write(0, 3, Buffer.from([1, 2, 3])), /capacity/)
st.end()
})

Expand All @@ -25,11 +24,6 @@ tape('Memory', (t) => {
st.end()
})

t.test('should not write value beyond capacity', (st) => {
st.throws(() => m.write(30, 3, Buffer.from([1, 2, 3])), /capacity/)
st.end()
})

t.test('should write value', (st) => {
m.write(29, 3, Buffer.from([1, 2, 3]))
st.ok(m.read(29, 5).equals(Buffer.from([1, 2, 3, 0, 0])))
Expand All @@ -40,4 +34,44 @@ tape('Memory', (t) => {
st.throws(() => m.write(0, 5, Buffer.from([8, 8, 8])), /size/)
st.end()
})

t.test(
'should expand by word (32 bytes) properly when writing to previously untouched location',
(st) => {
const memory = new Memory()
memory.write(0, 1, Buffer.from([1]))
st.equal(memory._store.length, 32)

memory.write(1, 3, Buffer.from([2, 2, 2]))
st.equal(memory._store.length, 32)

memory.write(4, 32, Buffer.allocUnsafe(32).fill(3))
st.equal(memory._store.length, 64)

memory.write(36, 32, Buffer.allocUnsafe(32).fill(4))
st.equal(memory._store.length, 96)

st.end()
}
)

t.test('should expand by word (32 bytes) when reading a previously untouched location', (st) => {
const memory = new Memory()
memory.read(0, 8)
st.equal(memory._store.length, 32)

memory.read(1, 16)
st.equal(memory._store.length, 32)

memory.read(1, 32)
st.equal(memory._store.length, 64)

memory.read(32, 32)
st.equal(memory._store.length, 64)

memory.read(33, 32)
st.equal(memory._store.length, 96)

st.end()
})
})

0 comments on commit 0b7cc1b

Please sign in to comment.