bugc: hash the data of dynamic bytes and strings

packages/bugc/CHANGELOG.md

@@ -79,6 +79,12 @@ support. Changes to the specification itself are tracked in the root
 
 ### Fixed
 
+- `keccak256` of dynamic `bytes` or a `string` now hashes its data, and
+  `keccak256(msg.data)` hashes the calldata. Before, code generation
+  hashed the word that refers to the data (its memory address, or 0 for
+  `msg.data`). So at optimization level 0, `keccak256("transfer()")` was
+  wrong, while levels 1 to 3 folded it to the right constant; at every
+  level, the hash of a slice or of `msg.data` was wrong ([#360]).
 - 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 +296,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
+[#360]: https://github.com/ethdebug/format/pull/360

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

@@ -1289,6 +1289,69 @@ code {
     }
   });
 
+  describe("hashes of dynamic bytes", () => {
+    const hashOf = (data: string | Uint8Array) =>
+      BigInt(
+        "0x" +
+          bytesToHex(
+            keccak256(
+              typeof data === "string" ? new TextEncoder().encode(data) : data,
+            ),
+          ),
+      );
+
+    const long = "season 2 starts friday, see you on the leaderboard";
+    const calldata = "0xaabbccdd" + "11".repeat(32);
+
+    const hashes: Record<string, [string, bigint]> = {
+      "a string literal": [
+        `out = keccak256("setMotd(string)") as uint256;`,
+        hashOf("setMotd(string)"),
+      ],
+      "a selector": [
+        `out = keccak256("setMotd(string)") as bytes4 as uint256;`,
+        0x5fe59b9dn,
+      ],
+      "an empty string": [`out = keccak256("") as uint256;`, hashOf("")],
+      "a string longer than a word": [
+        `out = keccak256("${long}") as uint256;`,
+        hashOf(long),
+      ],
+      "a string local": [
+        `let s = "setMotd(string)";
+  out = keccak256(s) as uint256;`,
+        hashOf("setMotd(string)"),
+      ],
+      "a calldata slice": [
+        `out = keccak256(msg.data[0:4]) as uint256;`,
+        hashOf(new Uint8Array([0xaa, 0xbb, 0xcc, 0xdd])),
+      ],
+      "msg.data": [
+        `out = keccak256(msg.data) as uint256;`,
+        hashOf(new Uint8Array(Buffer.from(calldata.slice(2), "hex"))),
+      ],
+    };
+
+    for (const [name, [body, expected]] of Object.entries(hashes)) {
+      for (const level of [0, 1, 2, 3] as const) {
+        it(`should hash ${name} (level ${level})`, async () => {
+          const source = `name Hash;
+storage { [0] n: uint256; [1] out: uint256; }
+code {
+  ${body}
+}`;
+          const result = await executeProgram(source, {
+            calldata,
+            optimizationLevel: level,
+          });
+
+          expect(result.callSuccess).toBe(true);
+          expect(await result.getStorage(1n)).toBe(expected);
+        });
+      }
+    }
+  });
+
   describe("modulo", () => {
     const program = (expr: string) => `name Modulo;
 

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

@@ -44,7 +44,7 @@ export function generate<S extends Stack>(
     case "env":
       return generateEnvOp(inst);
     case "hash":
-      return generateHashOp(inst);
+      return generateHashOp(inst, msgDataIds);
     case "length":
       return generateLength(inst, msgDataIds);
     case "compute_slot":

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

@@ -1,20 +1,59 @@
-import type * as Ir from "#ir";
+import * as Ir from "#ir";
 import type { Stack } from "#evm";
 
 import { type Transition, pipe, operations } from "#evmgen/operations";
+import { Memory } from "#evmgen/analysis";
 
-import { loadValue, storeValueIfNeeded } from "../values/index.js";
+import { loadValue, storeValueIfNeeded, valueId } from "../values/index.js";
 
-const { PUSHn, MSTORE, KECCAK256 } = operations;
+const { PUSHn, MSTORE, MLOAD, ADD, KECCAK256, CALLDATASIZE, CALLDATACOPY } =
+  operations;
+
+const freeMemoryPointer = BigInt(Memory.regions.FREE_MEMORY_POINTER);
 
 /**
- * Generate code for hash operations
+ * Generate code for hash operations.
+ *
+ * A word value (such as a `bytes32`) hashes its 32 bytes. Dynamic
+ * `bytes` or a `string` in memory is the address of a length word,
+ * followed by the data: hash the data. `msg.data` hashes the calldata.
  */
 export function generateHashOp<S extends Stack>(
   inst: Ir.Instruction.Hash,
+  msgDataIds: ReadonlySet<string> = new Set(),
 ): Transition<S, readonly ["value", ...S]> {
   const debug = inst.operationDebug;
 
+  // msg.data is a temp defined by an env `msg_data` instruction, so
+  // the caller passes the ids of those temps. Copy the calldata to the
+  // free memory, without allocating it, and hash it there.
+  if (msgDataIds.has(valueId(inst.value))) {
+    return pipe<S>()
+      .then(CALLDATASIZE({ debug }), { as: "size" })
+      .then(PUSHn(0n, { debug }), { as: "offset" })
+      .then(PUSHn(freeMemoryPointer, { debug }), { as: "offset" })
+      .then(MLOAD({ debug }), { as: "destOffset" })
+      .then(CALLDATACOPY({ debug }))
+      .then(CALLDATASIZE({ debug }), { as: "size" })
+      .then(PUSHn(freeMemoryPointer, { debug }), { as: "offset" })
+      .then(MLOAD({ debug }), { as: "offset" })
+      .then(KECCAK256({ debug }), { as: "value" })
+      .then(storeValueIfNeeded(inst.dest, { debug }))
+      .done();
+  }
+
+  if (Ir.Type.isRef(inst.value.type) && inst.value.type.location === "memory") {
+    return pipe<S>()
+      .then(loadValue(inst.value, { debug }), { as: "offset" })
+      .then(MLOAD({ debug }), { as: "size" })
+      .then(PUSHn(32n, { debug }), { as: "b" })
+      .then(loadValue(inst.value, { debug }), { as: "a" })
+      .then(ADD({ debug }), { as: "offset" })
+      .then(KECCAK256({ debug }), { as: "value" })
+      .then(storeValueIfNeeded(inst.dest, { debug }))
+      .done();
+  }
+
   return pipe<S>()
     .then(loadValue(inst.value, { debug }))
     .then(PUSHn(0n, { debug }), { as: "offset" })