| author |  Andrew Kelley <andrew@ziglang.org> 2023-04-28 11:43:57-07:00 |
| committer |  GitHub <noreply@github.com> 2023-04-28 11:43:57-07:00 |
| log | fd6200eda6d4fe19c34a59430a88a9ce38d6d7a4 |
| tree | e409cf30751281f3e8ebe70fbe67400aa4a4cdea |
| parent | 011bc59e8aa4b62a286e4dd6e92371af5dac8b9f |
| parent | 15dafd16e65a8cd8ea434f0b4783f08875254cd4 |
| signature |  Signed by PGP key 4AEE18F83AFDEB23 |
Merge pull request #15431 from kcbanner/fix_decl_value_arena
sema: Rework Decl.value_arena to fix another memory corruption issue3 files changed, 98 insertions(+), 34 deletions(-)
test- cases
src/Module.zig+59-14
| ... | @@ -411,6 +411,46 @@ pub const WipCaptureScope = struct { | ... | @@ -411,6 +411,46 @@ pub const WipCaptureScope = struct { |
| 411 | } | 411 | } |
| 412 | }; | 412 | }; |
| 413 | 413 | ||
| 414 | const ValueArena = struct { | ||
| 415 | state: std.heap.ArenaAllocator.State, | ||
| 416 | state_acquired: ?*std.heap.ArenaAllocator.State = null, | ||
| 417 | |||
| 418 | /// If this ValueArena replaced an existing one during re-analysis, this is the previous instance | ||
| 419 | prev: ?*ValueArena = null, | ||
| 420 | |||
| 421 | /// Returns an allocator backed by either promoting `state`, or by the existing ArenaAllocator | ||
| 422 | /// that has already promoted `state`. `out_arena_allocator` provides storage for the initial promotion, | ||
| 423 | /// and must live until the matching call to release(). | ||
| 424 | pub fn acquire(self: *ValueArena, child_allocator: Allocator, out_arena_allocator: *std.heap.ArenaAllocator) Allocator { | ||
| 425 | if (self.state_acquired) |state_acquired| { | ||
| 426 | return @fieldParentPtr(std.heap.ArenaAllocator, "state", state_acquired).allocator(); | ||
| 427 | } | ||
| 428 | |||
| 429 | out_arena_allocator.* = self.state.promote(child_allocator); | ||
| 430 | self.state_acquired = &out_arena_allocator.state; | ||
| 431 | return out_arena_allocator.allocator(); | ||
| 432 | } | ||
| 433 | |||
| 434 | /// Releases the allocator acquired by `acquire. `arena_allocator` must match the one passed to `acquire`. | ||
| 435 | pub fn release(self: *ValueArena, arena_allocator: *std.heap.ArenaAllocator) void { | ||
| 436 | if (@fieldParentPtr(std.heap.ArenaAllocator, "state", self.state_acquired.?) == arena_allocator) { | ||
| 437 | self.state = self.state_acquired.?.*; | ||
| 438 | self.state_acquired = null; | ||
| 439 | } | ||
| 440 | } | ||
| 441 | |||
| 442 | pub fn deinit(self: ValueArena, child_allocator: Allocator) void { | ||
| 443 | assert(self.state_acquired == null); | ||
| 444 | |||
| 445 | const prev = self.prev; | ||
| 446 | self.state.promote(child_allocator).deinit(); | ||
| 447 | |||
| 448 | if (prev) |p| { | ||
| 449 | p.deinit(child_allocator); | ||
| 450 | } | ||
| 451 | } | ||
| 452 | }; | ||
| 453 | |||
| 414 | pub const Decl = struct { | 454 | pub const Decl = struct { |
| 415 | /// Allocated with Module's allocator; outlives the ZIR code. | 455 | /// Allocated with Module's allocator; outlives the ZIR code. |
| 416 | name: [*:0]const u8, | 456 | name: [*:0]const u8, |
| ... | @@ -429,7 +469,7 @@ pub const Decl = struct { | ... | @@ -429,7 +469,7 @@ pub const Decl = struct { |
| 429 | @"addrspace": std.builtin.AddressSpace, | 469 | @"addrspace": std.builtin.AddressSpace, |
| 430 | /// The memory for ty, val, align, linksection, and captures. | 470 | /// The memory for ty, val, align, linksection, and captures. |
| 431 | /// If this is `null` then there is no memory management needed. | 471 | /// If this is `null` then there is no memory management needed. |
| 432 | value_arena: ?*std.heap.ArenaAllocator.State = null, | 472 | value_arena: ?*ValueArena = null, |
| 433 | /// The direct parent namespace of the Decl. | 473 | /// The direct parent namespace of the Decl. |
| 434 | /// Reference to externally owned memory. | 474 | /// Reference to externally owned memory. |
| 435 | /// In the case of the Decl corresponding to a file, this is | 475 | /// In the case of the Decl corresponding to a file, this is |
| ... | @@ -607,7 +647,7 @@ pub const Decl = struct { | ... | @@ -607,7 +647,7 @@ pub const Decl = struct { |
| 607 | variable.deinit(gpa); | 647 | variable.deinit(gpa); |
| 608 | gpa.destroy(variable); | 648 | gpa.destroy(variable); |
| 609 | } | 649 | } |
| 610 | if (decl.value_arena) |arena_state| { | 650 | if (decl.value_arena) |value_arena| { |
| 611 | if (decl.owns_tv) { | 651 | if (decl.owns_tv) { |
| 612 | if (decl.val.castTag(.str_lit)) |str_lit| { | 652 | if (decl.val.castTag(.str_lit)) |str_lit| { |
| 613 | mod.string_literal_table.getPtrContext(str_lit.data, .{ | 653 | mod.string_literal_table.getPtrContext(str_lit.data, .{ |
| ... | @@ -615,7 +655,7 @@ pub const Decl = struct { | ... | @@ -615,7 +655,7 @@ pub const Decl = struct { |
| 615 | }).?.* = .none; | 655 | }).?.* = .none; |
| 616 | } | 656 | } |
| 617 | } | 657 | } |
| 618 | arena_state.promote(gpa).deinit(); | 658 | value_arena.deinit(gpa); |
| 619 | decl.value_arena = null; | 659 | decl.value_arena = null; |
| 620 | decl.has_tv = false; | 660 | decl.has_tv = false; |
| 621 | decl.owns_tv = false; | 661 | decl.owns_tv = false; |
| ... | @@ -624,9 +664,9 @@ pub const Decl = struct { | ... | @@ -624,9 +664,9 @@ pub const Decl = struct { |
| 624 | 664 | ||
| 625 | pub fn finalizeNewArena(decl: *Decl, arena: *std.heap.ArenaAllocator) !void { | 665 | pub fn finalizeNewArena(decl: *Decl, arena: *std.heap.ArenaAllocator) !void { |
| 626 | assert(decl.value_arena == null); | 666 | assert(decl.value_arena == null); |
| 627 | const arena_state = try arena.allocator().create(std.heap.ArenaAllocator.State); | 667 | const value_arena = try arena.allocator().create(ValueArena); |
| 628 | arena_state.* = arena.state; | 668 | value_arena.* = .{ .state = arena.state }; |
| 629 | decl.value_arena = arena_state; | 669 | decl.value_arena = value_arena; |
| 630 | } | 670 | } |
| 631 | 671 | ||
| 632 | /// This name is relative to the containing namespace of the decl. | 672 | /// This name is relative to the containing namespace of the decl. |
| ... | @@ -4537,15 +4577,20 @@ fn semaDecl(mod: *Module, decl_index: Decl.Index) !bool { | ... | @@ -4537,15 +4577,20 @@ fn semaDecl(mod: *Module, decl_index: Decl.Index) !bool { |
| 4537 | // We need the memory for the Type to go into the arena for the Decl | 4577 | // We need the memory for the Type to go into the arena for the Decl |
| 4538 | var decl_arena = std.heap.ArenaAllocator.init(gpa); | 4578 | var decl_arena = std.heap.ArenaAllocator.init(gpa); |
| 4539 | const decl_arena_allocator = decl_arena.allocator(); | 4579 | const decl_arena_allocator = decl_arena.allocator(); |
| 4540 | 4580 | const decl_value_arena = blk: { | |
| 4541 | const decl_arena_state = blk: { | ||
| 4542 | errdefer decl_arena.deinit(); | 4581 | errdefer decl_arena.deinit(); |
| 4543 | const s = try decl_arena_allocator.create(std.heap.ArenaAllocator.State); | 4582 | const s = try decl_arena_allocator.create(ValueArena); |
| 4583 | s.* = .{ .state = undefined }; | ||
| 4544 | break :blk s; | 4584 | break :blk s; |
| 4545 | }; | 4585 | }; |
| 4546 | defer { | 4586 | defer { |
| 4547 | decl_arena_state.* = decl_arena.state; | 4587 | if (decl.value_arena) |value_arena| { |
| 4548 | decl.value_arena = decl_arena_state; | 4588 | assert(value_arena.state_acquired == null); |
| 4589 | decl_value_arena.prev = value_arena; | ||
| 4590 | } | ||
| 4591 | |||
| 4592 | decl_value_arena.state = decl_arena.state; | ||
| 4593 | decl.value_arena = decl_value_arena; | ||
| 4549 | } | 4594 | } |
| 4550 | 4595 | ||
| 4551 | var analysis_arena = std.heap.ArenaAllocator.init(gpa); | 4596 | var analysis_arena = std.heap.ArenaAllocator.init(gpa); |
| ... | @@ -5493,9 +5538,9 @@ pub fn analyzeFnBody(mod: *Module, func: *Fn, arena: Allocator) SemaError!Air { | ... | @@ -5493,9 +5538,9 @@ pub fn analyzeFnBody(mod: *Module, func: *Fn, arena: Allocator) SemaError!Air { |
| 5493 | const decl = mod.declPtr(decl_index); | 5538 | const decl = mod.declPtr(decl_index); |
| 5494 | 5539 | ||
| 5495 | // Use the Decl's arena for captured values. | 5540 | // Use the Decl's arena for captured values. |
| 5496 | var decl_arena = decl.value_arena.?.promote(gpa); | 5541 | var decl_arena: std.heap.ArenaAllocator = undefined; |
| 5497 | defer decl.value_arena.?.* = decl_arena.state; | 5542 | const decl_arena_allocator = decl.value_arena.?.acquire(gpa, &decl_arena); |
| 5498 | const decl_arena_allocator = decl_arena.allocator(); | 5543 | defer decl.value_arena.?.release(&decl_arena); |
| 5499 | 5544 | ||
| 5500 | var sema: Sema = .{ | 5545 | var sema: Sema = .{ |
| 5501 | .mod = mod, | 5546 | .mod = mod, |
src/Sema.zig+18-20
| ... | @@ -2856,9 +2856,9 @@ fn zirEnumDecl( | ... | @@ -2856,9 +2856,9 @@ fn zirEnumDecl( |
| 2856 | const decl_val = try sema.analyzeDeclVal(block, src, new_decl_index); | 2856 | const decl_val = try sema.analyzeDeclVal(block, src, new_decl_index); |
| 2857 | done = true; | 2857 | done = true; |
| 2858 | 2858 | ||
| 2859 | var decl_arena = new_decl.value_arena.?.promote(gpa); | 2859 | var decl_arena: std.heap.ArenaAllocator = undefined; |
| 2860 | defer new_decl.value_arena.?.* = decl_arena.state; | 2860 | const decl_arena_allocator = new_decl.value_arena.?.acquire(gpa, &decl_arena); |
| 2861 | const decl_arena_allocator = decl_arena.allocator(); | 2861 | defer new_decl.value_arena.?.release(&decl_arena); |
| 2862 | 2862 | ||
| 2863 | extra_index = try mod.scanNamespace(&enum_obj.namespace, extra_index, decls_len, new_decl); | 2863 | extra_index = try mod.scanNamespace(&enum_obj.namespace, extra_index, decls_len, new_decl); |
| 2864 | 2864 | ||
| ... | @@ -26999,13 +26999,12 @@ const ComptimePtrMutationKit = struct { | ... | @@ -26999,13 +26999,12 @@ const ComptimePtrMutationKit = struct { |
| 26999 | 26999 | ||
| 27000 | fn beginArena(self: *ComptimePtrMutationKit, mod: *Module) Allocator { | 27000 | fn beginArena(self: *ComptimePtrMutationKit, mod: *Module) Allocator { |
| 27001 | const decl = mod.declPtr(self.decl_ref_mut.decl_index); | 27001 | const decl = mod.declPtr(self.decl_ref_mut.decl_index); |
| 27002 | self.decl_arena = decl.value_arena.?.promote(mod.gpa); | 27002 | return decl.value_arena.?.acquire(mod.gpa, &self.decl_arena); |
| 27003 | return self.decl_arena.allocator(); | ||
| 27004 | } | 27003 | } |
| 27005 | 27004 | ||
| 27006 | fn finishArena(self: *ComptimePtrMutationKit, mod: *Module) void { | 27005 | fn finishArena(self: *ComptimePtrMutationKit, mod: *Module) void { |
| 27007 | const decl = mod.declPtr(self.decl_ref_mut.decl_index); | 27006 | const decl = mod.declPtr(self.decl_ref_mut.decl_index); |
| 27008 | decl.value_arena.?.* = self.decl_arena.state; | 27007 | decl.value_arena.?.release(&self.decl_arena); |
| 27009 | self.decl_arena = undefined; | 27008 | self.decl_arena = undefined; |
| 27010 | } | 27009 | } |
| 27011 | }; | 27010 | }; |
| ... | @@ -27036,6 +27035,7 @@ fn beginComptimePtrMutation( | ... | @@ -27036,6 +27035,7 @@ fn beginComptimePtrMutation( |
| 27036 | .elem_ptr => { | 27035 | .elem_ptr => { |
| 27037 | const elem_ptr = ptr_val.castTag(.elem_ptr).?.data; | 27036 | const elem_ptr = ptr_val.castTag(.elem_ptr).?.data; |
| 27038 | var parent = try sema.beginComptimePtrMutation(block, src, elem_ptr.array_ptr, elem_ptr.elem_ty); | 27037 | var parent = try sema.beginComptimePtrMutation(block, src, elem_ptr.array_ptr, elem_ptr.elem_ty); |
| 27038 | |||
| 27039 | switch (parent.pointee) { | 27039 | switch (parent.pointee) { |
| 27040 | .direct => |val_ptr| switch (parent.ty.zigTypeTag()) { | 27040 | .direct => |val_ptr| switch (parent.ty.zigTypeTag()) { |
| 27041 | .Array, .Vector => { | 27041 | .Array, .Vector => { |
| ... | @@ -30653,10 +30653,9 @@ fn resolveStructLayout(sema: *Sema, ty: Type) CompileError!void { | ... | @@ -30653,10 +30653,9 @@ fn resolveStructLayout(sema: *Sema, ty: Type) CompileError!void { |
| 30653 | try sema.perm_arena.alloc(u32, struct_obj.fields.count()) | 30653 | try sema.perm_arena.alloc(u32, struct_obj.fields.count()) |
| 30654 | else blk: { | 30654 | else blk: { |
| 30655 | const decl = sema.mod.declPtr(struct_obj.owner_decl); | 30655 | const decl = sema.mod.declPtr(struct_obj.owner_decl); |
| 30656 | var decl_arena = decl.value_arena.?.promote(sema.mod.gpa); | 30656 | var decl_arena: std.heap.ArenaAllocator = undefined; |
| 30657 | defer decl.value_arena.?.* = decl_arena.state; | 30657 | const decl_arena_allocator = decl.value_arena.?.acquire(sema.mod.gpa, &decl_arena); |
| 30658 | const decl_arena_allocator = decl_arena.allocator(); | 30658 | defer decl.value_arena.?.release(&decl_arena); |
| 30659 | |||
| 30660 | break :blk try decl_arena_allocator.alloc(u32, struct_obj.fields.count()); | 30659 | break :blk try decl_arena_allocator.alloc(u32, struct_obj.fields.count()); |
| 30661 | }; | 30660 | }; |
| 30662 | 30661 | ||
| ... | @@ -30700,9 +30699,9 @@ fn semaBackingIntType(mod: *Module, struct_obj: *Module.Struct) CompileError!voi | ... | @@ -30700,9 +30699,9 @@ fn semaBackingIntType(mod: *Module, struct_obj: *Module.Struct) CompileError!voi |
| 30700 | 30699 | ||
| 30701 | const decl_index = struct_obj.owner_decl; | 30700 | const decl_index = struct_obj.owner_decl; |
| 30702 | const decl = mod.declPtr(decl_index); | 30701 | const decl = mod.declPtr(decl_index); |
| 30703 | var decl_arena = decl.value_arena.?.promote(gpa); | 30702 | var decl_arena: std.heap.ArenaAllocator = undefined; |
| 30704 | defer decl.value_arena.?.* = decl_arena.state; | 30703 | const decl_arena_allocator = decl.value_arena.?.acquire(gpa, &decl_arena); |
| 30705 | const decl_arena_allocator = decl_arena.allocator(); | 30704 | defer decl.value_arena.?.release(&decl_arena); |
| 30706 | 30705 | ||
| 30707 | const zir = struct_obj.namespace.file_scope.zir; | 30706 | const zir = struct_obj.namespace.file_scope.zir; |
| 30708 | const extended = zir.instructions.items(.data)[struct_obj.zir_index].extended; | 30707 | const extended = zir.instructions.items(.data)[struct_obj.zir_index].extended; |
| ... | @@ -31394,9 +31393,9 @@ fn semaStructFields(mod: *Module, struct_obj: *Module.Struct) CompileError!void | ... | @@ -31394,9 +31393,9 @@ fn semaStructFields(mod: *Module, struct_obj: *Module.Struct) CompileError!void |
| 31394 | } | 31393 | } |
| 31395 | 31394 | ||
| 31396 | const decl = mod.declPtr(decl_index); | 31395 | const decl = mod.declPtr(decl_index); |
| 31397 | var decl_arena = decl.value_arena.?.promote(gpa); | 31396 | var decl_arena: std.heap.ArenaAllocator = undefined; |
| 31398 | defer decl.value_arena.?.* = decl_arena.state; | 31397 | const decl_arena_allocator = decl.value_arena.?.acquire(gpa, &decl_arena); |
| 31399 | const decl_arena_allocator = decl_arena.allocator(); | 31398 | defer decl.value_arena.?.release(&decl_arena); |
| 31400 | 31399 | ||
| 31401 | var analysis_arena = std.heap.ArenaAllocator.init(gpa); | 31400 | var analysis_arena = std.heap.ArenaAllocator.init(gpa); |
| 31402 | defer analysis_arena.deinit(); | 31401 | defer analysis_arena.deinit(); |
| ... | @@ -31734,10 +31733,9 @@ fn semaUnionFields(mod: *Module, union_obj: *Module.Union) CompileError!void { | ... | @@ -31734,10 +31733,9 @@ fn semaUnionFields(mod: *Module, union_obj: *Module.Union) CompileError!void { |
| 31734 | extra_index += body.len; | 31733 | extra_index += body.len; |
| 31735 | 31734 | ||
| 31736 | const decl = mod.declPtr(decl_index); | 31735 | const decl = mod.declPtr(decl_index); |
| 31737 | 31736 | var decl_arena: std.heap.ArenaAllocator = undefined; | |
| 31738 | var decl_arena = decl.value_arena.?.promote(gpa); | 31737 | const decl_arena_allocator = decl.value_arena.?.acquire(gpa, &decl_arena); |
| 31739 | defer decl.value_arena.?.* = decl_arena.state; | 31738 | defer decl.value_arena.?.release(&decl_arena); |
| 31740 | const decl_arena_allocator = decl_arena.allocator(); | ||
| 31741 | 31739 | ||
| 31742 | var analysis_arena = std.heap.ArenaAllocator.init(gpa); | 31740 | var analysis_arena = std.heap.ArenaAllocator.init(gpa); |
| 31743 | defer analysis_arena.deinit(); | 31741 | defer analysis_arena.deinit(); |
test/cases/decl_value_arena.zig created+21
| ... | @@ -0,0 +1,21 @@ | ||
| 1 | pub const Protocols: struct { | ||
| 2 | list: *const fn(*Connection) void = undefined, | ||
| 3 | handShake: type = struct { | ||
| 4 | const stepStart: u8 = 0; | ||
| 5 | }, | ||
| 6 | } = .{}; | ||
| 7 | |||
| 8 | pub const Connection = struct { | ||
| 9 | streamBuffer: [0]u8 = undefined, | ||
| 10 | __lastReceivedPackets: [0]u8 = undefined, | ||
| 11 | |||
| 12 | handShakeState: u8 = Protocols.handShake.stepStart, | ||
| 13 | }; | ||
| 14 | |||
| 15 | pub fn main() void { | ||
| 16 | var conn: Connection = undefined; | ||
| 17 | _ = conn; | ||
| 18 | } | ||
| 19 | |||
| 20 | // run | ||
| 21 | // | ||