diff --git a/packages/bugc/CHANGELOG.md b/packages/bugc/CHANGELOG.md index 0481d5f4da..9714910944 100644 --- a/packages/bugc/CHANGELOG.md +++ b/packages/bugc/CHANGELOG.md @@ -50,6 +50,18 @@ support. Changes to the specification itself are tracked in the root - A cast from dynamic `bytes` to an integer or `address` is now a type error, as in Solidity: cast to `bytesN` first, as in `msg.data[4:36] as bytes32 as uint256` ([#334]). +- An index out of bounds now reverts with Solidity's `Panic(0x32)`, as + in Solidity: `a[i]` on an array or `bytes` in memory, or on a + fixed-size array in storage, reverts unless `i` is less than the + length, for reads and writes, at each level of a nested array. A slice + `b[s:e]` reverts unless `s <= e <= b.length`, so `msg.data[0:4]` + reverts on shorter calldata. The `REVERT` has a `revert` context with + the panic code. Before, an index past the end read or wrote the memory + that follows, such as the next inner array's length word. A dynamic + array in storage is not checked ([#344]). +- An array or bytes index and a slice bound must now be an unsigned + integer, as in Solidity; cast a signed one, as in `a[i as uint256]`. + Before, a signed index compiled ([#344]). ### Fixed @@ -187,6 +199,9 @@ support. Changes to the specification itself are tracked in the root skipped no length word and read a whole word, so it gave the length's bytes; a write failed with error EVM999, as an array element write did ([#343]). +- `.length` of an array literal, as in `[1, 2, 3].length`, is now the + number of elements. Before, at every optimization level, it was 32 + ([#344]). ## 0.1.0-preview.1 — 2026-10-03 @@ -240,3 +255,4 @@ First publication. [#340]: https://github.com/ethdebug/format/pull/340 [#341]: https://github.com/ethdebug/format/pull/341 [#343]: https://github.com/ethdebug/format/pull/343 +[#344]: https://github.com/ethdebug/format/pull/344 diff --git a/packages/bugc/src/evmgen/analysis/liveness.ts b/packages/bugc/src/evmgen/analysis/liveness.ts index 60aa09a7a6..3a92c67542 100644 --- a/packages/bugc/src/evmgen/analysis/liveness.ts +++ b/packages/bugc/src/evmgen/analysis/liveness.ts @@ -284,6 +284,9 @@ function getUsedValues(inst: Ir.Instruction): Set { case "length": addValue(inst.object); break; + case "assert": + addValue(inst.condition); + break; case "allocate": addValue(inst.size); break; @@ -338,6 +341,7 @@ function getDefinedValue(inst: Ir.Instruction): string | undefined { return inst.dest; // These instructions don't define values case "write": // NEW: unified write + case "assert": return undefined; } } diff --git a/packages/bugc/src/evmgen/analysis/memory.ts b/packages/bugc/src/evmgen/analysis/memory.ts index f8d68cf755..194fe13dfa 100644 --- a/packages/bugc/src/evmgen/analysis/memory.ts +++ b/packages/bugc/src/evmgen/analysis/memory.ts @@ -265,6 +265,7 @@ function simulateInstruction(stack: string[], inst: Ir.Instruction): string[] { case "unary": case "cast": case "length": + case "assert": newStack.pop(); // One operand break; // NEW: unified read - pops slot/offset/length as needed @@ -356,6 +357,9 @@ function getUsedValues(inst: Ir.Instruction): Set { case "hash": addValue(inst.value); break; + case "assert": + addValue(inst.condition); + break; // Call instruction removed - calls are now block terminators } @@ -412,6 +416,7 @@ function definedType(inst: Ir.Instruction): Ir.Type | undefined { case "compute_offset": return Ir.Type.Scalar.word; case "write": + case "assert": return undefined; } } diff --git a/packages/bugc/src/evmgen/bounds.test.ts b/packages/bugc/src/evmgen/bounds.test.ts new file mode 100644 index 0000000000..7218e3dd72 --- /dev/null +++ b/packages/bugc/src/evmgen/bounds.test.ts @@ -0,0 +1,432 @@ +import { describe, it, expect } from "vitest"; +import { bytesToHex } from "ethereum-cryptography/utils"; + +import { compile } from "#compiler"; +import { executeProgram } from "#test/evm/behavioral"; + +// Solidity's revert data for an index out of bounds: Panic(0x32) +const panic32 = "4e487b71" + 0x32n.toString(16).padStart(64, "0"); + +const levels = [0, 1, 2, 3] as const; + +// Each program writes its results to storage slots 0, 1, ...; it is +// called with the calldata given, if any +type Passes = Record; + +// Each program must revert with Panic(0x32) when called with the +// calldata given, if any +type Reverts = Record; + +function passes(programs: Passes) { + for (const [name, [body, expected, calldata = ""]] of Object.entries( + programs, + )) { + for (const level of levels) { + it(`should ${name} (level ${level})`, async () => { + const result = await executeProgram(`name Program;\n${body}`, { + calldata, + optimizationLevel: level, + }); + + expect(result.callSuccess).toBe(true); + for (const [slot, value] of expected.entries()) { + expect(await result.getStorage(BigInt(slot))).toBe(value); + } + }); + } + } +} + +function reverts(programs: Reverts) { + for (const [name, [body, calldata = ""]] of Object.entries(programs)) { + for (const level of levels) { + it(`should revert: ${name} (level ${level})`, async () => { + const result = await executeProgram(`name Program;\n${body}`, { + calldata, + optimizationLevel: level, + }); + + expect(result.callSuccess).toBe(false); + expect(bytesToHex(result.returnValue)).toBe(panic32); + // Nothing the program wrote before it reverted stays + expect(await result.getStorage(0n)).toBe(0n); + }); + } + } +} + +describe("memory array bounds", () => { + passes({ + "read the first and last elements": [ + `storage { [0] r0: uint256; [1] r2: uint256; } +code { + let a: array = [1, 2, 3]; + r0 = a[0]; + r2 = a[2]; +}`, + [1n, 3n], + ], + "write the first and last elements": [ + `storage { [0] r0: uint256; [1] r2: uint256; } +code { + let a: array = [1, 2, 3]; + a[0] = 10; + a[2] = 30; + r0 = a[0]; + r2 = a[2]; +}`, + [10n, 30n], + ], + "read the last element at an index from calldata": [ + `storage { [0] r: uint256; } +code { + let a: array = [1, 2, 3]; + r = a[msg.data.length - 1]; +}`, + [3n], + "0x000000", + ], + "sum elements in a loop that runs exactly to the length": [ + `storage { [0] r: uint256; } +code { + let a: array = [1, 2, 3, 4]; + let sum = 0; + for (let i = 0; i < a.length; i = i + 1) { + a[i] = a[i] * 2; + sum = sum + a[i]; + } + r = sum; +}`, + [20n], + ], + "index an array literal": [ + `storage { [0] r: uint256; [1] s: uint256; } +create { s = 2; } +code { + r = [1, 2, 3][s]; +}`, + [3n], + ], + "read and write nested elements at the edges": [ + `storage { [0] r0: uint256; [1] r1: uint256; [2] r2: uint256; } +code { + let m: array> = [[1], [2, 3]]; + m[1][1] = 30; + m[0][0] = 10; + r0 = m[0][0]; + r1 = m[1][0]; + r2 = m[1][1]; +}`, + [10n, 2n, 30n], + ], + }); + + reverts({ + "read at the length": [ + `storage { [0] r: uint256; } +code { + let a: array = [1, 2, 3]; + r = a[3]; +}`, + ], + "write at the length": [ + `storage { [0] r: uint256; } +code { + let a: array = [1, 2, 3]; + r = 1; + a[3] = 7; +}`, + ], + "read at an index from calldata": [ + `storage { [0] r: uint256; } +code { + let a: array = [1, 2, 3]; + r = 1; + r = a[msg.data.length]; +}`, + "0x000000", + ], + "write at an index from calldata": [ + `storage { [0] r: uint256; } +code { + let a: array = [1, 2, 3]; + r = 1; + a[msg.data.length] = 7; +}`, + "0x000000", + ], + "read at a huge index": [ + `storage { [0] r: uint256; } +code { + let a: array = [1, 2, 3]; + let i = 0; + i = i - 1; + r = a[i]; +}`, + ], + "write at a huge index": [ + `storage { [0] r: uint256; } +code { + let a: array = [1, 2, 3]; + r = 1; + a[57896044618658097711785492504343953926634992332820282019728792003956564819968] = 7; +}`, + ], + "read at an index that wraps below zero": [ + `storage { [0] r: uint256; } +code { + let a: array = [1, 2, 3]; + r = a[0 - 1]; +}`, + ], + "index an array literal past its length": [ + `storage { [0] r: uint256; [1] s: uint256; } +create { s = 5; } +code { + r = [1, 2, 3][s]; +}`, + ], + "write at a negative index cast to uint256": [ + `storage { [0] r: uint256; } +code { + let a: array = [1, 2, 3]; + let i: int256 = -1 as int256; + r = 1; + a[i as uint256] = 99; +}`, + ], + "read past a shorter array assigned to the variable": [ + `storage { [0] r: uint256; } +code { + let a: array = [1, 2, 3]; + a = [4]; + r = a[1]; +}`, + ], + "write in a loop that runs one past the length": [ + `storage { [0] r: uint256; } +code { + let a: array = [0, 0, 0]; + r = 1; + for (let i = 0; i <= a.length; i = i + 1) { + a[i] = i; + } +}`, + ], + "write past the end of an inner array": [ + `storage { [0] r: uint256; } +code { + let m: array> = [[1], [2]]; + r = 1; + m[0][1] = 99; +}`, + ], + "read past the end of an inner array": [ + `storage { [0] r: uint256; } +code { + let m: array> = [[1], [2]]; + r = m[0][1]; +}`, + ], + "read past the end of the outer array": [ + `storage { [0] r: uint256; } +code { + let m: array> = [[1], [2]]; + r = m[2][0]; +}`, + ], + "write past the end of an array parameter": [ + `define { + function set(xs: array, i: uint256) -> uint256 { + xs[i] = 1; + return 0; + }; +} +storage { [0] r: uint256; } +code { + let a: array = [1, 2, 3]; + r = 1; + r = set(a, 3); +}`, + ], + }); +}); + +describe("memory bytes bounds", () => { + passes({ + "read and write the first and last bytes": [ + `storage { [0] r0: uint256; [1] r2: uint256; } +code { + let b = msg.data[0:3]; + b[0] = b[0] + 1 as uint8; + b[2] = b[2] + 1 as uint8; + r0 = b[0]; + r2 = b[2]; +}`, + [11n, 13n], + "0x0a0b0c", + ], + }); + + reverts({ + "read at the length": [ + `storage { [0] r: uint256; } +code { + let b = msg.data[0:3]; + r = b[3]; +}`, + "0x0a0b0c", + ], + "write at the length": [ + `storage { [0] r: uint256; } +code { + let b = msg.data[0:3]; + r = 1; + b[3] = 1 as uint8; +}`, + "0x0a0b0c", + ], + }); +}); + +describe("slice bounds", () => { + passes({ + "slice all of msg.data": [ + `storage { [0] n: uint256; [1] r: uint256; } +code { + let b = msg.data[0:3]; + n = b.length; + r = b[2]; +}`, + [3n, 12n], + "0x0a0b0c", + ], + "slice all of memory bytes": [ + `storage { [0] n: uint256; [1] r: uint256; } +code { + let b = msg.data[0:3]; + let c = b[1:3]; + n = c.length; + r = c[1]; +}`, + [2n, 12n], + "0x0a0b0c", + ], + "take an empty slice at the end": [ + `storage { [0] n: uint256; } +code { + let b = msg.data[0:3]; + let c = b[3:3]; + n = c.length + 1; +}`, + [1n], + "0x0a0b0c", + ], + }); + + reverts({ + "slice msg.data past its end": [ + `storage { [0] r: uint256; } +code { + let b = msg.data[0:4]; + r = b.length; +}`, + "0x0a0b0c", + ], + "slice memory bytes past their end": [ + `storage { [0] r: uint256; } +code { + let b = msg.data[0:3]; + let c = b[1:4]; + r = c.length; +}`, + "0x0a0b0c", + ], + "slice with the start after the end": [ + `storage { [0] r: uint256; } +code { + let b = msg.data[0:3]; + let c = b[2:1]; + r = c.length; +}`, + "0x0a0b0c", + ], + }); +}); + +describe("storage array bounds", () => { + passes({ + "read and write the first and last elements of a fixed array": [ + `storage { [0] r0: uint256; [1] r2: uint256; [2] f: array; } +code { + f[0] = 10; + f[2] = 30; + r0 = f[0]; + r2 = f[2]; +}`, + [10n, 30n], + ], + }); + + reverts({ + "write a fixed array at its length": [ + `storage { [0] r: uint256; [1] f: array; } +code { + r = 1; + f[3] = 7; +}`, + ], + "write a fixed array at an index that wraps below zero": [ + `storage { [0] r: uint256; [1] f: array; } +code { + r = 1; + f[0 - 1] = 7; +}`, + ], + "read a fixed array at an index from calldata": [ + `storage { [0] r: uint256; [1] f: array; } +code { + r = 1; + r = f[msg.data.length]; +}`, + "0x000000", + ], + }); +}); + +describe("bounds check debug info", () => { + const source = `name Program; +storage { [0] r: uint256; } +code { + let a: array = [1, 2, 3]; + r = a[msg.data.length]; +}`; + const access = "a[msg.data.length]"; + + for (const level of levels) { + it(`should mark the revert with its panic (level ${level})`, async () => { + const result = await compile({ + to: "bytecode", + source, + optimizer: { level }, + }); + if (!result.success) throw new Error("compile failed"); + + const reverts = result.value.bytecode.runtimeProgram.instructions.filter( + (instruction) => instruction.operation?.mnemonic === "REVERT", + ); + expect(reverts).toHaveLength(1); + + const context = reverts[0].context as Record; + expect(context.revert).toEqual({ + panic: 0x32, + reason: { + pointer: { location: "memory", offset: 0x1c, length: 0x24 }, + }, + }); + expect(context.code).toMatchObject({ + range: { offset: source.indexOf(access), length: access.length }, + }); + }); + } +}); diff --git a/packages/bugc/src/evmgen/debug/local-variables.soundness.test.ts b/packages/bugc/src/evmgen/debug/local-variables.soundness.test.ts index 95c9690b13..d3d6d750e3 100644 --- a/packages/bugc/src/evmgen/debug/local-variables.soundness.test.ts +++ b/packages/bugc/src/evmgen/debug/local-variables.soundness.test.ts @@ -966,11 +966,12 @@ code { storage { [0] r: uint256; [1] s: uint256; } create { s = 1; } code { - let b = msg.data[0:4]; + let b = msg.data[0:0]; if (s > 0) { r = b.length; } r = r + b.length; }`, - locals: { b: { shape: { kind: "bytes" }, values: ["0x00000000"] } }, + // Called with no calldata, so a longer slice would revert + locals: { b: { shape: { kind: "bytes" }, values: ["0x"] } }, }, { name: "a uint8 cast", diff --git a/packages/bugc/src/evmgen/generation/instruction.ts b/packages/bugc/src/evmgen/generation/instruction.ts index 2a10742367..1c6a890a3c 100644 --- a/packages/bugc/src/evmgen/generation/instruction.ts +++ b/packages/bugc/src/evmgen/generation/instruction.ts @@ -20,6 +20,7 @@ import { generateWrite, generateAllocate, generateComputeOffset, + generateAssert, } from "./instructions/index.js"; /** @@ -54,6 +55,8 @@ export function generate( return generateAllocate(inst); case "compute_offset": return generateComputeOffset(inst); + case "assert": + return generateAssert(inst); // Call instruction removed - calls are now block terminators default: { // This should be unreachable if all instruction types are handled diff --git a/packages/bugc/src/evmgen/generation/instructions/assert.ts b/packages/bugc/src/evmgen/generation/instructions/assert.ts new file mode 100644 index 0000000000..0211990078 --- /dev/null +++ b/packages/bugc/src/evmgen/generation/instructions/assert.ts @@ -0,0 +1,75 @@ +import type * as Format from "@ethdebug/format"; +import type * as Ir from "#ir"; +import type { Stack } from "#evm"; + +import type { State } from "#evmgen/state"; +import { type Transition, pipe, operations } from "#evmgen/operations"; +import { calculateSize } from "#evmgen/serialize"; + +import { loadValue } from "../values/index.js"; + +const { PUSHn, PUSH2, JUMPI, JUMPDEST, MSTORE, REVERT } = operations; + +/** Selector of Solidity's `Panic(uint256)` error */ +const PANIC_SELECTOR = 0x4e487b71n; + +/** + * Generate code for an assert: jump over the revert if the condition + * holds, else revert with `Panic(code)`, ABI-encoded as Solidity does. + * + * PUSH2 ok JUMPI + * mstore(0, selector) mstore(0x20, code) revert(0x1c, 0x24) + * ok: JUMPDEST + * + * The jump target is a label inside the block; its offset goes in + * `blockOffsets`, so the jump is patched like a jump to a block. + */ +export function generateAssert( + inst: Ir.Instruction.Assert, +): Transition { + const debug = inst.operationDebug; + const revertDebug = { + ...debug, + context: { + ...debug.context, + revert: { + panic: inst.panic, + reason: { + pointer: { location: "memory", offset: 0x1c, length: 0x24 }, + }, + }, + } as Format.Program.Context, + }; + + return ((state: State): State => { + const label = `$assert_${state.nextId}`; + + const loaded = pipe() + .then(loadValue(inst.condition, { debug }), { as: "b" }) + .done()({ ...state, nextId: state.nextId + 1 }); + + const patchIndex = loaded.instructions.length; + const jumped = pipe() + .then(PUSH2([0, 0], { debug }), { as: "counter" }) + .then(JUMPI({ debug })) + .then(PUSHn(PANIC_SELECTOR, { debug }), { as: "value" }) + .then(PUSHn(0n, { debug }), { as: "offset" }) + .then(MSTORE({ debug })) + .then(PUSHn(BigInt(inst.panic), { debug }), { as: "value" }) + .then(PUSHn(0x20n, { debug }), { as: "offset" }) + .then(MSTORE({ debug })) + .then(PUSHn(0x24n, { debug }), { as: "size" }) + .then(PUSHn(0x1cn, { debug }), { as: "offset" }) + .then(REVERT({ debug: revertDebug })) + .done()(loaded); + + return JUMPDEST({ debug })({ + ...jumped, + patches: [...jumped.patches, { index: patchIndex, target: label }], + blockOffsets: { + ...jumped.blockOffsets, + [label]: calculateSize(jumped.instructions), + }, + }); + }) as unknown as Transition; +} diff --git a/packages/bugc/src/evmgen/generation/instructions/index.ts b/packages/bugc/src/evmgen/generation/instructions/index.ts index a952a63dcc..1b16a0c62f 100644 --- a/packages/bugc/src/evmgen/generation/instructions/index.ts +++ b/packages/bugc/src/evmgen/generation/instructions/index.ts @@ -9,3 +9,4 @@ export { generateRead, generateWrite } from "./storage.js"; export { generateComputeSlot } from "./compute-slot.js"; export { generateAllocate } from "./allocate.js"; export { generateComputeOffset } from "./compute-offset.js"; +export { generateAssert } from "./assert.js"; diff --git a/packages/bugc/src/evmgen/memory-arrays.test.ts b/packages/bugc/src/evmgen/memory-arrays.test.ts index 601d37b0df..f3ffe31daf 100644 --- a/packages/bugc/src/evmgen/memory-arrays.test.ts +++ b/packages/bugc/src/evmgen/memory-arrays.test.ts @@ -55,6 +55,15 @@ code { }`, [4n], ], + "take the length of an array literal": [ + `storage { [0] r: uint256; [1] s: uint256; } +create { s = 2; } +code { + r = [1, 2, 3].length; + s = [1, 2, 3][s]; +}`, + [3n, 3n], + ], "read nested elements": [ `storage { [0] r: uint256; [1] s: uint256; [2] n: uint256; } code { diff --git a/packages/bugc/src/ir/analysis/formatter.ts b/packages/bugc/src/ir/analysis/formatter.ts index e6c4f3f1b5..0fea3d5570 100644 --- a/packages/bugc/src/ir/analysis/formatter.ts +++ b/packages/bugc/src/ir/analysis/formatter.ts @@ -196,6 +196,9 @@ export class Formatter { case "length": return `${destWithType(inst.dest)} = length ${this.formatValue(inst.object)}`; + case "assert": + return `assert ${this.formatValue(inst.condition)}, panic 0x${inst.panic.toString(16)}`; + // NEW: unified read instruction case "read": { const location = inst.location; diff --git a/packages/bugc/src/ir/spec/instruction.ts b/packages/bugc/src/ir/spec/instruction.ts index d12ecaa5af..ddc5b6b6ee 100644 --- a/packages/bugc/src/ir/spec/instruction.ts +++ b/packages/bugc/src/ir/spec/instruction.ts @@ -25,7 +25,9 @@ export type Instruction = | Instruction.Hash | Instruction.Cast // Length operations - | Instruction.Length; + | Instruction.Length + // Runtime checks + | Instruction.Assert; export namespace Instruction { export interface Base { @@ -409,4 +411,20 @@ export namespace Instruction { objectDebug?: Instruction.Debug; dest: string; } + + /** + * Revert with Solidity's `Panic(uint256)` error, with code `panic`, + * unless `condition` is nonzero + */ + export interface Assert extends Instruction.Base { + kind: "assert"; + condition: Value; + conditionDebug?: Instruction.Debug; + panic: number; + } + + export namespace Assert { + /** Panic code for an index or slice out of bounds */ + export const outOfBounds = 0x32; + } } diff --git a/packages/bugc/src/ir/utils/debug.ts b/packages/bugc/src/ir/utils/debug.ts index 042fb9c880..0f171abb83 100644 --- a/packages/bugc/src/ir/utils/debug.ts +++ b/packages/bugc/src/ir/utils/debug.ts @@ -307,6 +307,12 @@ export function extractSubInstructionContexts( } break; + case "assert": + if (instruction.conditionDebug?.context) { + contexts.push(instruction.conditionDebug.context); + } + break; + // env has no operands case "env": break; @@ -364,6 +370,9 @@ export function extractSubInstructionContexts( case "length": addValueDebug(instruction.object); break; + case "assert": + addValueDebug(instruction.condition); + break; } return contexts; diff --git a/packages/bugc/src/irgen/generate/expressions/access.ts b/packages/bugc/src/irgen/generate/expressions/access.ts index e252a94efc..ab6ac3772d 100644 --- a/packages/bugc/src/irgen/generate/expressions/access.ts +++ b/packages/bugc/src/irgen/generate/expressions/access.ts @@ -13,7 +13,12 @@ import { findStorageAccessChain, emitStorageChainLoad, } from "../storage.js"; -import { emitMemoryElementOffset, emitMemoryByteOffset } from "../memory.js"; +import { + emitBoundsCheck, + emitLength, + emitMemoryElementOffset, + emitMemoryByteOffset, +} from "../memory.js"; /** * Build an access expression (array/member access) @@ -201,6 +206,11 @@ const makeBuildSliceAccess = ( const start = yield* buildExpression(expr.start, { kind: "rvalue" }); const end = yield* buildExpression(expr.end, { kind: "rvalue" }); + // Revert unless start <= end <= length + const objectLength = yield* emitLength(object, expr); + yield* emitBoundsCheck("le", start, end, expr); + yield* emitBoundsCheck("le", end, objectLength, expr); + // Slicing bytes returns dynamic bytes (memory reference) const resultType: Ir.Type = Ir.Type.Ref.memory(); diff --git a/packages/bugc/src/irgen/generate/expressions/array.ts b/packages/bugc/src/irgen/generate/expressions/array.ts index 3af5b4528c..f29b911297 100644 --- a/packages/bugc/src/irgen/generate/expressions/array.ts +++ b/packages/bugc/src/irgen/generate/expressions/array.ts @@ -152,7 +152,7 @@ export function* buildArray( } as Ir.Instruction.Write); } - return Ir.Value.temp(basePtr, Ir.Type.Scalar.uint256); + return Ir.Value.temp(basePtr, Ir.Type.Ref.memory()); } // Same implementation as above but with proper type @@ -217,7 +217,7 @@ export function* buildArray( } as Ir.Instruction.Write); } - return Ir.Value.temp(basePtr, Ir.Type.Scalar.uint256); + return Ir.Value.temp(basePtr, Ir.Type.Ref.memory()); } } } diff --git a/packages/bugc/src/irgen/generate/memory.ts b/packages/bugc/src/irgen/generate/memory.ts index 2b263eeae3..acf852ff67 100644 --- a/packages/bugc/src/irgen/generate/memory.ts +++ b/packages/bugc/src/irgen/generate/memory.ts @@ -3,7 +3,58 @@ import * as Ir from "#ir"; import { Process } from "./process.js"; /** - * Compute the address of a memory array's element. + * Revert with `Panic(0x32)` unless `left` is less than `right` (or + * equal to it, for `op` "le"), compared as unsigned words: the type + * checker allows only unsigned indexes, and a signed comparison would + * let a negative index through. + */ +export function* emitBoundsCheck( + op: "lt" | "le", + left: Ir.Value, + right: Ir.Value, + node: Ast.Node | undefined, +): Process { + const debug = node ? yield* Process.Debug.forAstNode(node) : {}; + + const condition = yield* Process.Variables.newTemp(); + yield* Process.Instructions.emit({ + kind: "binary", + op, + left: unsigned(left), + right: unsigned(right), + dest: condition, + operationDebug: debug, + } as Ir.Instruction.BinaryOp); + + yield* Process.Instructions.emit({ + kind: "assert", + condition: Ir.Value.temp(condition, Ir.Type.Scalar.bool), + panic: Ir.Instruction.Assert.outOfBounds, + operationDebug: debug, + } as Ir.Instruction.Assert); +} + +/** + * The length of an array or `bytes` in memory: the word at its + * address. (For `msg.data`, code generation reads the calldata size.) + */ +export function* emitLength( + object: Ir.Value, + node: Ast.Node, +): Process { + const length = yield* Process.Variables.newTemp(); + yield* Process.Instructions.emit({ + kind: "length", + object, + dest: length, + operationDebug: yield* Process.Debug.forAstNode(node), + } as Ir.Instruction.Length); + return Ir.Value.temp(length, Ir.Type.Scalar.uint256); +} + +/** + * Compute the address of a memory array's element, reverting if the + * index is out of bounds. * * An array in memory is the address of its length word; one word per * element follows the length. @@ -13,6 +64,9 @@ export function* emitMemoryElementOffset( index: Ir.Value, node: Ast.Node, ): Process { + const length = yield* emitLength(array, node); + yield* emitBoundsCheck("lt", index, length, node); + const debug = yield* Process.Debug.forAstNode(node); const elementsBase = yield* Process.Variables.newTemp(); @@ -41,7 +95,8 @@ export function* emitMemoryElementOffset( } /** - * Compute the address of a byte of `bytes` in memory. + * Compute the address of a byte of `bytes` in memory, reverting if + * the index is out of bounds. * * A `bytes` value in memory is the address of its length word; the * data follows the length. @@ -51,6 +106,9 @@ export function* emitMemoryByteOffset( index: Ir.Value, node: Ast.Node, ): Process { + const length = yield* emitLength(bytes, node); + yield* emitBoundsCheck("lt", index, length, node); + const debug = yield* Process.Debug.forAstNode(node); const dataBase = yield* Process.Variables.newTemp(); @@ -76,3 +134,11 @@ export function* emitMemoryByteOffset( return Ir.Value.temp(offset, Ir.Type.Scalar.uint256); } + +/** `value` as a uint256, so a comparison of it is unsigned */ +function unsigned(value: Ir.Value): Ir.Value { + const type = Ir.Type.Scalar.uint256; + return value.kind === "const" && typeof value.value === "bigint" + ? { ...value, value: BigInt.asUintN(256, value.value), type } + : { ...value, type }; +} diff --git a/packages/bugc/src/irgen/generate/storage.ts b/packages/bugc/src/irgen/generate/storage.ts index be3297e094..83cea74561 100644 --- a/packages/bugc/src/irgen/generate/storage.ts +++ b/packages/bugc/src/irgen/generate/storage.ts @@ -6,6 +6,7 @@ import { Type } from "#types"; import { Error as IrgenError } from "#irgen/errors"; import { fromBugType } from "#irgen/type"; import { Process } from "./process.js"; +import { emitBoundsCheck } from "./memory.js"; import { buildExpression } from "./expressions/index.js"; import type { Context } from "./expressions/context.js"; @@ -192,6 +193,20 @@ export function* emitStorageChainLoad( // Update to the value type currentOrigin = currentOrigin.value; } else if (currentOrigin && Type.isArray(currentOrigin)) { + // A fixed-size array reverts on an index out of bounds. (A + // dynamic array in storage grows when written past its end.) + if (currentOrigin.size !== undefined) { + yield* emitBoundsCheck( + "lt", + access.key, + Ir.Value.constant( + BigInt(currentOrigin.size), + Ir.Type.Scalar.uint256, + ), + node, + ); + } + // Array access - first compute the array's first slot (hash of base) const firstSlotTempId = yield* Process.Variables.newTemp(); yield* Process.Instructions.emit({ @@ -334,6 +349,20 @@ export function* emitStorageChainStore( currentSlot = Ir.Value.temp(slotTemp, Ir.Type.Scalar.uint256); currentOrigin = currentOrigin.value; } else if (currentOrigin && Type.isArray(currentOrigin)) { + // A fixed-size array reverts on an index out of bounds. (A + // dynamic array in storage grows when written past its end.) + if (currentOrigin.size !== undefined) { + yield* emitBoundsCheck( + "lt", + access.key, + Ir.Value.constant( + BigInt(currentOrigin.size), + Ir.Type.Scalar.uint256, + ), + node, + ); + } + // Array access - first compute the array's first slot (hash of base) const firstSlotTemp = yield* Process.Variables.newTemp(); yield* Process.Instructions.emit({ diff --git a/packages/bugc/src/optimizer/steps/common-subexpression-elimination.ts b/packages/bugc/src/optimizer/steps/common-subexpression-elimination.ts index d5b771792e..b3fc8311f3 100644 --- a/packages/bugc/src/optimizer/steps/common-subexpression-elimination.ts +++ b/packages/bugc/src/optimizer/steps/common-subexpression-elimination.ts @@ -242,6 +242,9 @@ export class CommonSubexpressionEliminationStep extends BaseOptimizationStep { case "length": result.object = replaceValue(result.object); break; + case "assert": + result.condition = replaceValue(result.condition); + break; } return result; diff --git a/packages/bugc/src/optimizer/steps/constant-folding.test.ts b/packages/bugc/src/optimizer/steps/constant-folding.test.ts index a6f94fa993..3486c037fd 100644 --- a/packages/bugc/src/optimizer/steps/constant-folding.test.ts +++ b/packages/bugc/src/optimizer/steps/constant-folding.test.ts @@ -209,4 +209,53 @@ describe("ConstantFoldingStep", () => { expect(foldMod(7n, 0n)).toMatchObject({ kind: "binary", op: "mod" }); }); }); + + const context: OptimizationContext = { + trackTransformation: () => {}, + getTransformations: () => [], + getAnalysis: () => undefined, + setAnalysis: () => {}, + }; + + const uint = (value: bigint): Ir.Value => + Ir.Value.constant(value, Ir.Type.Scalar.uint256); + + it("should remove an assert only if its condition is a true constant", () => { + const assert = (condition: Ir.Value): Ir.Instruction => ({ + kind: "assert", + condition, + panic: 0x32, + operationDebug: {}, + }); + const module = createTestModule([ + { + kind: "binary", + op: "lt", + left: uint(2n), + right: uint(3n), + dest: "t0", + operationDebug: {}, + }, + assert(Ir.Value.temp("t0", Ir.Type.Scalar.bool)), + { + kind: "binary", + op: "lt", + left: uint(3n), + right: uint(3n), + dest: "t1", + operationDebug: {}, + }, + assert(Ir.Value.temp("t1", Ir.Type.Scalar.bool)), + assert(Ir.Value.temp("t2", Ir.Type.Scalar.bool)), + ]); + + const block = step.run(module, context).main.blocks.get("entry")!; + + expect(block.instructions.map((inst) => inst.kind)).toEqual([ + "const", + "const", + "assert", + "assert", + ]); + }); }); diff --git a/packages/bugc/src/optimizer/steps/constant-folding.ts b/packages/bugc/src/optimizer/steps/constant-folding.ts index 5035f77a21..e70dcc885b 100644 --- a/packages/bugc/src/optimizer/steps/constant-folding.ts +++ b/packages/bugc/src/optimizer/steps/constant-folding.ts @@ -90,6 +90,18 @@ export class ConstantFoldingStep extends BaseOptimizationStep { } else { newInstructions.push(inst); } + } else if ( + inst.kind === "assert" && + this.holds(inst.condition, constants) + ) { + // A check that always passes does nothing + context.trackTransformation({ + type: "delete", + pass: this.name, + original: Ir.Utils.extractContexts(inst), + result: [], + reason: `Removed assert of a true constant`, + }); } else if (inst.kind === "length" && this.canFoldLength(inst)) { // Try to fold length operation const folded = this.foldLength(inst); @@ -331,6 +343,17 @@ export class ConstantFoldingStep extends BaseOptimizationStep { }; } + /** Whether `value` is a constant that is not zero or false */ + private holds( + value: Ir.Value, + constants: Map, + ): boolean { + const constant = this.getConstantValue(value, constants); + return ( + constant === true || (typeof constant === "bigint" && constant !== 0n) + ); + } + private canFoldLength(inst: Ir.Instruction): boolean { if (inst.kind !== "length") return false; diff --git a/packages/bugc/src/optimizer/steps/constant-propagation.ts b/packages/bugc/src/optimizer/steps/constant-propagation.ts index 261cea22fa..8f4c000f08 100644 --- a/packages/bugc/src/optimizer/steps/constant-propagation.ts +++ b/packages/bugc/src/optimizer/steps/constant-propagation.ts @@ -149,6 +149,9 @@ export class ConstantPropagationStep extends BaseOptimizationStep { case "length": result.object = propagateValue(result.object); break; + case "assert": + result.condition = propagateValue(result.condition); + break; } // Check if we actually changed anything diff --git a/packages/bugc/src/optimizer/steps/dead-code-elimination.ts b/packages/bugc/src/optimizer/steps/dead-code-elimination.ts index 88eada2f53..da5a55d6e7 100644 --- a/packages/bugc/src/optimizer/steps/dead-code-elimination.ts +++ b/packages/bugc/src/optimizer/steps/dead-code-elimination.ts @@ -136,6 +136,9 @@ export class DeadCodeEliminationStep extends BaseOptimizationStep { case "length": this.collectValueUse(inst.object, used); break; + case "assert": + this.collectValueUse(inst.condition, used); + break; case "compute_offset": this.collectValueUse(inst.base, used); if (Ir.Instruction.ComputeOffset.isArray(inst)) { @@ -166,6 +169,7 @@ export class DeadCodeEliminationStep extends BaseOptimizationStep { switch (inst.kind) { case "write": case "allocate": // Allocate modifies the free memory pointer + case "assert": // Assert may revert return true; default: return false; diff --git a/packages/bugc/src/typechecker/checker.test.ts b/packages/bugc/src/typechecker/checker.test.ts index 57c8bce8c1..d3551403e5 100644 --- a/packages/bugc/src/typechecker/checker.test.ts +++ b/packages/bugc/src/typechecker/checker.test.ts @@ -337,6 +337,38 @@ describe("checkProgram", () => { message: "Array index must be numeric", }); }); + + it("should reject a signed index", () => { + const programs: Record = { + "an array read": ["let x = a[i];", "Array index"], + "an array write": ["a[i] = 1;", "Array index"], + "a bytes read": ["let x = b[i];", "Bytes index"], + "a bytes write": ["b[i] = 1 as uint8;", "Bytes index"], + "a slice start": ["let x = b[i:2];", "Slice start index"], + "a slice end": ["let x = b[0:i];", "Slice end index"], + }; + + for (const [name, [statement, what]] of Object.entries(programs)) { + const result = check(` + name Test; + code { + let a: array = [1, 2, 3]; + let b = msg.data[0:3]; + let i: int256 = 1 as int256; + ${statement} + } + `); + + expect(result.success, name).toBe(false); + expect(Result.countErrors(result), name).toBe(1); + expect(result).toHaveMessage({ + severity: Severity.Error, + message: + `${what} must be an unsigned integer, not int256; ` + + "cast it, as in `i as uint256`", + }); + } + }); }); describe("Control Flow", () => { diff --git a/packages/bugc/src/typechecker/expressions.ts b/packages/bugc/src/typechecker/expressions.ts index 81fe0c6be5..d01fa9c74b 100644 --- a/packages/bugc/src/typechecker/expressions.ts +++ b/packages/bugc/src/typechecker/expressions.ts @@ -428,12 +428,13 @@ export const expressionChecker: Pick, "expression"> = { Type.isElementary(objectType) && Type.Elementary.isBytes(objectType) ) { - if ( - !Type.isElementary(startResult.type) || - !Type.Elementary.isNumeric(startResult.type) - ) { + const startError = indexTypeError( + "Slice start index", + startResult.type, + ); + if (startError) { const error = new TypeError( - "Slice start index must be numeric", + startError, startExpr.loc || undefined, undefined, undefined, @@ -441,12 +442,10 @@ export const expressionChecker: Pick, "expression"> = { ); errors.push(error); } - if ( - !Type.isElementary(endResult.type) || - !Type.Elementary.isNumeric(endResult.type) - ) { + const endError = indexTypeError("Slice end index", endResult.type); + if (endError) { const error = new TypeError( - "Slice end index must be numeric", + endError, endExpr.loc || undefined, undefined, undefined, @@ -490,12 +489,10 @@ export const expressionChecker: Pick, "expression"> = { const indexType = indexResult.type; if (Type.isArray(objectType)) { - if ( - !Type.isElementary(indexType) || - !Type.Elementary.isNumeric(indexType) - ) { + const indexError = indexTypeError("Array index", indexType); + if (indexError) { const error = new TypeError( - "Array index must be numeric", + indexError, indexExpr.loc || undefined, undefined, undefined, @@ -521,13 +518,14 @@ export const expressionChecker: Pick, "expression"> = { Type.Elementary.isBytes(objectType) ) { // Allow indexing into bytes types - returns uint8 - if (!isAssignable(Type.Elementary.uint(8), indexType)) { + const indexError = indexTypeError("Bytes index", indexType); + if (indexError) { const error = new TypeError( - `Bytes index must be a numeric type, got ${Type.format(indexType)}`, + indexError, indexExpr.loc || undefined, undefined, undefined, - ErrorCode.TYPE_MISMATCH, + ErrorCode.INVALID_INDEX_TYPE, ); errors.push(error); } @@ -1204,3 +1202,20 @@ function checkSignedness( ), ]; } + +/** + * Why `type` cannot index an array or bytes, if it cannot: as in + * Solidity, an index must be an unsigned integer + */ +function indexTypeError(what: string, type: Type): string | undefined { + if (!Type.isElementary(type) || !Type.Elementary.isNumeric(type)) { + return `${what} must be numeric`; + } + if (Type.Elementary.isInt(type)) { + return ( + `${what} must be an unsigned integer, not ${Type.format(type)}; ` + + `cast it, as in \`i as uint256\`` + ); + } + return undefined; +} diff --git a/packages/web/docs/implementation-guides/compiler/case-study-bug.mdx b/packages/web/docs/implementation-guides/compiler/case-study-bug.mdx index 0e9c79d057..749054fec2 100644 --- a/packages/web/docs/implementation-guides/compiler/case-study-bug.mdx +++ b/packages/web/docs/implementation-guides/compiler/case-study-bug.mdx @@ -106,6 +106,25 @@ so `x + 1` with `x: uint8` is a `uint8` and wraps at 8 bits. -(-128 as int8); // -128 ``` +### Bounds + +An index out of bounds reverts, as in Solidity. `a[i]` on an array or on +dynamic `bytes` in memory, or on a fixed-size array in storage, reverts +unless `i` is less than the length; this holds for reads and writes, and +at each level of a nested array. A slice `b[s:e]` reverts unless +`s <= e <= b.length`. An index or slice bound must be an unsigned +integer; cast a signed one, as in `a[i as uint256]`. The revert data is Solidity's `Panic(0x32)`, and +the `REVERT` carries a `revert` context with `panic: 50` and a pointer to +that data. A dynamic array in storage is not checked: BUG has no `push`, +so a write past the end is how such an array grows. + +```bug +let a: array = [1, 2, 3]; +a[2] = 30; // ok +a[3] = 40; // reverts with Panic(0x32) +msg.data[0:4]; // reverts if the calldata is shorter +``` + ## Compilation pipeline BUG uses a multi-stage compilation pipeline: