bugc: fix return-epilogue stack cleanup for multiple stale values

packages/bugc/src/evmgen/generation/control-flow/terminator.ts

@@ -338,28 +338,25 @@ function generateReturnEpilogue<S extends Stack>(
 
     // Pop stale values, keeping only the return value
     // (if any). Multi-block functions can accumulate
-    // leftover values (e.g. branch condition results)
-    // from predecessor blocks.
+    // leftover values (e.g. branch condition results, or
+    // operands DUP'd by loadValue) below the return value.
+    //
+    // With the return value on TOS, repeatedly SWAP1 the
+    // stale value just beneath it up to the top and POP it.
+    // Each SWAP1/POP removes one stale value while leaving
+    // the return value on TOS — so any number of stale
+    // values are cleaned without disturbing the result.
     const keep = value ? 1 : 0;
     while (s.stack.length > keep) {
-      if (keep > 0 && s.stack.length > 1) {
-        const depth = s.stack.length - 1;
+      if (keep > 0) {
         s = {
           ...s,
           instructions: [
             ...s.instructions,
-            {
-              mnemonic: `SWAP${depth}`,
-              opcode: 0x8f + depth,
-              debug,
-            },
+            { mnemonic: "SWAP1", opcode: 0x90, debug },
           ],
-          stack: [s.stack[depth], ...s.stack.slice(1, depth), s.stack[0]],
-          brands: [
-            s.brands[depth],
-            ...s.brands.slice(1, depth),
-            s.brands[0],
-          ] as Stack,
+          stack: [s.stack[1], s.stack[0], ...s.stack.slice(2)],
+          brands: [s.brands[1], s.brands[0], ...s.brands.slice(2)] as Stack,
         };
       }
       s = {

packages/bugc/src/evmgen/return-epilogue.test.ts

@@ -0,0 +1,94 @@
+/**
+ * Regression: returning from a branch must yield the return
+ * expression, not a stale stack value.
+ *
+ * A user function's return epilogue cleans up leftover stack values
+ * (e.g. a branch condition, or an operand DUP'd by loadValue) before
+ * jumping back to the caller, keeping only the return value. When two
+ * or more stale values sit below the return value, the cleanup must
+ * still preserve the return value. A prior rotation bug returned the
+ * deepest stale value (the branch condition) instead.
+ */
+import { describe, it, expect } from "vitest";
+
+import { executeProgram } from "#test/evm/behavioral";
+
+type OptLevel = 0 | 1 | 2 | 3;
+const LEVELS: OptLevel[] = [0, 1, 2, 3];
+
+async function storageResult(source: string, level: OptLevel): Promise<bigint> {
+  const res = await executeProgram(source, {
+    calldata: "",
+    optimizationLevel: level,
+  });
+  expect(res.callSuccess).toBe(true);
+  return res.getStorage(0n);
+}
+
+describe("return epilogue preserves the return value across branches", () => {
+  // The true branch (JUMPI-taken) return has two stale values below
+  // the return value at runtime: the `eq` const operand and the
+  // branch condition, both left by loadValue's DUP.
+  const pick = `name Pick;
+define {
+  function pick(x: uint256) -> uint256 {
+    if (x == 0) { return 9; } else { return 7; }
+  };
+}
+storage { [0] r: uint256; }
+create { r = 0; }
+code { r = pick(0); }`;
+
+  for (const level of LEVELS) {
+    it(`returns from the true branch (level ${level})`, async () => {
+      expect(await storageResult(pick, level)).toBe(9n);
+    });
+  }
+
+  // Returning a parameter (not a constant) from the true branch.
+  const base = `name Base;
+define {
+  function base(n: uint256, acc: uint256) -> uint256 {
+    if (n == 0) { return acc; } else { return acc; }
+  };
+}
+storage { [0] r: uint256; }
+create { r = 0; }
+code { r = base(0, 42); }`;
+
+  for (const level of LEVELS) {
+    it(`returns a parameter from the true branch (level ${level})`, async () => {
+      expect(await storageResult(base, level)).toBe(42n);
+    });
+  }
+
+  // A call in return position, from a branch that itself returns a
+  // call result. Exercises the epilogue on a continuation-return with
+  // leftover stack values, without recursion. `g(0)` takes its true
+  // branch and returns `f(0)`, which takes its true branch and
+  // returns 11.
+  const chain = `name Chain;
+define {
+  function f(x: uint256) -> uint256 {
+    if (x == 0) { return 11; } else { return 22; }
+  };
+  function g(x: uint256) -> uint256 {
+    if (x == 0) { return f(0); } else { return f(1); }
+  };
+}
+storage { [0] r: uint256; }
+create { r = 0; }
+code { r = g(0); }`;
+
+  for (const level of LEVELS) {
+    it(`returns a call result from a branch (level ${level})`, async () => {
+      expect(await storageResult(chain, level)).toBe(11n);
+    });
+  }
+
+  // NOTE: self-recursive functions (e.g. `sum(5,0)` returning 15) are
+  // still miscompiled by a separate, deeper defect — untracked leaked
+  // stack values desync the linear stack model at the recursive call
+  // site, corrupting the callee's arguments. That is tracked apart
+  // from this epilogue fix; see the accompanying issue.
+});