diff --git a/packages/bugc/CHANGELOG.md b/packages/bugc/CHANGELOG.md index fb6e5e4ea..1b5cb6a56 100644 --- a/packages/bugc/CHANGELOG.md +++ b/packages/bugc/CHANGELOG.md @@ -65,6 +65,14 @@ support. Changes to the specification itself are tracked in the root the function has another `return;`. Before, return merging removed the call's continuation block, and code generation failed ("Jump target ... not found") ([#388]). +- `==` and `!=` on `bytes` or `string` in memory compare the contents, + by the `keccak256` of each operand's data. Before, they compared the + two memory addresses, so equal contents in two places were unequal + ([#391]). +- A hex literal longer than 32 bytes hashes and compares by its bytes + at every optimization level. Before, constant folding hashed its + text. A string literal that starts with "0x" is its text. Before, + code generation decoded it as hex ([#391]). - `push` of bytes in calldata onto an `array` in storage now stores the bytes. Before, it stored the calldata reference's word (offset and length) ([#379]). @@ -563,3 +571,4 @@ First publication. [#383]: https://github.com/ethdebug/format/pull/383 [#384]: https://github.com/ethdebug/format/pull/384 [#388]: https://github.com/ethdebug/format/pull/388 +[#391]: https://github.com/ethdebug/format/pull/391 diff --git a/packages/bugc/src/evmgen/__snapshots__/activation-pairing.test.ts.snap b/packages/bugc/src/evmgen/__snapshots__/activation-pairing.test.ts.snap index 4a23c8dd1..8195b3707 100644 --- a/packages/bugc/src/evmgen/__snapshots__/activation-pairing.test.ts.snap +++ b/packages/bugc/src/evmgen/__snapshots__/activation-pairing.test.ts.snap @@ -46,10 +46,10 @@ exports[`the code does not depend on the markers > is the same for every example "advanced/nested-structs.bug at O1": "ae8772307e46e33f94b8e36017ef8a4c23743fbb26085693a495239d25da3008", "advanced/nested-structs.bug at O2": "d141d683602322f4195283df977a6c5618e72e995bfca7d8b285ae3a0f779bb4", "advanced/nested-structs.bug at O3": "d141d683602322f4195283df977a6c5618e72e995bfca7d8b285ae3a0f779bb4", - "advanced/token-registry.bug at O0": "430a947f02160f905a04efdeea0b5d69fb7aa6be6b2dc54734be26ec0511988a", - "advanced/token-registry.bug at O1": "2de1cfe1d642a35a409d46df00f9ebb08ce0c43736020a5aa06cae30983b72e6", - "advanced/token-registry.bug at O2": "2c0dcbce9d26b8182ce6607aeb480005fc485e7e21c44405be48c451c945c2e3", - "advanced/token-registry.bug at O3": "989c5871f2e3c40b13af199fcc441a7aee8427a7935243af8f152d6758124642", + "advanced/token-registry.bug at O0": "5e8c272398293d3005131bbb42f9ff447f7eeb34c37f8a0ace51189986d53815", + "advanced/token-registry.bug at O1": "e64bc9fa74ad41315751723cbe5f5efa67496523d60c4d41a23eb7274a1efd7d", + "advanced/token-registry.bug at O2": "f70c11b4702b7d5eab762a68809048055f512bbaad6fb8e584d62a678b325b37", + "advanced/token-registry.bug at O3": "72c79740aed4c6a0a2e79347b2652c96a6cbbcb260f0fd6c710809fcb08c144b", "advanced/voting-system.bug at O0": "e49f6349c9c8fc5d3e14cd737a860891139518b0e0ea73d3c01f314a60e80a30", "advanced/voting-system.bug at O1": "32f9c89a0890e1591b4e1aa29ec2b52d83a13c6ed06c4fc3391245002c505e38", "advanced/voting-system.bug at O2": "3479679f427cb43fcc7288d34666f865d94c20a05f216f719f41a218624823ec", diff --git a/packages/bugc/src/evmgen/bytes-equality.test.ts b/packages/bugc/src/evmgen/bytes-equality.test.ts new file mode 100644 index 000000000..71f4d033b --- /dev/null +++ b/packages/bugc/src/evmgen/bytes-equality.test.ts @@ -0,0 +1,131 @@ +import { describe, it, expect } from "vitest"; + +import { keccak256 } from "ethereum-cryptography/keccak"; + +import { executeProgram } from "#test/evm/behavioral"; + +const levels = [0, 1, 2, 3] as const; + +const word = (n: number) => n.toString(16).padStart(64, "0"); +const hex = (s: string) => Buffer.from(s).toString("hex"); + +/** Calldata: the two lengths, then the two byte strings */ +const calldata = (a: string, b: string) => + word(a.length) + word(b.length) + hex(a) + hex(b); + +/** + * Two byte strings from the calldata, each copied to its own place in + * memory, compared as `bytes` and as `string`, with a literal, and + * with a string in storage + */ +const source = `name Equality; +storage { + [0] eq: bool; [1] ne: bool; [2] seq: bool; [3] sne: bool; + [4] lit: bool; [5] stored: string; [6] seqs: bool; +} +code { + let la = msg.data[0:32] as bytes32 as uint256; + let lb = msg.data[32:64] as bytes32 as uint256; + let a: bytes = msg.data[64:64 + la]; + let b: bytes = msg.data[64 + la:64 + la + lb]; + eq = a == b; + ne = a != b; + let x = a as string; + let y = b as string; + seq = x == y; + sne = x != y; + lit = x == "abc"; + stored = x; + seqs = stored == y; +}`; + +const cases: [string, string, string][] = [ + ["equal", "abc", "abc"], + ["unequal, same length", "abc", "abd"], + ["different lengths", "abc", "abcd"], + ["empty", "", ""], + ["empty and not", "", "a"], + ["equal, 32 bytes", "a".repeat(32), "a".repeat(32)], + ["equal, 33 bytes", "a".repeat(33), "a".repeat(33)], + ["unequal past 32 bytes", "a".repeat(33), "a".repeat(32) + "b"], + ["32 and 33 bytes", "a".repeat(32), "a".repeat(33)], + ["unequal in the first byte, 64", "b" + "a".repeat(63), "a".repeat(64)], +]; + +describe("== and != on bytes and string compare contents", () => { + for (const level of levels) { + describe(`at O${level}`, () => { + for (const [name, a, b] of cases) { + it(name, async () => { + const result = await executeProgram(source, { + calldata: calldata(a, b), + optimizationLevel: level, + }); + expect(result.callSuccess).toBe(true); + const equal = BigInt(a === b); + expect(await result.getStorage(0n)).toBe(equal); + expect(await result.getStorage(1n)).toBe(1n - equal); + expect(await result.getStorage(2n)).toBe(equal); + expect(await result.getStorage(3n)).toBe(1n - equal); + expect(await result.getStorage(4n)).toBe(BigInt(a === "abc")); + expect(await result.getStorage(6n)).toBe(equal); + }); + } + }); + } +}); + +/** A hex literal longer than 32 bytes, as dynamic `bytes` */ +const long = "11".repeat(32) + "22"; + +/** A string literal that starts as a hex literal does */ +const hexLike = "0x12"; + +/** + * Literals compared with copies from the calldata, and hashed: the + * same at every level, whether or not the optimizer folds them + */ +const literals = `name Literals; +storage { + [0] longEq: bool; [1] longHash: bytes32; + [2] textEq: bool; [3] textHash: bytes32; [4] castHash: bytes32; +} +code { + let la = msg.data[0:32] as bytes32 as uint256; + let lb = msg.data[32:64] as bytes32 as uint256; + let a: bytes = msg.data[64:64 + la]; + let b: bytes = msg.data[64 + la:64 + la + lb]; + longEq = a == 0x${long}; + longHash = keccak256(0x${long}); + textEq = (b as string) == "${hexLike}"; + textHash = keccak256("${hexLike}"); + castHash = keccak256("${hexLike}" as bytes); +}`; + +describe("bytes and string literals", () => { + const keccak = (data: Uint8Array) => + BigInt("0x" + Buffer.from(keccak256(data)).toString("hex")); + + for (const level of levels) { + it(`compare and hash by their bytes at O${level}`, async () => { + const a = Buffer.from(long, "hex"); + const b = Buffer.from(hexLike); + const result = await executeProgram(literals, { + calldata: + word(a.length) + + word(b.length) + + a.toString("hex") + + b.toString("hex"), + optimizationLevel: level, + }); + expect(result.callSuccess).toBe(true); + expect([ + await result.getStorage(0n), + await result.getStorage(1n), + await result.getStorage(2n), + await result.getStorage(3n), + await result.getStorage(4n), + ]).toEqual([1n, keccak(a), 1n, keccak(b), keccak(b)]); + }); + } +}); diff --git a/packages/bugc/src/evmgen/generation/instructions/const.ts b/packages/bugc/src/evmgen/generation/instructions/const.ts index 95821deb7..1828f5027 100644 --- a/packages/bugc/src/evmgen/generation/instructions/const.ts +++ b/packages/bugc/src/evmgen/generation/instructions/const.ts @@ -1,4 +1,4 @@ -import type * as Ir from "#ir"; +import * as Ir from "#ir"; import type { Stack } from "#evm"; import { @@ -42,27 +42,8 @@ export function generateConst( // References need memory allocation for the data they point to if (inst.type.kind === "ref" && inst.type.location === "memory") { - let bytes: Uint8Array; - let byteLength: bigint; - - // For memory references, we need to check the origin type to understand what data to store - // For now, we'll handle hex strings and regular strings - if (typeof inst.value === "string" && inst.value.startsWith("0x")) { - // Dynamic bytes from hex string - decode the hex - const hexStr = inst.value.slice(2); // Remove 0x prefix - const hexBytes = []; - for (let i = 0; i < hexStr.length; i += 2) { - hexBytes.push(parseInt(hexStr.substr(i, 2), 16)); - } - bytes = new Uint8Array(hexBytes); - byteLength = BigInt(bytes.length); - } else { - // String or non-hex bytes - use UTF-8 encoding - const strValue = String(inst.value); - const encoder = new TextEncoder(); - bytes = encoder.encode(strValue); - byteLength = BigInt(bytes.length); - } + const bytes = Ir.Utils.constantBytes(String(inst.value), inst.type); + const byteLength = BigInt(bytes.length); // Calculate memory needed: 32 bytes for length + actual data (padded to 32-byte words) const dataWords = (byteLength + 31n) / 32n; diff --git a/packages/bugc/src/ir/utils/constant.ts b/packages/bugc/src/ir/utils/constant.ts new file mode 100644 index 000000000..2df501f38 --- /dev/null +++ b/packages/bugc/src/ir/utils/constant.ts @@ -0,0 +1,27 @@ +import { Type as BugType } from "#types"; + +import { Type } from "../spec/type.js"; + +/** + * The bytes of a string constant of dynamic `bytes` or a `string`: a + * `string` is its text, in UTF-8, and `bytes` (from a hex literal too + * long for a word) are the bytes the "0x…" hex names. Code generation + * stores these bytes, and constant folding hashes them. + */ +export function constantBytes(value: string, type: Type): Uint8Array { + const isString = + Type.isRef(type) && + type.origin !== "synthetic" && + BugType.isElementary(type.origin) && + BugType.Elementary.isString(type.origin); + if (isString || !value.startsWith("0x")) { + return new TextEncoder().encode(value); + } + + const hex = value.slice(2); + const bytes = new Uint8Array(Math.ceil(hex.length / 2)); + for (let i = 0; i < bytes.length; i++) { + bytes[i] = parseInt(hex.slice(2 * i, 2 * i + 2), 16); + } + return bytes; +} diff --git a/packages/bugc/src/ir/utils/index.ts b/packages/bugc/src/ir/utils/index.ts index 9a9db3395..13c031aeb 100644 --- a/packages/bugc/src/ir/utils/index.ts +++ b/packages/bugc/src/ir/utils/index.ts @@ -3,3 +3,4 @@ export * from "./cast.js"; export * from "./def-sites.js"; export * from "./debug-info.js"; export * from "./signed.js"; +export * from "./constant.js"; diff --git a/packages/bugc/src/irgen/generate/expressions/operator.ts b/packages/bugc/src/irgen/generate/expressions/operator.ts index 10545359b..886397ec2 100644 --- a/packages/bugc/src/irgen/generate/expressions/operator.ts +++ b/packages/bugc/src/irgen/generate/expressions/operator.ts @@ -4,6 +4,7 @@ import { Severity } from "#result"; import { Error as IrgenError, assertExhausted } from "#irgen/errors"; import { fromBugType } from "#irgen/type"; +import { Type } from "#types"; import { Process } from "../process.js"; import type { Context } from "./context.js"; @@ -137,19 +138,31 @@ const makeBuildBinaryOperator = ( const resultType = fromBugType(nodeType); // Evaluate operands - const leftVal = yield* buildExpression(expr.operands[0], { + let leftVal = yield* buildExpression(expr.operands[0], { kind: "rvalue", }); - const rightVal = yield* buildExpression(expr.operands[1], { + let rightVal = yield* buildExpression(expr.operands[1], { kind: "rvalue", }); - // Generate temp for result - const tempId = yield* Process.Variables.newTemp(); - const op = mapBinaryOp(expr.operator); const operationDebug = yield* Process.Debug.forAstNode(expr); + // `==` and `!=` on dynamic bytes or strings compare their + // contents, by comparing the hashes of the data. The operands need + // no copy to memory (`emitInMemory`): the type checker rejects + // bytes and strings in calldata here, so both are in memory. + if ( + (op === "eq" || op === "ne") && + isDynamicBytes(yield* Process.Types.nodeType(expr.operands[0])) + ) { + leftVal = yield* emitHash(leftVal, operationDebug); + rightVal = yield* emitHash(rightVal, operationDebug); + } + + // Generate temp for result + const tempId = yield* Process.Variables.newTemp(); + // Emit binary operation yield* Process.Instructions.emit({ kind: "binary", @@ -165,6 +178,29 @@ const makeBuildBinaryOperator = ( : Ir.Value.temp(tempId, resultType); }; +/** Whether a type is dynamic `bytes` or a `string` */ +function isDynamicBytes(type: Type | undefined): boolean { + if (!type || !Type.isElementary(type)) return false; + return Type.Elementary.isBytes(type) + ? Type.Elementary.Bytes.isDynamic(type) + : Type.Elementary.isString(type); +} + +/** keccak256 of the data of dynamic bytes or a string in memory */ +function* emitHash( + value: Ir.Value, + operationDebug: Ir.Instruction.Debug, +): Process { + const dest = yield* Process.Variables.newTemp(); + yield* Process.Instructions.emit({ + kind: "hash", + values: [value], + dest, + operationDebug, + } as Ir.Instruction.Hash); + return Ir.Value.temp(dest, Ir.Type.Scalar.bytes32); +} + /** * Whether an operation's result can fall outside its type. A quotient * or remainder of unsigned values is no greater than the dividend, and diff --git a/packages/bugc/src/optimizer/steps/constant-folding.ts b/packages/bugc/src/optimizer/steps/constant-folding.ts index 28e617cc9..5d0d09180 100644 --- a/packages/bugc/src/optimizer/steps/constant-folding.ts +++ b/packages/bugc/src/optimizer/steps/constant-folding.ts @@ -321,7 +321,7 @@ export class ConstantFoldingStep extends BaseOptimizationStep { ); if (values.length === 1 && typeof values[0] === "string") { - return new TextEncoder().encode(values[0]); + return Ir.Utils.constantBytes(values[0], inst.values[0].type); } const words: Uint8Array[] = []; diff --git a/packages/bugc/src/typechecker/checker.test.ts b/packages/bugc/src/typechecker/checker.test.ts index 55dcd5707..92ad77a6b 100644 --- a/packages/bugc/src/typechecker/checker.test.ts +++ b/packages/bugc/src/typechecker/checker.test.ts @@ -148,6 +148,52 @@ describe("checkProgram", () => { expect(Result.hasMessages(result)).toBe(false); }); + it("should type check == and != on bytes and strings", () => { + const result = check(` + name Test; + storage { [0] s: string; } + code { + let a: bytes = msg.data[0:4]; + let b: bytes = msg.data[4:8]; + let x = a as string; + let b1 = a == b; + let b2 = a != b; + let b3 = x == "abc"; + let b4 = s != x; + let b5 = msg.data[0:4] as bytes4 == 0x12345678 as bytes4; + let c: bytes calldata = msg.data[0:4]; + let b6 = (c as bytes) == a; + } + `); + + expect(result.success).toBe(true); + expect(Result.hasMessages(result)).toBe(false); + }); + + it("should reject == between bytes, strings and bytesN", () => { + const result = check(` + name Test; + storage {} + code { + let a: bytes = msg.data[0:4]; + let x = a as string; + let b1 = a == x; + let b2 = a == (msg.data[0:4] as bytes4); + } + `); + + expect(result.success).toBe(false); + expect(Result.countErrors(result)).toBe(2); + expect(result).toHaveMessage({ + severity: Severity.Error, + message: "Cannot compare bytes with string", + }); + expect(result).toHaveMessage({ + severity: Severity.Error, + message: "Cannot compare bytes with bytes4", + }); + }); + it("should type check logical operators", () => { const result = check(` name Test; 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 7cdd86d6a..c1bb09696 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,24 @@ so `x + 1` with `x: uint8` is a `uint8` and wraps at 8 bits. -(-128 as int8); // -128 ``` +### Equality + +`==` and `!=` on dynamic `bytes` or on `string` compare the contents, +not the place in memory: the compiler compares the `keccak256` of the +data of each operand. As for a mapping key, this assumes that two +different contents do not have the same hash. Both operands must have +the same type, in memory: bytes or a string in calldata do not +compare, so cast or copy them to memory first. A `bytesN` is a value, +and compares as a word. + +```bug +let a: bytes = msg.data[0:4]; +let b: bytes = msg.data[0:4]; // a second copy +a == b; // true +(a as string) == "abcd"; // true if a is "abcd" +msg.data[0:4] as bytes4 == 0xa9059cbb as bytes4; +``` + ### Bounds An index out of bounds reverts, as in Solidity. `a[i]` on an array (in @@ -174,7 +192,8 @@ motd = name; // stored as Solidity stores a string As a `string` in memory, a `string calldata` has no index or slice, and bytes do not assign to it without a cast. It too can only be the type of a `let`, a parameter or a cast. Bytes or a string in calldata -cannot be compared with `==` or `!=`, or be a mapping key. +cannot be compared with `==` or `!=`, or be a mapping key: cast or +copy them to memory first, as in `(text as bytes) == other`. A parameter of bytes or a string in calldata takes its argument's word, with no copy, both in a real call and when the optimizer inlines the