From 28ff123e7c49565074e163b280d502e12339f845 Mon Sep 17 00:00:00 2001 From: "g. nicholas d'andrea" Date: Tue, 6 Oct 2026 01:42:00 -0400 Subject: [PATCH 1/5] bugc: sign-extend narrow signed values read from storage A storage read of a signed integer narrower than a word extracted the value's bytes and stopped, so a negative packed field such as an int8 holding -56 read as 200. Sign-extend from the type's width after the read, with the steps a cast to a word would take. --- packages/bugc/CHANGELOG.md | 6 ++ packages/bugc/src/evmgen/behavioral.test.ts | 55 +++++++++++++++++++ .../evmgen/generation/instructions/cast.ts | 22 +++++--- .../evmgen/generation/instructions/storage.ts | 22 +++++++- 4 files changed, 97 insertions(+), 8 deletions(-) diff --git a/packages/bugc/CHANGELOG.md b/packages/bugc/CHANGELOG.md index 50805812b8..919d000f49 100644 --- a/packages/bugc/CHANGELOG.md +++ b/packages/bugc/CHANGELOG.md @@ -87,6 +87,11 @@ support. Changes to the specification itself are tracked in the root significant byte. Before, it gave the offset from the low-order end, so a debugger read the wrong bytes: for example, an `address` at the start of a slot read as the slot's high-order 20 bytes ([#336]). +- Reading a signed integer narrower than a word from storage now + sign-extends it, so a struct field, variable or mapping value of type + `int8` that holds `-56` reads as `-56`. Before, at every optimization + level, a packed field read back as its unsigned bytes, such as `200` + ([#337]). ## 0.1.0-preview.1 — 2026-10-03 @@ -134,3 +139,4 @@ First publication. [#334]: https://github.com/ethdebug/format/pull/334 [#335]: https://github.com/ethdebug/format/pull/335 [#336]: https://github.com/ethdebug/format/pull/336 +[#337]: https://github.com/ethdebug/format/pull/337 diff --git a/packages/bugc/src/evmgen/behavioral.test.ts b/packages/bugc/src/evmgen/behavioral.test.ts index d304190af3..c5deaee2da 100644 --- a/packages/bugc/src/evmgen/behavioral.test.ts +++ b/packages/bugc/src/evmgen/behavioral.test.ts @@ -1322,4 +1322,59 @@ code { size = msg.data.length; }`; } } }); + + describe("signed storage reads", () => { + const neg = (n: bigint) => 2n ** 256n - n; + + const source = `name SignedReads; +define { + struct R { a: int8; b: int16; c: int64; d: int128; }; +} +storage { + [0] s: R; + [1] x: int8; + [2] y: int16; + [3] m: mapping; + [10] o0: int256; + [11] o1: int256; + [12] o2: int256; + [13] o3: int256; + [14] o4: int256; + [15] o5: int256; + [16] o6: int256; +} +create { + s.a = -56 as int8; + s.b = -2 as int16; + s.c = -3 as int64; + s.d = -4 as int128; + x = -56 as int8; + y = -300 as int16; + m[5] = -7 as int16; +} +code { + o0 = s.a; + o1 = s.b; + o2 = s.c; + o3 = s.d; + o4 = x; + o5 = y; + o6 = m[5]; +}`; + + const expected = [56n, 2n, 3n, 4n, 56n, 300n, 7n].map(neg); + + for (const level of [0, 1, 2, 3] as const) { + it(`should sign-extend narrow signed values (level ${level})`, async () => { + const result = await executeProgram(source, { + calldata: "", + optimizationLevel: level, + }); + expect(result.callSuccess).toBe(true); + for (const [i, value] of expected.entries()) { + expect(await result.getStorage(10n + BigInt(i))).toBe(value); + } + }); + } + }); }); diff --git a/packages/bugc/src/evmgen/generation/instructions/cast.ts b/packages/bugc/src/evmgen/generation/instructions/cast.ts index 65a529592d..6f4d035ef7 100644 --- a/packages/bugc/src/evmgen/generation/instructions/cast.ts +++ b/packages/bugc/src/evmgen/generation/instructions/cast.ts @@ -19,18 +19,26 @@ export function generateCast( const debug = inst.operationDebug; const steps = Ir.Utils.castSteps(inst.value.type, inst.targetType); - const cast = steps + return pipe() + .then(loadValue(inst.value, { debug }), { as: "value" }) + .then(generateCastSteps(steps, debug)) + .then(storeValueIfNeeded(inst.dest, { debug })) + .done(); +} + +/** + * Apply cast steps to the value on top of the stack + */ +export function generateCastSteps( + steps: readonly Ir.Utils.CastStep[], + debug: Ir.Instruction.Debug | undefined, +): Transition { + return steps .map((step) => generateStep(step, debug)) .reduce>( (previous, next) => (state) => next(previous(state)), (state) => state, ); - - return pipe() - .then(loadValue(inst.value, { debug }), { as: "value" }) - .then(cast) - .then(storeValueIfNeeded(inst.dest, { debug })) - .done(); } function generateStep( diff --git a/packages/bugc/src/evmgen/generation/instructions/storage.ts b/packages/bugc/src/evmgen/generation/instructions/storage.ts index 11a85b6c39..9eebee0f47 100644 --- a/packages/bugc/src/evmgen/generation/instructions/storage.ts +++ b/packages/bugc/src/evmgen/generation/instructions/storage.ts @@ -1,9 +1,10 @@ -import type * as Ir from "#ir"; +import * as Ir from "#ir"; import type { Stack } from "#evm"; import { type Transition, rebrand, pipe, operations } from "#evmgen/operations"; import { loadValue, storeValueIfNeeded } from "../values/index.js"; +import { generateCastSteps } from "./cast.js"; const { SWAP1, @@ -78,6 +79,21 @@ export function generateRead( return pipe().then(PUSHn(0n, { debug }), { as: "value" }).done(); } +/** + * Sign-extend a value read from (transient) storage to a full word, + * when its type is a signed integer narrower than a word. Storage + * keeps such a value in its own bytes only. + */ +function signExtend( + inst: Ir.Instruction.Read, + debug: Ir.Instruction.Debug, +): Transition { + return generateCastSteps( + Ir.Utils.castSteps(inst.type, Ir.Type.Scalar.uint256), + debug, + ); +} + /** * Storage read: SLOAD with optional partial-slot extraction */ @@ -93,6 +109,7 @@ function generateStorageRead( return pipe() .then(loadValue(inst.slot!, { debug }), { as: "key" }) .then(SLOAD({ debug }), { as: "value" }) + .then(signExtend(inst, debug)) .then(storeValueIfNeeded(inst.dest, { debug })) .done(); } @@ -123,6 +140,7 @@ function generateStorageRead( // shiftedValue & mask .then(AND({ debug }), { as: "value" }) + .then(signExtend(inst, debug)) .then(storeValueIfNeeded(inst.dest, { debug })) .done() ); @@ -143,6 +161,7 @@ function generateTransientRead( return pipe() .then(loadValue(inst.slot!, { debug }), { as: "key" }) .then(TLOAD({ debug }), { as: "value" }) + .then(signExtend(inst, debug)) .then(storeValueIfNeeded(inst.dest, { debug })) .done(); } @@ -174,6 +193,7 @@ function generateTransientRead( ) .then(AND({ debug }), { as: "value" }) + .then(signExtend(inst, debug)) .then(storeValueIfNeeded(inst.dest, { debug })) .done() ); From e7b1e5febc576faf7de44b5a5f57b1e68024414b Mon Sep 17 00:00:00 2001 From: "g. nicholas d'andrea" Date: Tue, 6 Oct 2026 01:10:18 -0400 Subject: [PATCH 2/5] bugc: mask packed storage writes to the field's width A write to a storage value narrower than a slot shifted the value into place without masking it to its length. A negative signed value is sign-extended across the whole word, so `s.c = -2 as int16` filled every higher byte of the slot with ff and clobbered the fields packed above it. Mask the value to its length before the shift. --- packages/bugc/CHANGELOG.md | 6 ++ packages/bugc/src/evmgen/behavioral.test.ts | 70 +++++++++++++++++++ .../evmgen/generation/instructions/storage.ts | 9 ++- .../src/irgen/generate/statements/assign.ts | 19 ++--- packages/bugc/src/irgen/generate/storage.ts | 34 ++++++--- 5 files changed, 113 insertions(+), 25 deletions(-) diff --git a/packages/bugc/CHANGELOG.md b/packages/bugc/CHANGELOG.md index 919d000f49..64c7cf1cf0 100644 --- a/packages/bugc/CHANGELOG.md +++ b/packages/bugc/CHANGELOG.md @@ -92,6 +92,12 @@ support. Changes to the specification itself are tracked in the root `int8` that holds `-56` reads as `-56`. Before, at every optimization level, a packed field read back as its unsigned bytes, such as `200` ([#337]). +- A write to a storage value narrower than a slot, whether a struct + field or a variable such as `x: int8`, now writes only that value's + bytes. Before, at every optimization level, a negative signed value + such as `-2 as int16` filled every higher byte of the slot with `ff`, + so the fields packed above it read back wrong, and `x = -56` stored a + full word where Solidity stores one byte ([#337]). ## 0.1.0-preview.1 — 2026-10-03 diff --git a/packages/bugc/src/evmgen/behavioral.test.ts b/packages/bugc/src/evmgen/behavioral.test.ts index c5deaee2da..7f47d2225a 100644 --- a/packages/bugc/src/evmgen/behavioral.test.ts +++ b/packages/bugc/src/evmgen/behavioral.test.ts @@ -1323,6 +1323,76 @@ code { size = msg.data.length; }`; } }); + describe("packed storage writes", () => { + // Fields pack from the low-order end of slot 0: + // a (1 byte), b (1), c (2), d (4), e (8). + const define = `define { + struct S { a: int8; b: uint8; c: int16; d: uint32; e: int64; }; +}`; + + // a = -56, b = 7, c = -2, d = 0x01020304, e = -3 + const word = BigInt( + "0x" + "00".repeat(16) + "fffffffffffffffd" + "01020304" + "fffe07c8", + ); + + const fields = [ + "s.a = -56 as int8;", + "s.b = 7 as uint8;", + "s.c = -2 as int16;", + "s.d = 16909060 as uint32;", + "s.e = -3 as int64;", + ]; + + for (const level of [0, 1, 2, 3] as const) { + it(`should mask constants in create (level ${level})`, async () => { + const source = `name PackedCreate; +${define} +storage { [0] s: S; [1] x: uint256; } +create { ${fields.join(" ")} } +code { x = 1; }`; + + const result = await executeProgram(source, { + optimizationLevel: level, + }); + expect(await result.getStorage(0n)).toBe(word); + }); + + it(`should mask constants in either order (level ${level})`, async () => { + const source = `name PackedReverse; +${define} +storage { [0] s: S; [1] x: uint256; } +create { x = 1; } +code { ${[...fields].reverse().join(" ")} }`; + + const result = await executeProgram(source, { + calldata: "", + optimizationLevel: level, + }); + expect(result.callSuccess).toBe(true); + expect(await result.getStorage(0n)).toBe(word); + }); + } + }); + + describe("narrow storage variables", () => { + const source = `name Narrow; +storage { [0] x: int8; [1] y: int16; [2] n: int256; } +create { x = -56 as int8; n = -2 as int256; } +code { let v = n as int16; y = v; }`; + + for (const level of [0, 1, 2, 3] as const) { + it(`should write only their own bytes (level ${level})`, async () => { + const result = await executeProgram(source, { + calldata: "", + optimizationLevel: level, + }); + expect(result.callSuccess).toBe(true); + expect(await result.getStorage(0n)).toBe(0xc8n); + expect(await result.getStorage(1n)).toBe(0xfffen); + }); + } + }); + describe("signed storage reads", () => { const neg = (n: bigint) => 2n ** 256n - n; diff --git a/packages/bugc/src/evmgen/generation/instructions/storage.ts b/packages/bugc/src/evmgen/generation/instructions/storage.ts index 9eebee0f47..6132b3c541 100644 --- a/packages/bugc/src/evmgen/generation/instructions/storage.ts +++ b/packages/bugc/src/evmgen/generation/instructions/storage.ts @@ -435,8 +435,13 @@ function generateStorageWrite( // current & clearMask .then(AND({ debug }), { as: "clearedCurrent" }) - // Prepare new value at offset - .then(loadValue(inst.value!, { debug }), { as: "value" }) + // Prepare new value at offset, masked to its length so that + // a sign-extended negative value cannot clobber its neighbors + .then(loadValue(inst.value!, { debug }), { as: "b" }) + .then(PUSHn((1n << (BigInt(length) * 8n)) - 1n, { debug }), { + as: "a", + }) + .then(AND({ debug }), { as: "value" }) .then(PUSHn(BigInt(offset) * 8n, { debug }), { as: "shift" }) .then(SHL({ debug }), { as: "shiftedValue" }) diff --git a/packages/bugc/src/irgen/generate/statements/assign.ts b/packages/bugc/src/irgen/generate/statements/assign.ts index d3c2d6fe9d..dd68424b75 100644 --- a/packages/bugc/src/irgen/generate/statements/assign.ts +++ b/packages/bugc/src/irgen/generate/statements/assign.ts @@ -7,7 +7,11 @@ import { buildExpression } from "../expressions/index.js"; import { Process } from "../process.js"; import type { Context } from "../expressions/context.js"; -import { findStorageAccessChain, emitStorageChainStore } from "../storage.js"; +import { + findStorageAccessChain, + emitStorageChainStore, + emitStorageVariableStore, +} from "../storage.js"; /** * Build an assignment statement @@ -112,18 +116,7 @@ function* assignToTarget(node: Ast.Expression, value: Ir.Value): Process { // Check if it's storage const storageSlot = yield* Process.Storage.findSlot(name); if (storageSlot) { - yield* Process.Instructions.emit({ - kind: "write", - location: "storage", - slot: Ir.Value.constant( - BigInt(storageSlot.slot), - Ir.Type.Scalar.uint256, - ), - offset: Ir.Value.constant(0n, Ir.Type.Scalar.uint256), - length: Ir.Value.constant(32n, Ir.Type.Scalar.uint256), // 32 bytes for uint256 - value, - operationDebug: yield* Process.Debug.forAstNode(node), - } as Ir.Instruction.Write); + yield* emitStorageVariableStore(storageSlot, value, node); return; } diff --git a/packages/bugc/src/irgen/generate/storage.ts b/packages/bugc/src/irgen/generate/storage.ts index 1d22695ee7..7d63cc9982 100644 --- a/packages/bugc/src/irgen/generate/storage.ts +++ b/packages/bugc/src/irgen/generate/storage.ts @@ -128,6 +128,29 @@ function getFieldSize(type: Ir.Type): number { return 32; } +/** + * Emit a write of a whole storage variable. A variable narrower than a + * slot writes only its own bytes, at the low-order end of the slot. + */ +export function* emitStorageVariableStore( + slot: { slot: number; declaration: Ast.Declaration.Storage }, + value: Ir.Value, + node: Ast.Node | undefined, +): Process { + const type = yield* Process.Types.nodeType(slot.declaration); + const size = type ? getFieldSize(fromBugType(type)) : 32; + + yield* Process.Instructions.emit({ + kind: "write", + location: "storage", + slot: Ir.Value.constant(BigInt(slot.slot), Ir.Type.Scalar.uint256), + offset: Ir.Value.constant(0n, Ir.Type.Scalar.uint256), + length: Ir.Value.constant(BigInt(size), Ir.Type.Scalar.uint256), + value, + operationDebug: node ? yield* Process.Debug.forAstNode(node) : {}, + } as Ir.Instruction.Write); +} + /** * Emit a storage chain load */ @@ -277,16 +300,7 @@ export function* emitStorageChainStore( ): Process { // Handle direct storage variable assignment (no accesses) if (chain.accesses.length === 0) { - // Direct storage assignment using new unified format - yield* Process.Instructions.emit({ - kind: "write", - location: "storage", - slot: Ir.Value.constant(BigInt(chain.slot.slot), Ir.Type.Scalar.uint256), - offset: Ir.Value.constant(0n, Ir.Type.Scalar.uint256), - length: Ir.Value.constant(32n, Ir.Type.Scalar.uint256), - value, - operationDebug: node ? yield* Process.Debug.forAstNode(node) : {}, - } as Ir.Instruction.Write); + yield* emitStorageVariableStore(chain.slot, value, node); return; } From 7c4937ece46539b378152559437bce4c32245855 Mon Sep 17 00:00:00 2001 From: "g. nicholas d'andrea" Date: Tue, 6 Oct 2026 01:11:27 -0400 Subject: [PATCH 3/5] bugc: mask each value when merging packed writes Read/write merging combined adjacent writes to one slot with shl and or, and never masked each value to its length. A sign-extended narrow value set every bit above it, so a negative field overwrote its neighbors at level 3. Mask each value to its length before the shift. --- packages/bugc/CHANGELOG.md | 3 + packages/bugc/src/evmgen/behavioral.test.ts | 55 +++++++++- .../src/optimizer/steps/read-write-merging.ts | 101 +++++++----------- 3 files changed, 93 insertions(+), 66 deletions(-) diff --git a/packages/bugc/CHANGELOG.md b/packages/bugc/CHANGELOG.md index 64c7cf1cf0..9418fe500b 100644 --- a/packages/bugc/CHANGELOG.md +++ b/packages/bugc/CHANGELOG.md @@ -98,6 +98,9 @@ support. Changes to the specification itself are tracked in the root such as `-2 as int16` filled every higher byte of the slot with `ff`, so the fields packed above it read back wrong, and `x = -56` stored a full word where Solidity stores one byte ([#337]). +- At optimization level 3, merging adjacent writes to one slot now masks + each value to its width. Before, a negative signed value written next + to other fields of a packed struct overwrote them ([#337]). ## 0.1.0-preview.1 — 2026-10-03 diff --git a/packages/bugc/src/evmgen/behavioral.test.ts b/packages/bugc/src/evmgen/behavioral.test.ts index 7f47d2225a..727dfb12c5 100644 --- a/packages/bugc/src/evmgen/behavioral.test.ts +++ b/packages/bugc/src/evmgen/behavioral.test.ts @@ -1371,6 +1371,48 @@ code { ${[...fields].reverse().join(" ")} }`; expect(result.callSuccess).toBe(true); expect(await result.getStorage(0n)).toBe(word); }); + + it(`should mask runtime values (level ${level})`, async () => { + const source = `name PackedRuntime; +${define} +storage { + [0] s: S; + [1] na: int256; + [2] nc: int256; + [3] ne: int256; + [4] t: S; +} +create { + na = -56 as int256; + nc = -2 as int256; + ne = -3 as int256; +} +code { + let a = na as int8; + let b = 7 as uint8; + let c = nc as int16; + let d = 16909060 as uint32; + let e = ne as int64; + s.a = a; + s.b = b; + s.c = c; + s.d = d; + s.e = e; + t.e = s.e; + t.d = s.d; + t.c = s.c; + t.b = s.b; + t.a = s.a; +}`; + + const result = await executeProgram(source, { + calldata: "", + optimizationLevel: level, + }); + expect(result.callSuccess).toBe(true); + expect(await result.getStorage(0n)).toBe(word); + expect(await result.getStorage(4n)).toBe(word); + }); } }); @@ -1405,6 +1447,8 @@ storage { [1] x: int8; [2] y: int16; [3] m: mapping; + [4] t: R; + [5] n: int256; [10] o0: int256; [11] o1: int256; [12] o2: int256; @@ -1412,6 +1456,8 @@ storage { [14] o4: int256; [15] o5: int256; [16] o6: int256; + [17] o7: int256; + [18] o8: int256; } create { s.a = -56 as int8; @@ -1421,8 +1467,13 @@ create { x = -56 as int8; y = -300 as int16; m[5] = -7 as int16; + n = -9 as int256; } code { + let a = n as int8; + let b = n as int16; + t.a = a; + t.b = b; o0 = s.a; o1 = s.b; o2 = s.c; @@ -1430,9 +1481,11 @@ code { o4 = x; o5 = y; o6 = m[5]; + o7 = t.a; + o8 = t.b; }`; - const expected = [56n, 2n, 3n, 4n, 56n, 300n, 7n].map(neg); + const expected = [56n, 2n, 3n, 4n, 56n, 300n, 7n, 9n, 9n].map(neg); for (const level of [0, 1, 2, 3] as const) { it(`should sign-extend narrow signed values (level ${level})`, async () => { diff --git a/packages/bugc/src/optimizer/steps/read-write-merging.ts b/packages/bugc/src/optimizer/steps/read-write-merging.ts index cb24ba1c7b..44a8ac9b58 100644 --- a/packages/bugc/src/optimizer/steps/read-write-merging.ts +++ b/packages/bugc/src/optimizer/steps/read-write-merging.ts @@ -253,79 +253,50 @@ export class ReadWriteMergingStep extends BaseOptimizationStep { let combinedValue: Ir.Value | null = null; let combinedDebug: Ir.Instruction.Debug = {}; - for (let i = 0; i < writeInfos.length; i++) { - const info = writeInfos[i]; - const shiftBits = info.offset * 8n; + // Emit `dest = left op right` and return dest as a value + const emit = ( + op: "and" | "shl" | "or", + left: Ir.Value, + right: Ir.Value, + ): Ir.Value => { + const dest = `t${tempCounter++}`; + instructions.push({ + kind: "binary", + op, + left, + right, + dest, + operationDebug: combinedDebug, + }); + return { kind: "temp", id: dest, type: Ir.Type.Scalar.uint256 }; + }; + const constant = (value: bigint): Ir.Value => ({ + kind: "const", + value, + type: Ir.Type.Scalar.uint256, + }); + for (const info of writeInfos) { // Track debug contexts as we process writes combinedDebug = Ir.Utils.combineDebugContexts( combinedDebug, info.write.operationDebug, ); - if (shiftBits > 0n) { - // Need to shift the value - const shiftTemp = `t${tempCounter++}`; - instructions.push({ - kind: "binary", - op: "shl", - left: info.write.value, - right: { - kind: "const", - value: shiftBits, - type: Ir.Type.Scalar.uint256, - }, - dest: shiftTemp, - operationDebug: combinedDebug, - }); - - const shiftedValue: Ir.Value = { - kind: "temp", - id: shiftTemp, - type: Ir.Type.Scalar.uint256, - }; - - if (combinedValue === null) { - combinedValue = shiftedValue; - } else { - // OR with previous combined value - const orTemp = `t${tempCounter++}`; - instructions.push({ - kind: "binary", - op: "or", - left: combinedValue, - right: shiftedValue, - dest: orTemp, - operationDebug: combinedDebug, - }); - combinedValue = { - kind: "temp", - id: orTemp, - type: Ir.Type.Scalar.uint256, - }; - } - } else { - // No shift needed - if (combinedValue === null) { - combinedValue = info.write.value; - } else { - // OR with previous combined value - const orTemp = `t${tempCounter++}`; - instructions.push({ - kind: "binary", - op: "or", - left: combinedValue, - right: info.write.value, - dest: orTemp, - operationDebug: combinedDebug, - }); - combinedValue = { - kind: "temp", - id: orTemp, - type: Ir.Type.Scalar.uint256, - }; - } + // Mask the value to its length, so that a sign-extended + // negative value cannot clobber the other parts. Code generation + // masks the merged value to its length, which covers the last part + let part = info.write.value; + if (info !== writeInfos[writeInfos.length - 1]) { + part = emit("and", part, constant((1n << (info.length * 8n)) - 1n)); + } + + if (info.offset > 0n) { + part = emit("shl", part, constant(info.offset * 8n)); } + + combinedValue = + combinedValue === null ? part : emit("or", combinedValue, part); } // Calculate the merged write parameters From 974b358f5199fc42257e1784b997c081a5e62fe4 Mon Sep 17 00:00:00 2001 From: "g. nicholas d'andrea" Date: Tue, 6 Oct 2026 01:12:36 -0400 Subject: [PATCH 4/5] bugc: place merged packed writes at their own offsets Read/write merging shifted each value by its field's offset in the slot, then wrote the result at the offset of the first field. When the first field was not at offset 0, every value moved by that offset twice. Shift each value by its offset from the first field. --- packages/bugc/CHANGELOG.md | 4 ++++ packages/bugc/src/evmgen/behavioral.test.ts | 22 +++++++++++++++++++ .../src/optimizer/steps/read-write-merging.ts | 6 +++-- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/packages/bugc/CHANGELOG.md b/packages/bugc/CHANGELOG.md index 9418fe500b..c164b1a09a 100644 --- a/packages/bugc/CHANGELOG.md +++ b/packages/bugc/CHANGELOG.md @@ -101,6 +101,10 @@ support. Changes to the specification itself are tracked in the root - At optimization level 3, merging adjacent writes to one slot now masks each value to its width. Before, a negative signed value written next to other fields of a packed struct overwrote them ([#337]). +- At optimization level 3, merged writes to one slot that do not start + at the slot's first byte now land at their fields' offsets. Before, + each value was shifted by its field's offset twice, so writing `s.b` + and `s.c` but not `s.a` put them in the wrong bytes ([#337]). ## 0.1.0-preview.1 — 2026-10-03 diff --git a/packages/bugc/src/evmgen/behavioral.test.ts b/packages/bugc/src/evmgen/behavioral.test.ts index 727dfb12c5..e378e69560 100644 --- a/packages/bugc/src/evmgen/behavioral.test.ts +++ b/packages/bugc/src/evmgen/behavioral.test.ts @@ -1413,6 +1413,28 @@ code { expect(await result.getStorage(0n)).toBe(word); expect(await result.getStorage(4n)).toBe(word); }); + + it(`should keep the fields around writes past offset 0 (level ${level})`, async () => { + const source = `name PackedMiddle; +${define} +storage { [0] s: S; [1] nc: int256; } +create { nc = -2 as int256; s.a = -56 as int8; s.e = -3 as int64; } +code { + let b = 7 as uint8; + let c = nc as int16; + let d = 16909060 as uint32; + s.b = b; + s.c = c; + s.d = d; +}`; + + const result = await executeProgram(source, { + calldata: "", + optimizationLevel: level, + }); + expect(result.callSuccess).toBe(true); + expect(await result.getStorage(0n)).toBe(word); + }); } }); diff --git a/packages/bugc/src/optimizer/steps/read-write-merging.ts b/packages/bugc/src/optimizer/steps/read-write-merging.ts index 44a8ac9b58..dab58a6942 100644 --- a/packages/bugc/src/optimizer/steps/read-write-merging.ts +++ b/packages/bugc/src/optimizer/steps/read-write-merging.ts @@ -291,8 +291,10 @@ export class ReadWriteMergingStep extends BaseOptimizationStep { part = emit("and", part, constant((1n << (info.length * 8n)) - 1n)); } - if (info.offset > 0n) { - part = emit("shl", part, constant(info.offset * 8n)); + // Place the part relative to the merged write's offset + const shift = (info.offset - writeInfos[0].offset) * 8n; + if (shift > 0n) { + part = emit("shl", part, constant(shift)); } combinedValue = From f7bc413b98dd17b253e23432c9d3e0d457bffbb7 Mon Sep 17 00:00:00 2001 From: "g. nicholas d'andrea" Date: Tue, 6 Oct 2026 01:31:46 -0400 Subject: [PATCH 5/5] bugc: do not merge packed writes that overlap Read/write merging accepted writes that overlap, such as two writes to one field, and combined their values with or, so the field got the bits of both. Merge only writes that each start where the previous one ends, and leave other groups as they are, in order. --- packages/bugc/CHANGELOG.md | 3 +++ packages/bugc/src/evmgen/behavioral.test.ts | 27 +++++++++++++++++++ .../src/optimizer/steps/read-write-merging.ts | 11 ++++---- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/packages/bugc/CHANGELOG.md b/packages/bugc/CHANGELOG.md index c164b1a09a..3fc4c7d36d 100644 --- a/packages/bugc/CHANGELOG.md +++ b/packages/bugc/CHANGELOG.md @@ -105,6 +105,9 @@ support. Changes to the specification itself are tracked in the root at the slot's first byte now land at their fields' offsets. Before, each value was shifted by its field's offset twice, so writing `s.b` and `s.c` but not `s.a` put them in the wrong bytes ([#337]). +- At optimization level 3, two writes in a row to the same field of a + packed struct now leave the second value. Before, read/write merging + combined the two values with `or` ([#337]). ## 0.1.0-preview.1 — 2026-10-03 diff --git a/packages/bugc/src/evmgen/behavioral.test.ts b/packages/bugc/src/evmgen/behavioral.test.ts index e378e69560..1b934f90dd 100644 --- a/packages/bugc/src/evmgen/behavioral.test.ts +++ b/packages/bugc/src/evmgen/behavioral.test.ts @@ -1435,6 +1435,33 @@ code { expect(result.callSuccess).toBe(true); expect(await result.getStorage(0n)).toBe(word); }); + + // Writes to one field: the later write replaces the earlier + for (const [writes, expected] of [ + ["s.a = a; s.a = ua;", 0x03n], + ["s.b = b; s.a = a; s.b = ub;", 0x0003c8n], + ] as const) { + it(`should apply \`${writes}\` in order (level ${level})`, async () => { + const source = `name PackedOverlap; +define { struct T { a: int8; b: int16; }; } +storage { [0] s: T; [1] n: int256; [2] u: uint256; } +create { n = -56 as int256; u = 3; } +code { + let a = n as int8; + let b = n as int16; + let ua = u as int8; + let ub = u as int16; + ${writes} +}`; + + const result = await executeProgram(source, { + calldata: "", + optimizationLevel: level, + }); + expect(result.callSuccess).toBe(true); + expect(await result.getStorage(0n)).toBe(expected); + }); + } } }); diff --git a/packages/bugc/src/optimizer/steps/read-write-merging.ts b/packages/bugc/src/optimizer/steps/read-write-merging.ts index dab58a6942..140342d1a6 100644 --- a/packages/bugc/src/optimizer/steps/read-write-merging.ts +++ b/packages/bugc/src/optimizer/steps/read-write-merging.ts @@ -239,7 +239,7 @@ export class ReadWriteMergingStep extends BaseOptimizationStep { // Sort by offset writeInfos.sort((a, b) => Number(a.offset - b.offset)); - // Check if writes are adjacent or overlapping (for simple merging) + // Check that the writes tile one range of the slot const canSimpleMerge = this.areWritesAdjacent(writeInfos); if (!canSimpleMerge) { @@ -357,16 +357,17 @@ export class ReadWriteMergingStep extends BaseOptimizationStep { } /** - * Check if writes are adjacent or can be easily combined + * Check that each write starts where the previous one ends */ private areWritesAdjacent(writeInfos: WriteInfo[]): boolean { for (let i = 1; i < writeInfos.length; i++) { const prev = writeInfos[i - 1]; const curr = writeInfos[i]; - // Check if current write starts at or before previous write ends - if (curr.offset > prev.offset + prev.length) { - // Gap between writes - not adjacent + // Each write must start where the previous one ends. A gap would + // clobber the bytes between them, and with an overlap the later + // write must replace the earlier one's bytes, not combine with them + if (curr.offset !== prev.offset + prev.length) { return false; } }