bugc: keep an inlined body's control flow inside the body (#356)

Author: gnidan

packages/bugc/CHANGELOG.md

@@ -79,6 +79,16 @@ support. Changes to the specification itself are tracked in the root
 
 ### Fixed
 
+- At optimization levels 2 and 3, an inlined body's control flow now has
+  a `transform: ["inline"]` context: the branch of an `if`, the jumps
+  between the body's blocks, each block's `JUMPDEST`, and the jump that
+  carries the body's `return`. Before, only the body's other instructions
+  had it, so a debugger that reads the transform closed the inlined frame
+  at the body's first branch ([#356]).
+- The block that bugc puts on a branch edge into a block with phis now
+  has the branch's context: its source range, its variables (the storage
+  variables too) and its transforms. Before, it had no context, so a
+  debugger listed no variables while it ran ([#356]).
 - A call to a function with no return type now compiles as a statement,
   as in `bump();`, at every optimization level. The function's `return`
   context has no `data`. Before, IR generation failed with "Cannot convert
@@ -290,3 +300,4 @@ First publication.
 [#349]: https://github.com/ethdebug/format/pull/349
 [#351]: https://github.com/ethdebug/format/pull/351
 [#352]: https://github.com/ethdebug/format/pull/352
+[#356]: https://github.com/ethdebug/format/pull/356

packages/bugc/src/evmgen/generation/block.ts

@@ -84,8 +84,11 @@ export function generate<S extends Stack>(
           }
         }
 
+        // A block of an inlined body is entered inside that body
+        const entryDebug = withEntryInline(block);
+
         if (callSiteCode) {
-          const entry = block.entryDebug;
+          const entry = entryDebug;
           const continuationDebug = {
             ...entry,
             context: {
@@ -95,7 +98,7 @@ export function generate<S extends Stack>(
           };
           result = result.then(JUMPDEST({ debug: continuationDebug }));
         } else {
-          result = result.then(JUMPDEST({ debug: block.entryDebug }));
+          result = result.then(JUMPDEST({ debug: entryDebug }));
         }
 
         // Annotate TOS with dest variable if this is a continuation with return value.
@@ -262,6 +265,46 @@ export function generate<S extends Stack>(
     .done();
 }
 
+/**
+ * A block's entry debug, with an `inline` transform for each inlined
+ * body the block's entry is in: each one its first operation is in,
+ * but for those whose invoke it carries (its entry is just before
+ * them).
+ */
+function withEntryInline(block: Ir.Block): Ir.Block.Debug | undefined {
+  const first = block.instructions[0] ?? block.terminator;
+  const context = first.operationDebug?.context;
+  const depth =
+    inlineCount(context) - Ir.Utils.activationsOf(context, "invoke").length;
+  if (depth <= 0) return block.entryDebug;
+  return {
+    ...block.entryDebug,
+    ...Ir.Utils.addTransform(
+      block.entryDebug,
+      ...Array<"inline">(depth).fill("inline"),
+    ),
+  };
+}
+
+/** How many `inline` transforms a context has, gathered ones too */
+function inlineCount(context: Format.Program.Context | undefined): number {
+  if (!context || typeof context !== "object") return 0;
+  const { transform, gather } = context as {
+    transform?: unknown;
+    gather?: unknown;
+  };
+  const own = Array.isArray(transform)
+    ? transform.filter((id) => id === "inline").length
+    : 0;
+  return Array.isArray(gather)
+    ? own +
+        (gather as Format.Program.Context[]).reduce(
+          (sum, c) => sum + inlineCount(c),
+          0,
+        )
+    : own;
+}
+
 /**
  * Generate code for the phi nodes of the block that `predecessor`
  * jumps to. The phis copy in parallel: one phi's source may be another

packages/bugc/src/evmgen/inline-bracket.test.ts

@@ -15,6 +15,7 @@ import { describe, it, expect } from "vitest";
 
 import { compile } from "#compiler";
 import { executeProgram } from "#test/evm/behavioral";
+import { traceLocals, localsOf } from "#test/evm/locals";
 import type * as Format from "@ethdebug/format";
 import { Program } from "@ethdebug/format";
 
@@ -206,3 +207,52 @@ describe("bracketing is a no-op for single-op invoke/return carriers", () => {
     expect(t.ret).toBeGreaterThan(0);
   });
 });
+
+// An inlined body with an `if`: its branch, the blocks it jumps to, and
+// the empty block bugc puts on a branch edge into a block with phis (b
+// is a phi after the `if`) are all part of the body
+const branchingBody = (t: number) => `name Cap;
+define {
+  function cap(x: uint256) -> uint256 {
+    let b = x * 2;
+    if (b > 100) { b = 100; }
+    return b;
+  };
+}
+storage { [0] r: uint256; [1] t: uint256; }
+create { t = ${t}; }
+code { r = cap(t); }`;
+
+describe("an inlined body's control flow is part of the body", () => {
+  for (const level of [2, 3] as const) {
+    for (const t of [70, 10]) {
+      it(`marks every step of the body inline (t = ${t}, O${level})`, async () => {
+        const { steps, instructionAt } = await traceLocals(
+          branchingBody(t),
+          level,
+        );
+        let open = 0;
+        let invoked = 0;
+        const outside: number[] = [];
+        const unlisted: number[] = [];
+        steps.forEach((step, i) => {
+          const instruction = instructionAt(step);
+          const f = flags({ debug: instruction });
+          if (f.invoke) {
+            open += 1;
+            invoked += 1;
+          }
+          if (open > 0 && !f.inline) outside.push(i);
+          if (open > 0 && localsOf(instruction?.context).length === 0) {
+            unlisted.push(i);
+          }
+          if (f.return) open -= 1;
+        });
+        expect(invoked).toBe(1);
+        expect(open).toBe(0);
+        expect(outside).toEqual([]);
+        expect(unlisted).toEqual([]);
+      });
+    }
+  }
+});

packages/bugc/src/evmgen/split-edges.ts

@@ -23,18 +23,30 @@ function splitFunction(func: Ir.Function): Ir.Function {
 
   // Route the edge from predId into targetId through a new empty
   // block if targetId has phis; return the block to branch to
-  const split = (predId: string, targetId: string): string => {
+  const split = (predId: string, pred: Ir.Block, targetId: string): string => {
     const target = blocks.get(targetId);
     if (!target || target.phis.length === 0) {
       return targetId;
     }
 
+    // The edge is the branch's: it has the branch's source range,
+    // variables and transforms (as `inline`, in an inlined body), but
+    // not its invoke or return
+    const { context, inlineSites, origin } = pred.terminator.operationDebug;
+    const edgeContext = Ir.Utils.withoutActivations(context);
     const edgeId = `${predId}_to_${targetId}`;
     blocks.set(edgeId, {
       id: edgeId,
       instructions: [],
-      // No debug context - compiler-generated edge block
-      terminator: { kind: "jump", target: targetId, operationDebug: {} },
+      terminator: {
+        kind: "jump",
+        target: targetId,
+        operationDebug: {
+          ...(edgeContext ? { context: edgeContext } : {}),
+          ...(inlineSites ? { inlineSites } : {}),
+          ...(origin ? { origin } : {}),
+        },
+      },
       predecessors: new Set([predId]),
       phis: [],
       debug: {},
@@ -60,9 +72,9 @@ function splitFunction(func: Ir.Function): Ir.Function {
     }
 
     const { trueTarget, falseTarget } = pred.terminator;
-    const newTrue = split(predId, trueTarget);
+    const newTrue = split(predId, pred, trueTarget);
     const newFalse =
-      falseTarget === trueTarget ? newTrue : split(predId, falseTarget);
+      falseTarget === trueTarget ? newTrue : split(predId, pred, falseTarget);
     blocks.set(predId, {
       ...blocks.get(predId)!,
       terminator: {

packages/bugc/src/optimizer/steps/inlining.ts

@@ -270,9 +270,11 @@ export class InliningStep extends BaseOptimizationStep {
           operationDebug: { ...returnedJump },
         };
       } else {
+        // The body's own control flow (an `if`'s branch, the jumps
+        // between its blocks) is part of the body, too
         terminator = remapTerminator(t, remapValue, blockRename);
         terminator.operationDebug = {
-          ...terminator.operationDebug,
+          ...Ir.Utils.addTransform(terminator.operationDebug, "inline"),
           inlineSites: inlineSites(t.operationDebug),
           ...(t.operationDebug?.origin
             ? { origin: t.operationDebug.origin }
@@ -354,8 +356,11 @@ export class InliningStep extends BaseOptimizationStep {
           returnId = from[0];
           continue;
         }
+        // The jump that carries the return is the body's exit, as a
+        // real call's exit JUMP is the callee's: the body runs up to
+        // it and through it
         block.terminator.operationDebug = withActivationsDebug(
-          block.terminator.operationDebug,
+          Ir.Utils.addTransform(block.terminator.operationDebug, "inline"),
           { returns },
         );
         break;