diff --git a/packages/bugc/examples/basic/functions.bug b/packages/bugc/examples/basic/functions.bug index 62b982377..6d217a775 100644 --- a/packages/bugc/examples/basic/functions.bug +++ b/packages/bugc/examples/basic/functions.bug @@ -33,7 +33,6 @@ code { // Store result result = total; /*@test function-call-result - fails: stack underflow in internal function calls variables: result: pointer: { location: storage, slot: 0 } diff --git a/packages/bugc/src/evmgen/analysis/layout.ts b/packages/bugc/src/evmgen/analysis/layout.ts index 24c9caa9e..10748ddb3 100644 --- a/packages/bugc/src/evmgen/analysis/layout.ts +++ b/packages/bugc/src/evmgen/analysis/layout.ts @@ -126,6 +126,9 @@ function dfsOrder( const trueBranch = dfsOrder(func, term.trueTarget, visited); const falseBranch = dfsOrder(func, term.falseTarget, visited); return [blockId, ...trueBranch, ...falseBranch]; + } else if (term.kind === "call") { + // Follow continuation block + return [blockId, ...dfsOrder(func, term.continuation, visited)]; } else { return [blockId]; } diff --git a/packages/bugc/src/evmgen/analysis/liveness.ts b/packages/bugc/src/evmgen/analysis/liveness.ts index 95e2871a7..60aa09a7a 100644 --- a/packages/bugc/src/evmgen/analysis/liveness.ts +++ b/packages/bugc/src/evmgen/analysis/liveness.ts @@ -78,6 +78,15 @@ export namespace Function { uses.add(retId); } lastUse.set(retId, `${blockId}:return`); + } else if (term.kind === "call") { + // Track call arguments as uses + for (const arg of term.arguments) { + const argId = valueId(arg); + if (!defs.has(argId)) { + uses.add(argId); + } + lastUse.set(argId, `${blockId}:call`); + } } blockUses.set(blockId, uses); @@ -135,6 +144,23 @@ export namespace Function { } } } + } else if (term.kind === "call") { + // Continuation block is a successor + const contIn = liveIn.get(term.continuation); + if (contIn) { + for (const val of contIn) newOut.add(val); + } + // Add phi sources for continuation + const contBlock = func.blocks.get(term.continuation); + if (contBlock) { + for (const phi of contBlock.phis) { + const source = phi.sources.get(blockId); + if (source && source.kind !== "const") { + newOut.add(valueId(source)); + crossBlockValues.add(valueId(source)); + } + } + } } liveOut.set(blockId, newOut); diff --git a/packages/bugc/src/evmgen/analysis/memory.ts b/packages/bugc/src/evmgen/analysis/memory.ts index 5305d8736..9b455c743 100644 --- a/packages/bugc/src/evmgen/analysis/memory.ts +++ b/packages/bugc/src/evmgen/analysis/memory.ts @@ -92,7 +92,11 @@ export namespace Module { } result.main = mainMemory.value; - // Process user-defined functions + // Process user-defined functions. + // Each function's allocations start after the previous + // function's end, so all simultaneously-active frames + // have non-overlapping memory. + let nextFuncOffset = result.main.nextStaticOffset; for (const [name, func] of module.functions) { const funcLiveness = liveness.functions[name]; if (!funcLiveness) { @@ -103,11 +107,15 @@ export namespace Module { ), ); } - const funcMemory = Function.plan(func, funcLiveness); + const funcMemory = Function.plan(func, funcLiveness, { + isUserFunction: true, + startOffset: nextFuncOffset, + }); if (!funcMemory.success) { return funcMemory; } result.functions[name] = funcMemory.value; + nextFuncOffset = funcMemory.value.nextStaticOffset; } return Result.ok(result); @@ -120,6 +128,12 @@ export namespace Function { allocations: Record; /** Next available memory offset after all static allocations */ nextStaticOffset: number; + /** + * Offset where this function saves its return PC. + * Only set for user-defined functions that need to + * preserve their return address across internal calls. + */ + savedReturnPcOffset?: number; } /** @@ -128,10 +142,14 @@ export namespace Function { export function plan( func: Ir.Function, liveness: Liveness.Function.Info, + options: { + isUserFunction?: boolean; + startOffset?: number; + } = {}, ): Result { try { const allocations: Record = {}; - let nextStaticOffset = regions.STATIC_MEMORY_START; + let nextStaticOffset = options.startOffset ?? regions.STATIC_MEMORY_START; const needsMemory = identifyMemoryValues(func, liveness); @@ -194,9 +212,19 @@ export namespace Function { nextStaticOffset = currentSlotOffset; } + // Reserve a slot for the saved return PC in user + // functions. This is needed because nested calls + // overwrite memory[0x60] with their own return PC. + let savedReturnPcOffset: number | undefined; + if (options.isUserFunction) { + savedReturnPcOffset = nextStaticOffset; + nextStaticOffset += SLOT_SIZE; + } + return Result.ok({ allocations, nextStaticOffset, + savedReturnPcOffset, }); } catch (error) { return Result.err( @@ -396,6 +424,11 @@ function getValueType(valueId: string, func: Ir.Function): Ir.Type | undefined { } } } + + // Check call terminator dest (return value) + if (block.terminator.kind === "call" && block.terminator.dest === valueId) { + return Ir.Type.Scalar.uint256; + } } return undefined; @@ -485,6 +518,21 @@ function identifyMemoryValues( } } } + + // Call terminator arguments need memory because the + // call convention cleans the stack before loading args. + // Non-const args must be reloaded from memory. + if (term.kind === "call") { + for (const arg of term.arguments) { + if (arg.kind !== "const") { + const argId = valueId(arg); + const type = getValueType(argId, func); + if (type) { + needsMemory.set(argId, type); + } + } + } + } } return needsMemory; diff --git a/packages/bugc/src/evmgen/behavioral.test.ts b/packages/bugc/src/evmgen/behavioral.test.ts index 18c10db35..7f842ee01 100644 --- a/packages/bugc/src/evmgen/behavioral.test.ts +++ b/packages/bugc/src/evmgen/behavioral.test.ts @@ -102,9 +102,7 @@ code { }); describe("internal functions", () => { - // Internal function calls currently fail at runtime - // (stack underflow). Tracked as known issue. - it.skip("should call defined functions", async () => { + it("should call defined functions", async () => { const source = `name FuncCall; define { @@ -132,6 +130,136 @@ code { expect(result.callSuccess).toBe(true); expect(await result.getStorage(0n)).toBe(30n); }); + + it("should call a single-arg function", async () => { + const source = `name SingleArgFunc; + +define { + function double(x: uint256) -> uint256 { + return x + x; + }; +} + +storage { + [0] result: uint256; +} + +create { + result = 0; +} + +code { + result = double(7); +}`; + + const result = await executeProgram(source, { + calldata: "", + }); + + expect(result.callSuccess).toBe(true); + expect(await result.getStorage(0n)).toBe(14n); + }); + + it("should call multiple functions", async () => { + const source = `name MultiFuncCall; + +define { + function double(x: uint256) -> uint256 { + return x + x; + }; + function triple(x: uint256) -> uint256 { + return x + x + x; + }; +} + +storage { + [0] a: uint256; + [1] b: uint256; +} + +create { + a = 0; + b = 0; +} + +code { + a = double(7); + b = triple(5); +}`; + + const result = await executeProgram(source, { + calldata: "", + }); + + expect(result.callSuccess).toBe(true); + expect(await result.getStorage(0n)).toBe(14n); + expect(await result.getStorage(1n)).toBe(15n); + }); + + it("should call a function from another function", async () => { + const source = `name FuncFromFunc; + +define { + function add(a: uint256, b: uint256) -> uint256 { + return a + b; + }; + function addThree(x: uint256, y: uint256, z: uint256) -> uint256 { + let sum1 = add(x, y); + let sum2 = add(sum1, z); + return sum2; + }; +} + +storage { + [0] result: uint256; +} + +create { + result = 0; +} + +code { + result = addThree(10, 20, 30); +}`; + + const result = await executeProgram(source, { + calldata: "", + }); + + expect(result.callSuccess).toBe(true); + expect(await result.getStorage(0n)).toBe(60n); + }); + + it("should call a function in a loop", async () => { + const source = `name FuncInLoop; + +define { + function increment(x: uint256) -> uint256 { + return x + 1; + }; +} + +storage { + [0] total: uint256; +} + +create { + total = 0; +} + +code { + for (let i = 0; i < 3; i = i + 1) { + total = increment(total); + } +}`; + + const result = await executeProgram(source, { + calldata: "", + }); + + expect(result.callSuccess).toBe(true); + expect(await result.getStorage(0n)).toBe(3n); + }); }); describe("loops", () => { diff --git a/packages/bugc/src/evmgen/generation/block.ts b/packages/bugc/src/evmgen/generation/block.ts index 3d540a13d..e425212fb 100644 --- a/packages/bugc/src/evmgen/generation/block.ts +++ b/packages/bugc/src/evmgen/generation/block.ts @@ -79,7 +79,9 @@ export function generate( result = result.then(JUMPDEST()); } - // Annotate TOS with dest variable if this is a continuation with return value + // Annotate TOS with dest variable if this is a continuation with return value. + // Also spill to memory if allocated, so the value survives stack cleanup + // before any subsequent call terminators. if (func && predecessor) { const predBlock = func.blocks.get(predecessor); if ( @@ -87,8 +89,28 @@ export function generate( predBlock.terminator.continuation === block.id && predBlock.terminator.dest ) { - // TOS has the return value, annotate it - result = result.then(annotateTop(predBlock.terminator.dest)); + const destId = predBlock.terminator.dest; + result = result.then(annotateTop(destId)).then((s) => { + const allocation = s.memory.allocations[destId]; + if (!allocation) return s; + // Spill return value to memory: DUP1, PUSH offset, MSTORE + return { + ...s, + instructions: [ + ...s.instructions, + { mnemonic: "DUP1" as const, opcode: 0x80 }, + { + mnemonic: "PUSH2" as const, + opcode: 0x61, + immediates: [ + (allocation.offset >> 8) & 0xff, + allocation.offset & 0xff, + ], + }, + { mnemonic: "MSTORE" as const, opcode: 0x52 }, + ], + }; + }); } } } diff --git a/packages/bugc/src/evmgen/generation/control-flow/terminator.ts b/packages/bugc/src/evmgen/generation/control-flow/terminator.ts index 39fa8f46c..44c4792da 100644 --- a/packages/bugc/src/evmgen/generation/control-flow/terminator.ts +++ b/packages/bugc/src/evmgen/generation/control-flow/terminator.ts @@ -23,28 +23,28 @@ export function generateTerminator( // on TOS from the previous instruction in the block. This avoids an // unnecessary DUP that would leave an extra value on the stack. if (isUserFunction) { - const returnDebug = { - context: { - remark: term.value - ? "function-return: return with value" - : "function-return: void return", - }, - }; - if (term.value) { - // Return with value (assume value already on TOS) - return pipe() - .then(PUSHn(0x60n, { debug: returnDebug }), { as: "offset" }) - .then(MLOAD({ debug: returnDebug }), { as: "counter" }) - .then(JUMP({ debug: returnDebug })) - .done() as unknown as Transition; - } else { - // Return without value (void return) - return pipe() - .then(PUSHn(0x60n, { debug: returnDebug }), { as: "offset" }) - .then(MLOAD({ debug: returnDebug }), { as: "counter" }) - .then(JUMP({ debug: returnDebug })) - .done() as unknown as Transition; - } + // Load return PC from the saved slot (not 0x60, + // which may have been overwritten by nested calls). + return pipe() + .peek((state, builder) => { + const pcOffset = state.memory.savedReturnPcOffset ?? 0x60; + const returnDebug = { + context: { + remark: term.value + ? "function-return: return with value" + : "function-return: void return", + }, + }; + return builder + .then(PUSHn(BigInt(pcOffset), { debug: returnDebug }), { + as: "offset", + }) + .then(MLOAD({ debug: returnDebug }), { + as: "counter", + }) + .then(JUMP({ debug: returnDebug })); + }) + .done() as unknown as Transition; } // Contract return (main function or create) @@ -157,6 +157,31 @@ export function generateCallTerminator( return ((state: State): State => { let currentState: State = state as State; + + // Clean the stack before setting up the call. + // Values produced by block instructions that are only + // used as call arguments will have been DUP'd by + // loadValue, leaving originals behind. Since this is a + // block terminator, all current stack values are dead + // after the call — POP them so the function receives a + // clean stack with only its arguments. + const cleanupDebug = { + context: { + remark: `call-preparation: clean stack for ${funcName}`, + }, + }; + while (currentState.stack.length > 0) { + currentState = { + ...currentState, + instructions: [ + ...currentState.instructions, + { mnemonic: "POP", opcode: 0x50, debug: cleanupDebug }, + ], + stack: currentState.stack.slice(1), + brands: currentState.brands.slice(1) as Stack, + }; + } + const returnPcPatchIndex = currentState.instructions.length; // Store return PC to memory at 0x60 @@ -188,7 +213,9 @@ export function generateCallTerminator( ], }; - // Push arguments using loadValue + // Push arguments using loadValue. + // Stack is clean, so loadValue will reload from memory + // (for temps) or re-push (for consts). const argsDebug = { context: { remark: `call-arguments: push ${args.length} argument(s) for ${funcName}`, @@ -227,6 +254,23 @@ export function generateCallTerminator( ], }; + // Reset stack tracking to match the runtime state at the + // continuation block. The function call consumes all args + // and leaves just the return value (if any) on the stack. + if (term.dest) { + currentState = { + ...currentState, + stack: [{ id: `call_return_${funcName}`, irValue: term.dest }], + brands: ["value" as const] as unknown as Stack, + }; + } else { + currentState = { + ...currentState, + stack: [], + brands: [] as unknown as Stack, + }; + } + return currentState; }) as Transition; } diff --git a/packages/bugc/src/evmgen/generation/function.ts b/packages/bugc/src/evmgen/generation/function.ts index 94bb3ff4d..59e068bf8 100644 --- a/packages/bugc/src/evmgen/generation/function.ts +++ b/packages/bugc/src/evmgen/generation/function.ts @@ -8,6 +8,7 @@ import type { Stack } from "#evm"; import type { State } from "#evmgen/state"; import type { Layout, Memory } from "#evmgen/analysis"; +import type { Error as EvmgenError } from "#evmgen/errors"; import * as Block from "./block.js"; import { serialize } from "../serialize.js"; @@ -83,6 +84,38 @@ function generatePrologue( }; } + // Save the return PC from 0x60 to a dedicated slot + // so nested function calls don't clobber it. + const savedPcOffset = currentState.memory.savedReturnPcOffset; + if (savedPcOffset !== undefined) { + const savePcDebug = { + context: { + remark: `prologue: save return PC to 0x${savedPcOffset.toString(16)}`, + }, + }; + const highByte = (savedPcOffset >> 8) & 0xff; + const lowByte = savedPcOffset & 0xff; + currentState = { + ...currentState, + instructions: [ + ...currentState.instructions, + { + mnemonic: "PUSH1", + opcode: 0x60, + immediates: [0x60], + debug: savePcDebug, + }, + { mnemonic: "MLOAD", opcode: 0x51 }, + { + mnemonic: "PUSH2", + opcode: 0x61, + immediates: [highByte, lowByte], + }, + { mnemonic: "MSTORE", opcode: 0x52 }, + ], + }; + } + // Return with empty stack return { ...currentState, @@ -100,7 +133,13 @@ export function generate( memory: Memory.Function.Info, layout: Layout.Function.Info, options: { isUserFunction?: boolean } = {}, -) { +): { + instructions: Evm.Instruction[]; + bytecode: number[]; + warnings: EvmgenError[]; + patches: State["patches"]; + blockOffsets: Record; +} { const initialState: State = { brands: [], stack: [], @@ -146,6 +185,21 @@ export function generate( stateAfterPrologue, ); + // For user functions, defer block/continuation patching + // to module level where the function's base offset is known. + // Block offsets computed here are relative to the function + // start; EVM JUMP needs absolute program counter values. + if (options.isUserFunction) { + const bytecode = serialize(finalState.instructions); + return { + instructions: finalState.instructions, + bytecode, + warnings: finalState.warnings, + patches: finalState.patches, + blockOffsets: finalState.blockOffsets, + }; + } + // Patch block jump targets (not function calls yet) const patchedState = patchJumps(finalState); @@ -157,6 +211,7 @@ export function generate( bytecode, warnings: patchedState.warnings, patches: finalState.patches, // Return patches for module-level patching + blockOffsets: finalState.blockOffsets, }; } @@ -196,26 +251,46 @@ function patchJumps(state: State): State { } /** - * Patch function call addresses in bytecode + * Patch function call addresses in bytecode. + * + * When `baseOffset` is provided (for user-defined functions), + * block/continuation patches are also resolved by adding + * `baseOffset` to each block-relative target. This is + * necessary because block offsets are computed relative to + * the function's instruction stream, but EVM JUMP needs + * absolute program counter values. */ export function patchFunctionCalls( bytecode: number[], instructions: Evm.Instruction[], patches: State["patches"], functionRegistry: Record, + options: { + baseOffset?: number; + blockOffsets?: Record; + } = {}, ): { bytecode: number[]; instructions: Evm.Instruction[] } { const patchedInstructions = [...instructions]; const patchedBytecode = [...bytecode]; for (const patch of patches) { - // Only handle function patches - if (patch.type !== "function") { - continue; - } + let targetOffset: number | undefined; - const targetOffset = functionRegistry[patch.target]; - if (targetOffset === undefined) { - throw new Error(`Function ${patch.target} not found in registry`); + if (patch.type === "function") { + targetOffset = functionRegistry[patch.target]; + if (targetOffset === undefined) { + throw new Error(`Function ${patch.target} not found in registry`); + } + } else if (options.baseOffset !== undefined && options.blockOffsets) { + // Block or continuation patch with base offset + const blockOffset = options.blockOffsets[patch.target]; + if (blockOffset === undefined) { + throw new Error(`Jump target ${patch.target} not found`); + } + targetOffset = blockOffset + options.baseOffset; + } else { + // Skip non-function patches when no base offset + continue; } // Convert offset to bytes for PUSH2 (2 bytes, big-endian) diff --git a/packages/bugc/src/evmgen/generation/instructions/storage.ts b/packages/bugc/src/evmgen/generation/instructions/storage.ts index c49ce52a6..55b1b6c8a 100644 --- a/packages/bugc/src/evmgen/generation/instructions/storage.ts +++ b/packages/bugc/src/evmgen/generation/instructions/storage.ts @@ -27,8 +27,10 @@ const { } = operations; // Scratch memory address for copy-based reads (returndata, code). -// Uses the "zero slot" at 0x60, which is safe for temporary use. -const SCRATCH_OFFSET = 0x60n; +// Uses scratch space at 0x00 (Solidity convention). Must NOT use +// 0x60 because the function calling convention stores the return +// PC there. +const SCRATCH_OFFSET = 0x00n; /** * Generate code for the new unified read instruction @@ -229,10 +231,10 @@ function generateCalldataRead( /** * Copy-based read for returndata and code locations. * Uses RETURNDATACOPY or CODECOPY to copy data to scratch - * memory at 0x60, then MLOAD to read. + * memory at 0x00, then MLOAD to read. * * Stack effect: copies `length` bytes from `offset` in the - * source to memory[0x60], then loads the 32-byte word. + * source to memory[0x00], then loads the 32-byte word. */ function generateCopyBasedRead( inst: Ir.Instruction.Read, diff --git a/packages/bugc/src/evmgen/generation/module.ts b/packages/bugc/src/evmgen/generation/module.ts index ba24e7172..92c306f8a 100644 --- a/packages/bugc/src/evmgen/generation/module.ts +++ b/packages/bugc/src/evmgen/generation/module.ts @@ -43,6 +43,7 @@ export function generate( bytecode: number[]; instructions: Evm.Instruction[]; patches: typeof runtimeResult.patches; + blockOffsets: Record; }> = []; for (const [name, func] of module.functions.entries()) { @@ -58,14 +59,18 @@ export function generate( bytecode: funcResult.bytecode, instructions: funcResult.instructions, patches: funcResult.patches, + blockOffsets: funcResult.blockOffsets, }); allWarnings = [...allWarnings, ...funcResult.warnings]; } } - // Build function registry with offsets + // Build function registry with offsets. + // Add 1 byte for the STOP guard inserted between the + // main function and user-defined functions. + const stopGuardSize = functionResults.length > 0 ? 1 : 0; const functionRegistry: Record = {}; - let currentOffset = runtimeResult.bytecode.length; + let currentOffset = runtimeResult.bytecode.length + stopGuardSize; for (const funcResult of functionResults) { functionRegistry[funcResult.name] = currentOffset; currentOffset += funcResult.bytecode.length; @@ -79,23 +84,41 @@ export function generate( functionRegistry, ); - // Patch function calls in user-defined functions + // Patch function calls and block jumps in user-defined + // functions. Block/continuation patches need the function's + // base offset added since block offsets are relative to + // the function start, but EVM JUMP needs absolute PC values. const patchedFunctions = functionResults.map((funcResult) => Function.patchFunctionCalls( funcResult.bytecode, funcResult.instructions, funcResult.patches, functionRegistry, + { + baseOffset: functionRegistry[funcResult.name], + blockOffsets: funcResult.blockOffsets, + }, ), ); - // Combine runtime with user functions + // Combine runtime with user functions. + // Insert STOP between main and user functions to prevent + // fall-through when the main function's last block omits + // STOP (the isLastBlock optimization). + const stopGuard: Evm.Instruction[] = + patchedFunctions.length > 0 + ? [{ mnemonic: "STOP" as const, opcode: 0x00 }] + : []; + const stopGuardBytes: number[] = patchedFunctions.length > 0 ? [0x00] : []; + const allRuntimeBytes = [ ...patchedRuntime.bytecode, + ...stopGuardBytes, ...patchedFunctions.flatMap((f) => f.bytecode), ]; const allRuntimeInstructions = [ ...patchedRuntime.instructions, + ...stopGuard, ...patchedFunctions.flatMap((f) => f.instructions), ];