Skip to content

Commit 0880a4f

Browse files
committed
bugc: add push to dynamic arrays in storage
1 parent 61acac4 commit 0880a4f

10 files changed

Lines changed: 424 additions & 21 deletions

File tree

‎packages/bugc/CHANGELOG.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ support. Changes to the specification itself are tracked in the root
1919
cast it to `bytes32` to match Solidity. A single dynamic `bytes` or
2020
`string` argument still hashes its data, and must be the only argument.
2121
The optimizer folds a hash of constant words at levels 1 to 3 ([#365]).
22+
- `push` on a dynamic array in storage, as in Solidity: `a.push(v)`
23+
writes `v` at index `a.length` and adds one to the length, and
24+
`a.push()` adds a zero element, as for an array of arrays. `a` may be
25+
in a mapping or a struct. A push has no value. On an array in memory
26+
or a fixed-size array, it is a type error ([#366]).
2227
- The `%` operator, with the precedence of `*` and `/`. It compiles to
2328
the EVM's `MOD` (`SMOD` for signed operands), so `x % 0` is `0`, as
2429
`x / 0` is ([#321]).
@@ -40,6 +45,11 @@ support. Changes to the specification itself are tracked in the root
4045

4146
- The IR `hash` instruction now has `values`, a list, in place of
4247
`value` and `valueDebug` ([#365]).
48+
- An index into a dynamic array in storage now reverts with Solidity's
49+
`Panic(0x32)` unless it is less than the array's length, for reads and
50+
writes, as an index into an array in memory does. Before, a write past
51+
the end wrote there and left the length as it was, so the array's
52+
pointer did not show the element ([#366]).
4353
- Storage arrays now have Solidity's layout. A fixed-size array is
4454
inline: its elements start at its own slot, so `[0] a: array<uint256,
4555
3>` takes slots 0 to 2, and a struct member that is a fixed-size array
@@ -102,6 +112,8 @@ support. Changes to the specification itself are tracked in the root
102112

103113
### Fixed
104114

115+
- `a.length` on a dynamic array in storage now reads the length in the
116+
array's slot. Before, it did not read that slot ([#366]).
105117
- At optimization levels 2 and 3, an inlined body's control flow now has
106118
a `transform: ["inline"]` context: the branch of an `if`, the jumps
107119
between the body's blocks, each block's `JUMPDEST`, and the jump that
@@ -391,3 +403,4 @@ First publication.
391403
[#363]: https://github.com/ethdebug/format/pull/363
392404
[#364]: https://github.com/ethdebug/format/pull/364
393405
[#365]: https://github.com/ethdebug/format/pull/365
406+
[#366]: https://github.com/ethdebug/format/pull/366

‎packages/bugc/examples/advanced/nested-arrays.bug‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ name NestedArrays;
33
// Example 7: Nested Arrays
44
//
55
// Expected IR:
6-
// // First dimension: matrix[i]
6+
// // First dimension: matrix[i], checked against matrix's length
77
// %outer_base = compute_slot kind="array", base=15
88
// %inner_array_slot = add %outer_base, %i
99
//
@@ -23,8 +23,15 @@ storage {
2323
}
2424

2525
code {
26-
let i = 2;
27-
let j = 5;
26+
// An index must be below the length, so push the rows and elements
27+
// first: matrix[1] gets two elements
28+
matrix.push();
29+
matrix.push();
30+
matrix[1].push(0);
31+
matrix[1].push(0);
32+
33+
let i = 1;
34+
let j = 1;
2835
let value = 999;
2936
matrix[i][j] = value;
3037
let elem = matrix[i][j];

‎packages/bugc/src/evmgen/storage-array-layout.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,9 @@ code { ps[1].a = 6; ps[1].b = 7; ps[0].b = 8; r = ps[1].b; }`,
125125
[0] d: array<uint16>;
126126
[1] r: uint256;
127127
}
128-
code { d[0] = 3; d[1] = 5; r = d[1] as uint256; }`,
128+
code { d.push(3); d.push(5); r = d[1] as uint256; }`,
129129
slots: [
130+
[0n, 2n],
130131
[hashed(0n), 0x00050003n],
131132
[1n, 5n],
132133
],
Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
/**
2+
* `push` on a dynamic array in storage, as in Solidity: `a.push(v)`
3+
* writes `v` at index `a.length`, then adds one to the length;
4+
* `a.push()` adds a zero element. An index at or past the length
5+
* reverts with Panic(0x32), for reads and writes, as in memory.
6+
*/
7+
import { describe, it, expect } from "vitest";
8+
import { keccak256 } from "ethereum-cryptography/keccak";
9+
import { bytesToHex } from "ethereum-cryptography/utils";
10+
import { createMachineState } from "@ethdebug/evm";
11+
import { dereference } from "@ethdebug/pointers";
12+
import type * as Format from "@ethdebug/format";
13+
14+
import { compile } from "#compiler";
15+
import { executeProgram } from "#test/evm/behavioral";
16+
import { traceLocals } from "#test/evm/locals";
17+
18+
const levels = [0, 1, 2, 3] as const;
19+
20+
const panic32 = "4e487b71" + 0x32n.toString(16).padStart(64, "0");
21+
22+
function word(value: bigint): Uint8Array {
23+
const bytes = new Uint8Array(32);
24+
for (let i = 0; i < 32; i++) {
25+
bytes[31 - i] = Number((value >> BigInt(8 * i)) & 0xffn);
26+
}
27+
return bytes;
28+
}
29+
30+
/** keccak256 of words, as a dynamic array's first slot */
31+
function hashed(...values: bigint[]): bigint {
32+
const data = new Uint8Array(32 * values.length);
33+
values.forEach((value, i) => data.set(word(value), 32 * i));
34+
return BigInt("0x" + bytesToHex(keccak256(data)));
35+
}
36+
37+
const passes: Record<string, [string, [bigint, bigint][]]> = {
38+
"push values and read them back": [
39+
`storage { [0] h: array<uint256>; [1] r: uint256; }
40+
code { h.push(5); h.push(7); r = h[1] + h.length; }`,
41+
[
42+
[0n, 2n],
43+
[hashed(0n), 5n],
44+
[hashed(0n) + 1n, 7n],
45+
[1n, 9n],
46+
],
47+
],
48+
"push narrow elements into one slot": [
49+
`storage { [0] h: array<uint8>; }
50+
code { h.push(1); h.push(2); }`,
51+
[
52+
[0n, 2n],
53+
[hashed(0n), 0x0201n],
54+
],
55+
],
56+
"push an empty element, then into it": [
57+
`storage { [0] m: array<array<uint256>>; [1] r: uint256; }
58+
code { m.push(); m[0].push(4); r = m[0][0] + m.length + m[0].length; }`,
59+
[
60+
[0n, 1n],
61+
[hashed(0n), 1n],
62+
[hashed(hashed(0n)), 4n],
63+
[1n, 6n],
64+
],
65+
],
66+
"push into a mapping's array": [
67+
`storage { [0] q: mapping<uint256, array<uint256>>; [1] r: uint256; }
68+
code { q[3].push(9); r = q[3][0]; }`,
69+
[
70+
[hashed(3n, 0n), 1n],
71+
[hashed(hashed(3n, 0n)), 9n],
72+
[1n, 9n],
73+
],
74+
],
75+
"push into a struct's array": [
76+
`define { struct S { n: uint8; xs: array<uint256>; }; }
77+
storage { [0] s: S; [2] r: uint256; }
78+
code { s.n = 1; s.xs.push(6); r = s.xs[0]; }`,
79+
[
80+
[0n, 1n],
81+
[1n, 1n],
82+
[hashed(1n), 6n],
83+
[2n, 6n],
84+
],
85+
],
86+
"write an element below the length": [
87+
`storage { [0] h: array<uint256>; }
88+
code { h.push(1); h.push(2); h[1] = 3; }`,
89+
[[hashed(0n) + 1n, 3n]],
90+
],
91+
};
92+
93+
const reverts: Record<string, string> = {
94+
"write at the length": `storage { [0] h: array<uint256>; }
95+
code { h.push(1); h[1] = 2; }`,
96+
"read an empty array": `storage { [0] h: array<uint256>; [1] r: uint256; }
97+
code { r = h[0]; }`,
98+
"write into an empty inner array": `storage { [0] m: array<array<uint256>>; }
99+
code { m.push(); m[0][0] = 1; }`,
100+
};
101+
102+
describe("push on a storage array", () => {
103+
for (const [name, [body, slots]] of Object.entries(passes)) {
104+
for (const level of levels) {
105+
it(`should ${name} (level ${level})`, async () => {
106+
const result = await executeProgram(`name Push;\n${body}`, {
107+
calldata: "",
108+
optimizationLevel: level,
109+
});
110+
expect(result.callSuccess).toBe(true);
111+
for (const [slot, value] of slots) {
112+
expect(await result.getStorage(slot), `slot ${slot}`).toBe(value);
113+
}
114+
});
115+
}
116+
}
117+
118+
for (const [name, body] of Object.entries(reverts)) {
119+
for (const level of levels) {
120+
it(`should revert: ${name} (level ${level})`, async () => {
121+
const result = await executeProgram(`name Push;\n${body}`, {
122+
calldata: "",
123+
optimizationLevel: level,
124+
});
125+
expect(result.callSuccess).toBe(false);
126+
expect(bytesToHex(result.returnValue)).toBe(panic32);
127+
});
128+
}
129+
}
130+
131+
it("gives the pushed elements to the array's pointer", async () => {
132+
const { program, executor, steps } = await traceLocals(
133+
`name Push;\nstorage { [0] h: array<uint16>; }
134+
code { h.push(5); h.push(7); h.push(9); }`,
135+
);
136+
const state = createMachineState(executor, {
137+
traceStep: steps[steps.length - 1],
138+
});
139+
const variable = (
140+
program.context as Format.Program.Context.Variables
141+
).variables.find((v) => v.identifier === "h")!;
142+
const cursor = await dereference(variable.pointer!, { state });
143+
const view = await cursor.view(state);
144+
const read = await Promise.all(
145+
view.regions
146+
.filter((region) => region.name === "element")
147+
.map(async (region) => (await view.read(region)).asUint()),
148+
);
149+
expect(read).toEqual([5n, 7n, 9n]);
150+
});
151+
});
152+
153+
describe("push type errors", () => {
154+
const errors: Record<string, string> = {
155+
"push on a fixed-size array": `storage { [0] f: array<uint256, 3>; }
156+
code { f.push(1); }`,
157+
"push on a memory array": `code { let a: array<uint256> = [1, 2]; a.push(3); }`,
158+
"push a value of the wrong type": `storage { [0] h: array<uint8>; }
159+
code { h.push(true); }`,
160+
"push two values": `storage { [0] h: array<uint256>; }
161+
code { h.push(1, 2); }`,
162+
};
163+
for (const [name, body] of Object.entries(errors)) {
164+
it(`rejects ${name}`, async () => {
165+
const result = await compile({
166+
to: "bytecode",
167+
source: `name Push;\n${body}`,
168+
});
169+
expect(result.success).toBe(false);
170+
});
171+
}
172+
});

‎packages/bugc/src/irgen/generate/expressions/access.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,14 @@ const makeBuildMemberAccess = (
110110
return Ir.Value.temp(tempId, resultType);
111111
}
112112

113+
// A dynamic array in storage keeps its length in its slot
114+
if (Type.isArray(objectType)) {
115+
const chain = yield* findStorageAccessChain(expr.object);
116+
if (chain) {
117+
return yield* emitStorageChainLoad(chain, resultType, expr);
118+
}
119+
}
120+
113121
// For dynamic arrays/bytes/strings, emit length instruction
114122
const object = yield* buildExpression(expr.object, { kind: "rvalue" });
115123
yield* Process.Instructions.emit({

‎packages/bugc/src/irgen/generate/expressions/array.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ export function* buildArray(
5454
slot,
5555
Ir.Value.constant(BigInt(i), Ir.Type.Scalar.uint256),
5656
expr,
57+
// Each index is below the literal's length
58+
false,
5759
);
5860
yield* Process.Instructions.emit({
5961
kind: "write",

‎packages/bugc/src/irgen/generate/expressions/call.ts‎

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type * as Ast from "#ast";
1+
import * as Ast from "#ast";
22
import * as Ir from "#ir";
33
import { Severity } from "#result";
44
import { Type } from "#types";
@@ -7,6 +7,12 @@ import { Error as IrgenError } from "#irgen/errors";
77
import { Process } from "../process.js";
88
import type { Context } from "./context.js";
99
import { fromBugType } from "#irgen/type";
10+
import {
11+
type StorageAccessChain,
12+
findStorageAccessChain,
13+
emitStorageChainLoad,
14+
emitStorageChainStore,
15+
} from "../storage.js";
1016

1117
/**
1218
* Build a call expression
@@ -21,6 +27,15 @@ export const makeBuildCall = (
2127
expr: Ast.Expression.Call,
2228
_context: Context,
2329
): Process<Ir.Value> {
30+
// `a.push(v)` or `a.push()` on a dynamic array in storage
31+
if (
32+
Ast.Expression.isAccess(expr.callee) &&
33+
Ast.Expression.Access.isMember(expr.callee) &&
34+
expr.callee.property === "push"
35+
) {
36+
return yield* buildPush(expr, expr.callee, buildExpression);
37+
}
38+
2439
// Check if this is a built-in function call
2540
if (
2641
expr.callee.kind === "expression:identifier" &&
@@ -124,3 +139,73 @@ export const makeBuildCall = (
124139
);
125140
return Ir.Value.constant(0n, Ir.Type.Scalar.uint256);
126141
};
142+
143+
/**
144+
* Push onto a dynamic array in storage, as Solidity does: write the
145+
* value (if any) at index `length`, then store `length + 1`. With no
146+
* value, the new element is zero: nothing can have written past the
147+
* length.
148+
*/
149+
function* buildPush(
150+
expr: Ast.Expression.Call,
151+
callee: Ast.Expression.Access.Member,
152+
buildExpression: (
153+
node: Ast.Expression,
154+
context: Context,
155+
) => Process<Ir.Value>,
156+
): Process<Ir.Value> {
157+
const marker = Ir.Value.constant(0n, Ir.Type.Scalar.uint256);
158+
const value =
159+
expr.arguments.length > 0
160+
? yield* buildExpression(expr.arguments[0], { kind: "rvalue" })
161+
: undefined;
162+
163+
const chain = yield* findStorageAccessChain(callee.object);
164+
if (!chain) {
165+
yield* Process.Errors.report(
166+
new IrgenError(
167+
"push needs a dynamic array in storage",
168+
expr.loc ?? undefined,
169+
Severity.Error,
170+
),
171+
);
172+
return marker;
173+
}
174+
// Each use of the chain gets its own copy of its accesses
175+
const copy = (
176+
extra: StorageAccessChain["accesses"] = [],
177+
): StorageAccessChain => ({
178+
slot: chain.slot,
179+
accesses: [...chain.accesses.map((access) => ({ ...access })), ...extra],
180+
});
181+
182+
const length = yield* emitStorageChainLoad(
183+
copy(),
184+
Ir.Type.Scalar.uint256,
185+
expr,
186+
);
187+
if (value) {
188+
yield* emitStorageChainStore(
189+
copy([{ kind: "index", key: length, unchecked: true }]),
190+
value,
191+
expr,
192+
);
193+
}
194+
195+
const next = yield* Process.Variables.newTemp();
196+
yield* Process.Instructions.emit({
197+
kind: "binary",
198+
op: "add",
199+
left: length,
200+
right: Ir.Value.constant(1n, Ir.Type.Scalar.uint256),
201+
dest: next,
202+
operationDebug: yield* Process.Debug.forAstNode(expr),
203+
} as Ir.Instruction.BinaryOp);
204+
yield* emitStorageChainStore(
205+
copy(),
206+
Ir.Value.temp(next, Ir.Type.Scalar.uint256),
207+
expr,
208+
);
209+
210+
return marker;
211+
}

0 commit comments

Comments
 (0)