Skip to content

Commit c728935

Browse files
authored
Consistently handle log topics and data ownership (#8)
* consistently handle log topics and data deinit() throughout. fixes some broken spec tests in zevm-stateless Signed-off-by: garyschulte <garyschulte@gmail.com>
1 parent cf0d429 commit c728935

File tree

4 files changed

+33
-36
lines changed

4 files changed

+33
-36
lines changed

src/context/journal.zig

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -397,12 +397,7 @@ pub const JournalInner = struct {
397397
pub fn deinit(self: *JournalInner) void {
398398
self.evm_state.deinit();
399399
self.transient_storage.deinit();
400-
// Free heap-allocated topic slices before freeing the log list
401-
for (self.logs.items) |log| {
402-
if (log.topics.len > 0) {
403-
alloc_mod.get().free(@constCast(log.topics));
404-
}
405-
}
400+
for (self.logs.items) |log| log.deinit(alloc_mod.get());
406401
self.logs.deinit(alloc_mod.get());
407402
self.journal.deinit(alloc_mod.get());
408403
self.warm_addresses.deinit();
@@ -441,12 +436,8 @@ pub const JournalInner = struct {
441436
self.journal.clearRetainingCapacity();
442437
self.warm_addresses.clearCoinbaseAndAccessList();
443438
self.transaction_id += 1;
444-
// Free heap-allocated topic slices (in case takeLogs() was not called)
445-
for (self.logs.items) |log| {
446-
if (log.topics.len > 0) {
447-
alloc_mod.get().free(@constCast(log.topics));
448-
}
449-
}
439+
// Free heap-allocated data/topics (in case takeLogs() was not called — e.g. failed tx)
440+
for (self.logs.items) |log| log.deinit(alloc_mod.get());
450441
self.logs.clearRetainingCapacity();
451442
}
452443

@@ -463,12 +454,8 @@ pub const JournalInner = struct {
463454
}
464455

465456
self.transient_storage.clearRetainingCapacity();
466-
// Free heap-allocated topic slices before clearing the log list
467-
for (self.logs.items) |log| {
468-
if (log.topics.len > 0) {
469-
alloc_mod.get().free(@constCast(log.topics));
470-
}
471-
}
457+
// Free heap-allocated data/topics before clearing the log list
458+
for (self.logs.items) |log| log.deinit(alloc_mod.get());
472459
self.logs.clearRetainingCapacity();
473460
self.transaction_id += 1;
474461
self.warm_addresses.clearCoinbaseAndAccessList();
@@ -483,12 +470,7 @@ pub const JournalInner = struct {
483470

484471
const evm_state = self.evm_state;
485472
self.evm_state = state.EvmState.init(alloc_mod.get());
486-
// Free heap-allocated topic slices before clearing the log list.
487-
for (self.logs.items) |log| {
488-
if (log.topics.len > 0) {
489-
alloc_mod.get().free(@constCast(log.topics));
490-
}
491-
}
473+
for (self.logs.items) |log| log.deinit(alloc_mod.get());
492474
self.logs.clearRetainingCapacity();
493475
self.transient_storage.clearRetainingCapacity();
494476
self.journal.clearRetainingCapacity();
@@ -722,6 +704,8 @@ pub const JournalInner = struct {
722704
/// Reverts all changes to evm_state until given checkpoint.
723705
pub fn checkpointRevert(self: *JournalInner, checkpoint: JournalCheckpoint) void {
724706
const is_spurious_dragon_enabled = primitives.isEnabledIn(self.spec, .spurious_dragon);
707+
// Free heap-allocated data/topics of logs being discarded by this revert.
708+
for (self.logs.items[checkpoint.log_i..]) |log| log.deinit(alloc_mod.get());
725709
self.logs.shrinkRetainingCapacity(checkpoint.log_i);
726710

727711
// iterate over last N journals sets and revert our global evm_state

src/handler/main.zig

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,7 @@ pub const ExecutionResult = struct {
6060

6161
/// Deinitialize execution result
6262
pub fn deinit(self: *ExecutionResult) void {
63-
// Free topic slices (heap-allocated via page_allocator by the journal's LOG opcodes).
64-
for (self.logs.items) |log| {
65-
if (log.topics.len > 0) {
66-
alloc_mod.get().free(@constCast(log.topics));
67-
}
68-
}
63+
for (self.logs.items) |log| log.deinit(alloc_mod.get());
6964
self.logs.deinit(alloc_mod.get());
7065
// Free heap-allocated return data copy (len==0 means static empty slice, skip).
7166
if (self.return_data.len > 0) {

src/interpreter/opcodes/host_ops.zig

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -516,12 +516,19 @@ pub fn makeLogFn(comptime n: u8) *const fn (ctx: *InstructionContext) void {
516516

517517
stack.shrinkUnsafe(2 + n);
518518

519-
// Get log data from memory (offset_u + size_u is safe; expandMemory verified it above)
519+
// Get log data from memory — copy to heap so the data outlives the interpreter.
520+
// The interpreter's memory buffer is freed when executeIterative returns (via defer),
521+
// but log entries must remain valid until postExecution reads them from the journal.
520522
const log_end = offset_u + size_u; // won't overflow: expandMemory already checked this
521-
const log_data: []const u8 = if (size_u > 0)
522-
ctx.interpreter.memory.buffer.items[offset_u..log_end]
523-
else
524-
&[_]u8{};
523+
const log_data: []const u8 = if (size_u > 0) blk: {
524+
const src = ctx.interpreter.memory.buffer.items[offset_u..log_end];
525+
const copy = alloc_mod.get().dupe(u8, src) catch {
526+
if (comptime n > 0) alloc_mod.get().free(topics);
527+
ctx.interpreter.halt(.out_of_gas);
528+
return;
529+
};
530+
break :blk copy;
531+
} else &[_]u8{};
525532

526533
// Emit the log
527534
const log_entry = primitives.Log{

src/primitives/main.zig

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,22 @@ pub const StorageValue = U256;
2626
/// Type alias for byte arrays
2727
pub const Bytes = []u8;
2828

29-
/// Log entry for EVM events
29+
/// Log entry for EVM events.
30+
///
31+
/// `data` and `topics` are heap-allocated (via the zevm_allocator) when non-empty,
32+
/// and must be freed with `deinit`. Zero-length slices use static empty literals and
33+
/// must NOT be freed.
3034
pub const Log = struct {
3135
address: Address,
3236
topics: []const Hash,
3337
data: []const u8,
38+
39+
/// Free heap-allocated data and topics. Safe to call on zero-length fields
40+
/// because the LOG opcode uses static empty literals for those cases.
41+
pub fn deinit(self: Log, alloc: std.mem.Allocator) void {
42+
if (self.data.len > 0) alloc.free(@constCast(self.data));
43+
if (self.topics.len > 0) alloc.free(@constCast(self.topics));
44+
}
3445
};
3546

3647
/// Optimize short address access.

0 commit comments

Comments
 (0)