bugc: lay storage arrays out as Solidity does

packages/bugc/CHANGELOG.md

@@ -28,6 +28,17 @@ support. Changes to the specification itself are tracked in the root
 
 ### Changed
 
+- Storage arrays now have Solidity's layout. A fixed-size array is
+  inline: its elements start at its own slot, so `[0] a: array<uint256,
+3>` takes slots 0 to 2, and a struct member that is a fixed-size array
+  takes all its slots. A dynamic array still keeps its length in its
+  slot and its elements from `keccak256(slot)`. In either, elements
+  narrower than 16 bytes now share a slot, as many as fit, from its
+  low-order end: an `array<uint8, 40>` takes two slots. Each storage
+  variable's pointer describes the same layout. Before, every array's
+  elements started at `keccak256(slot)`, one element to a slot, while
+  the pointer of a fixed-size array described it inline, and the pointer
+  of an array of narrow elements described them packed ([#364]).
 - An integer literal operand of an arithmetic or comparison operator now
   takes the type of the other operand when its value fits, so with
   `x: int8`, `x < 0`, `x == 1` and `-1 < x` compare as `int8`. A literal
@@ -301,3 +312,4 @@ First publication.
 [#351]: https://github.com/ethdebug/format/pull/351
 [#352]: https://github.com/ethdebug/format/pull/352
 [#356]: https://github.com/ethdebug/format/pull/356
+[#364]: https://github.com/ethdebug/format/pull/364

packages/bugc/examples/intermediate/arrays.bug

@@ -2,11 +2,11 @@ name ArraysAndLoops;
 
 storage {
   [0] numbers: array<uint256, 10>;
-  [1] sum: uint256;
-  [2] max: uint256;
-  [3] count: uint256;
-  [4] found: bool;
-  [5] searchValue: uint256;
+  [10] sum: uint256;
+  [11] max: uint256;
+  [12] count: uint256;
+  [13] found: bool;
+  [14] searchValue: uint256;
 }
 
 code {
@@ -21,7 +21,7 @@ code {
   after: deploy
   variables:
     sum:
-      pointer: { location: storage, slot: 1 }
+      pointer: { location: storage, slot: 10 }
       value: 0
   */
   for (let i = 0; i < 10; i = i + 1) {
@@ -78,21 +78,21 @@ code {
   fails: stack overflow during call - needs investigation
   variables:
     sum:
-      pointer: { location: storage, slot: 1 }
+      pointer: { location: storage, slot: 10 }
       value: 285
   */
   /*@test final-max
   fails: stack overflow during call - needs investigation
   variables:
     max:
-      pointer: { location: storage, slot: 2 }
+      pointer: { location: storage, slot: 11 }
       value: 81
   */
   /*@test final-count
   fails: stack overflow during call - needs investigation
   variables:
     count:
-      pointer: { location: storage, slot: 3 }
+      pointer: { location: storage, slot: 12 }
       value: 8
   */
 

packages/bugc/src/evmgen/behavioral.test.ts

@@ -1843,7 +1843,8 @@ code {
   o5 = m128[6];
 }`;
 
-    // bugc gives each array element its own slot
+    // A fixed-size array is inline, and its narrow elements share a
+    // slot, from the low-order end (a[1] is its bytes 2 and 3)
     const arrays = `name NarrowArrays;
 storage { [0] a: array<int16, 3>; [1] n: int256; [10] o0: int256; }
 create { n = -2 as int256; }
@@ -1882,7 +1883,7 @@ code { a[1] = n as int16; o0 = a[1]; }`;
           optimizationLevel: level,
         });
         expect(result.callSuccess).toBe(true);
-        expect(await result.getStorage(hash(pad(0n)) + 1n)).toBe(0xfffen);
+        expect(await result.getStorage(0n)).toBe(0xfffe0000n);
         expect(await result.getStorage(10n)).toBe(neg(2n));
       });
     }

packages/bugc/src/evmgen/constructor-array.test.ts

@@ -51,11 +51,11 @@ code {}
     const createFunc = module.create!;
     const entry = createFunc.blocks.get("entry")!;
 
-    // Check that we have compute_slot instructions for array access
+    // A fixed-size array is inline: no keccak256 of its slot
     const computeSlotInstructions = entry.instructions.filter(
       (i) => i.kind === "compute_slot" && i.slotKind === "array",
     );
-    expect(computeSlotInstructions.length).toBe(3);
+    expect(computeSlotInstructions.length).toBe(0);
 
     // Instructions are verified by checking write instructions below
 

packages/bugc/src/evmgen/generation/instructions/storage.ts

@@ -108,17 +108,42 @@ function signExtend<S extends Stack>(
   );
 }
 
+/** Whether a storage offset is the constant 0 (or absent) */
+function isZero(offset: Ir.Value | undefined): boolean {
+  return !offset || (offset.kind === "const" && BigInt(offset.value) === 0n);
+}
+
+/**
+ * Push a storage offset in bits: `offset * 8`. A storage array's
+ * element that shares its slot has an offset known only at run time.
+ */
+function bitOffset<S extends Stack>(
+  offset: Ir.Value | undefined,
+  debug: Ir.Instruction.Debug,
+): Transition<S, readonly ["shift", ...S]> {
+  if (!offset || offset.kind === "const") {
+    const bytes = offset ? BigInt(offset.value as bigint) : 0n;
+    return pipe<S>()
+      .then(PUSHn(bytes * 8n, { debug }), { as: "shift" })
+      .done();
+  }
+  return pipe<S>()
+    .then(loadValue(offset, { debug }), { as: "value" })
+    .then(PUSHn(3n, { debug }), { as: "shift" })
+    .then(SHL({ debug }), { as: "shift" })
+    .done();
+}
+
 /**
  * Storage read: SLOAD with optional partial-slot extraction
  */
 function generateStorageRead<S extends Stack>(
   inst: Ir.Instruction.Read,
   debug: Ir.Instruction.Debug,
 ): Transition<S, readonly ["value", ...S]> {
-  const offset = inst.offset?.kind === "const" ? inst.offset.value : 0n;
   const length = inst.length?.kind === "const" ? inst.length.value : 32n;
 
-  if (offset === 0n && length === 32n) {
+  if (isZero(inst.offset) && length === 32n) {
     // Full slot read - simple SLOAD
     return pipe<S>()
       .then(loadValue(inst.slot!, { debug }), { as: "key" })
@@ -136,7 +161,7 @@ function generateStorageRead<S extends Stack>(
 
       // Shift right by offset * 8 bits: storage writes count `offset`
       // from the low-order end of the slot
-      .then(PUSHn(BigInt(offset) * 8n, { debug }), { as: "shift" })
+      .then(bitOffset(inst.offset, debug), { as: "shift" })
       .then(SHR({ debug }), { as: "shiftedValue" })
       .then(PUSHn(1n, { debug }), { as: "b" })
 
@@ -402,10 +427,9 @@ function generateStorageWrite<S extends Stack>(
   inst: Ir.Instruction.Write,
   debug: Ir.Instruction.Debug,
 ): Transition<S, S> {
-  const offset = inst.offset?.kind === "const" ? inst.offset.value : 0n;
   const length = inst.length?.kind === "const" ? inst.length.value : 32n;
 
-  if (offset === 0n && length === 32n) {
+  if (isZero(inst.offset) && length === 32n) {
     // Full slot write - simple SSTORE
     return pipe<S>()
       .then(loadValue(inst.value!, { debug }), { as: "value" })
@@ -430,9 +454,7 @@ function generateStorageWrite<S extends Stack>(
       .then(SUB({ debug }), { as: "lengthMask" })
 
       // Shift mask to offset position
-      .then(PUSHn(BigInt(offset) * 8n, { debug }), {
-        as: "bitOffset",
-      })
+      .then(bitOffset(inst.offset, debug), { as: "bitOffset" })
       .then(
         rebrand<"bitOffset", "shift", "lengthMask", "value">({
           1: "shift",
@@ -460,7 +482,7 @@ function generateStorageWrite<S extends Stack>(
         as: "a",
       })
       .then(AND({ debug }), { as: "value" })
-      .then(PUSHn(BigInt(offset) * 8n, { debug }), { as: "shift" })
+      .then(bitOffset(inst.offset, debug), { as: "shift" })
       .then(SHL({ debug }), { as: "shiftedValue" })
 
       .then(

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

@@ -0,0 +1,180 @@
+/**
+ * Storage arrays have Solidity's layout. A fixed-size array is inline:
+ * its elements start at its own slot. A dynamic array keeps its length
+ * in its slot and its elements from keccak256(slot). Either way, an
+ * element narrower than 16 bytes shares a slot with its neighbors, as
+ * many as fit, from the low-order end; any other element starts a slot
+ * of its own and takes as many slots as its type needs. An array, as a
+ * struct member, starts a slot, and the next member starts the slot
+ * after it. Each storage variable's pointer must read the values the
+ * program wrote.
+ */
+import { describe, it, expect } from "vitest";
+import { keccak256 } from "ethereum-cryptography/keccak";
+import { bytesToHex } from "ethereum-cryptography/utils";
+import { createMachineState } from "@ethdebug/evm";
+import { dereference } from "@ethdebug/pointers";
+import type * as Format from "@ethdebug/format";
+
+import { executeProgram } from "#test/evm/behavioral";
+import { traceLocals } from "#test/evm/locals";
+
+const levels = [0, 1, 2, 3] as const;
+
+/** keccak256 of a slot number, as a dynamic array's first slot */
+function hashed(slot: bigint): bigint {
+  const word = new Uint8Array(32);
+  for (let i = 0; i < 32; i++) {
+    word[31 - i] = Number((slot >> BigInt(8 * i)) & 0xffn);
+  }
+  return BigInt("0x" + bytesToHex(keccak256(word)));
+}
+
+interface Case {
+  source: string;
+  /** Expected words, by slot */
+  slots: [bigint, bigint][];
+  /** Expected values, in order, of each variable's regions, by name */
+  pointers?: Record<string, Record<string, bigint[]>>;
+}
+
+const cases: Record<string, Case> = {
+  "a fixed-size array is inline": {
+    source: `storage {
+  [0] f: array<uint256, 3>;
+  [3] r: uint256;
+}
+code { f[0] = 10; f[2] = 30; r = f[0] + f[2]; }`,
+    slots: [
+      [0n, 10n],
+      [1n, 0n],
+      [2n, 30n],
+      [3n, 40n],
+    ],
+    pointers: { f: { element: [10n, 0n, 30n] } },
+  },
+  "narrow elements share a slot, from the low-order end": {
+    source: `storage {
+  [0] a: array<uint8, 40>;
+  [2] r: uint256;
+}
+code { a[0] = 1; a[1] = 2; a[33] = 7; r = (a[1] as uint256) + (a[33] as uint256); }`,
+    slots: [
+      [0n, 0x0201n],
+      [1n, 0x0700n],
+      [2n, 9n],
+    ],
+    pointers: {
+      a: {
+        element: Array.from({ length: 40 }, (_, i) =>
+          i === 0 ? 1n : i === 1 ? 2n : i === 33 ? 7n : 0n,
+        ),
+      },
+    },
+  },
+  "a fixed-size array of fixed-size arrays": {
+    source: `storage {
+  [0] m: array<array<uint256, 3>, 2>;
+  [6] r: uint256;
+}
+code { m[0][1] = 4; m[1][2] = 5; r = m[1][2]; }`,
+    slots: [
+      [1n, 4n],
+      [5n, 5n],
+      [6n, 5n],
+    ],
+    pointers: { m: { element: [0n, 4n, 0n, 0n, 0n, 5n] } },
+  },
+  "an array in a struct takes its slots": {
+    source: `define {
+  struct S { x: uint8; arr: array<uint256, 2>; y: uint8; };
+}
+storage {
+  [0] s: S;
+  [4] r: uint256;
+}
+code { s.x = 1; s.arr[1] = 2; s.y = 3; r = s.arr[1] + (s.y as uint256); }`,
+    slots: [
+      [0n, 1n],
+      [1n, 0n],
+      [2n, 2n],
+      [3n, 3n],
+      [4n, 5n],
+    ],
+    pointers: { s: { x: [1n], "arr-element": [0n, 2n], y: [3n] } },
+  },
+  "a fixed-size array of structs": {
+    source: `define {
+  struct P { a: uint256; b: uint256; };
+}
+storage {
+  [0] ps: array<P, 2>;
+  [4] r: uint256;
+}
+code { ps[1].a = 6; ps[1].b = 7; ps[0].b = 8; r = ps[1].b; }`,
+    slots: [
+      [1n, 8n],
+      [2n, 6n],
+      [3n, 7n],
+      [4n, 7n],
+    ],
+    pointers: { ps: { a: [0n, 6n], b: [8n, 7n] } },
+  },
+  "a dynamic array's narrow elements share a slot": {
+    source: `storage {
+  [0] d: array<uint16>;
+  [1] r: uint256;
+}
+code { d[0] = 3; d[1] = 5; r = d[1] as uint256; }`,
+    slots: [
+      [hashed(0n), 0x00050003n],
+      [1n, 5n],
+    ],
+  },
+};
+
+describe("storage array layout", () => {
+  for (const [name, { source, slots }] of Object.entries(cases)) {
+    for (const level of levels) {
+      it(`${name} (level ${level})`, async () => {
+        const result = await executeProgram(`name Layout;\n${source}`, {
+          calldata: "",
+          optimizationLevel: level,
+        });
+        expect(result.callSuccess).toBe(true);
+        for (const [slot, value] of slots) {
+          expect(await result.getStorage(slot), `slot ${slot}`).toBe(value);
+        }
+      });
+    }
+  }
+});
+
+describe("storage array pointers", () => {
+  for (const [name, { source, pointers }] of Object.entries(cases)) {
+    if (!pointers) continue;
+    it(name, async () => {
+      const { program, executor, steps } = await traceLocals(
+        `name Layout;\n${source}`,
+      );
+      const state = createMachineState(executor, {
+        traceStep: steps[steps.length - 1],
+      });
+      for (const [identifier, regions] of Object.entries(pointers)) {
+        const variable = (
+          program.context as Format.Program.Context.Variables
+        ).variables.find((v) => v.identifier === identifier)!;
+        const cursor = await dereference(variable.pointer!, { state });
+        const view = await cursor.view(state);
+        for (const [regionName, values] of Object.entries(regions)) {
+          const read = await Promise.all(
+            view.regions
+              .filter((region) => region.name === regionName)
+              .map(async (region) => (await view.read(region)).asUint()),
+          );
+          expect(read, `${identifier}: ${regionName}`).toEqual(values);
+        }
+      }
+    });
+  }
+});