Commit fb4ad37e0b

Andrew Kelley <andrew@ziglang.org>
2022-03-08 22:55:54
LLVM: fix memory leak of debug type names
This required adjusting `Type.nameAlloc` to be used with a general-purpose allocator and added `Type.nameAllocArena` for the arena use case (avoids allocation sometimes).
1 parent 874b51d
Changed files (4)
src/codegen/llvm.zig
@@ -1922,7 +1922,8 @@ pub const DeclGen = struct {
             .Int => {
                 const info = ty.intInfo(target);
                 assert(info.bits != 0);
-                const name = try ty.nameAlloc(gpa); // TODO this is a leak
+                const name = try ty.nameAlloc(gpa);
+                defer gpa.free(name);
                 const dwarf_encoding: c_uint = switch (info.signedness) {
                     .signed => DW.ATE.signed,
                     .unsigned => DW.ATE.unsigned,
@@ -1967,7 +1968,8 @@ pub const DeclGen = struct {
                 const di_file = try dg.object.getDIFile(gpa, owner_decl.src_namespace.file_scope);
                 const di_scope = try dg.namespaceToDebugScope(owner_decl.src_namespace);
 
-                const name = try ty.nameAlloc(gpa); // TODO this is a leak
+                const name = try ty.nameAlloc(gpa);
+                defer gpa.free(name);
                 var buffer: Type.Payload.Bits = undefined;
                 const int_ty = ty.intTagType(&buffer);
 
@@ -1989,7 +1991,8 @@ pub const DeclGen = struct {
             },
             .Float => {
                 const bits = ty.floatBits(target);
-                const name = try ty.nameAlloc(gpa); // TODO this is a leak
+                const name = try ty.nameAlloc(gpa);
+                defer gpa.free(name);
                 gop.value_ptr.* = dib.createBasicType(name, bits, DW.ATE.float);
                 return gop.value_ptr.*;
             },
@@ -2039,7 +2042,8 @@ pub const DeclGen = struct {
                     const ptr_ty = ty.slicePtrFieldType(&buf);
                     const len_ty = Type.usize;
 
-                    const name = try ty.nameAlloc(gpa); // TODO this is a leak
+                    const name = try ty.nameAlloc(gpa);
+                    defer gpa.free(name);
                     const di_file: ?*llvm.DIFile = null;
                     const line = 0;
                     const compile_unit_scope = dg.object.di_compile_unit.?.toScope();
@@ -2109,7 +2113,8 @@ pub const DeclGen = struct {
                 }
 
                 const elem_di_ty = try lowerDebugType(dg, ptr_info.pointee_type);
-                const name = try ty.nameAlloc(gpa); // TODO this is a leak
+                const name = try ty.nameAlloc(gpa);
+                defer gpa.free(name);
                 const ptr_di_ty = dib.createPointerType(
                     elem_di_ty,
                     target.cpu.arch.ptrBitWidth(),
@@ -2125,7 +2130,8 @@ pub const DeclGen = struct {
                     gop.value_ptr.* = dib.createBasicType("anyopaque", 0, DW.ATE.signed);
                     return gop.value_ptr.*;
                 }
-                const name = try ty.nameAlloc(gpa); // TODO this is a leak
+                const name = try ty.nameAlloc(gpa);
+                defer gpa.free(name);
                 const owner_decl = ty.getOwnerDecl();
                 const opaque_di_ty = dib.createForwardDeclType(
                     DW.TAG.structure_type,
@@ -2162,7 +2168,8 @@ pub const DeclGen = struct {
                 return vector_di_ty;
             },
             .Optional => {
-                const name = try ty.nameAlloc(gpa); // TODO this is a leak
+                const name = try ty.nameAlloc(gpa);
+                defer gpa.free(name);
                 var buf: Type.Payload.ElemType = undefined;
                 const child_ty = ty.optionalChild(&buf);
                 if (!child_ty.hasRuntimeBits()) {
@@ -2253,7 +2260,8 @@ pub const DeclGen = struct {
                     try dg.object.di_type_map.put(gpa, ty, err_set_di_ty);
                     return err_set_di_ty;
                 }
-                const name = try ty.nameAlloc(gpa); // TODO this is a leak
+                const name = try ty.nameAlloc(gpa);
+                defer gpa.free(name);
                 const di_file: ?*llvm.DIFile = null;
                 const line = 0;
                 const compile_unit_scope = dg.object.di_compile_unit.?.toScope();
@@ -2329,7 +2337,8 @@ pub const DeclGen = struct {
             },
             .Struct => {
                 const compile_unit_scope = dg.object.di_compile_unit.?.toScope();
-                const name = try ty.nameAlloc(gpa); // TODO this is a leak
+                const name = try ty.nameAlloc(gpa);
+                defer gpa.free(name);
                 const fwd_decl = dib.createReplaceableCompositeType(
                     DW.TAG.structure_type,
                     name.ptr,
@@ -2477,7 +2486,8 @@ pub const DeclGen = struct {
             .Union => {
                 const owner_decl = ty.getOwnerDecl();
 
-                const name = try ty.nameAlloc(gpa); // TODO this is a leak
+                const name = try ty.nameAlloc(gpa);
+                defer gpa.free(name);
                 const fwd_decl = dib.createReplaceableCompositeType(
                     DW.TAG.structure_type,
                     name.ptr,
src/link/Dwarf.zig
@@ -882,7 +882,7 @@ fn addDbgInfoType(
             const abi_size = ty.abiSize(target);
             try leb128.writeULEB128(dbg_info_buffer.writer(), abi_size);
             // DW.AT.name, DW.FORM.string
-            const struct_name = try ty.nameAlloc(arena);
+            const struct_name = try ty.nameAllocArena(arena);
             try dbg_info_buffer.ensureUnusedCapacity(struct_name.len + 1);
             dbg_info_buffer.appendSliceAssumeCapacity(struct_name);
             dbg_info_buffer.appendAssumeCapacity(0);
src/Sema.zig
@@ -12405,7 +12405,7 @@ fn zirTypeName(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai
     var anon_decl = try block.startAnonDecl(LazySrcLoc.unneeded);
     defer anon_decl.deinit();
 
-    const bytes = try ty.nameAlloc(anon_decl.arena());
+    const bytes = try ty.nameAllocArena(anon_decl.arena());
 
     const new_decl = try anon_decl.finish(
         try Type.Tag.array_u8_sentinel_0.create(anon_decl.arena(), bytes.len),
src/type.zig
@@ -1766,8 +1766,20 @@ pub const Type = extern union {
         }
     }
 
+    pub fn nameAllocArena(ty: Type, arena: Allocator) Allocator.Error![:0]const u8 {
+        return nameAllocAdvanced(ty, arena, true);
+    }
+
+    pub fn nameAlloc(ty: Type, gpa: Allocator) Allocator.Error![:0]const u8 {
+        return nameAllocAdvanced(ty, gpa, false);
+    }
+
     /// Returns a name suitable for `@typeName`.
-    pub fn nameAlloc(ty: Type, arena: Allocator) Allocator.Error![:0]const u8 {
+    pub fn nameAllocAdvanced(
+        ty: Type,
+        ally: Allocator,
+        is_arena: bool,
+    ) Allocator.Error![:0]const u8 {
         const t = ty.tag();
         switch (t) {
             .inferred_alloc_const => unreachable,
@@ -1812,71 +1824,79 @@ pub const Type = extern union {
             .noreturn,
             .var_args_param,
             .bound_fn,
-            => return @tagName(t),
+            => return maybeDupe(@tagName(t), ally, is_arena),
 
-            .enum_literal => return "@Type(.EnumLiteral)",
-            .@"null" => return "@Type(.Null)",
-            .@"undefined" => return "@Type(.Undefined)",
+            .enum_literal => return maybeDupe("@Type(.EnumLiteral)", ally, is_arena),
+            .@"null" => return maybeDupe("@Type(.Null)", ally, is_arena),
+            .@"undefined" => return maybeDupe("@Type(.Undefined)", ally, is_arena),
 
-            .empty_struct, .empty_struct_literal => return "struct {}",
+            .empty_struct, .empty_struct_literal => return maybeDupe("struct {}", ally, is_arena),
 
             .@"struct" => {
                 const struct_obj = ty.castTag(.@"struct").?.data;
-                return try arena.dupeZ(u8, std.mem.sliceTo(struct_obj.owner_decl.name, 0));
+                return try ally.dupeZ(u8, std.mem.sliceTo(struct_obj.owner_decl.name, 0));
             },
             .@"union", .union_tagged => {
                 const union_obj = ty.cast(Payload.Union).?.data;
-                return try arena.dupeZ(u8, std.mem.sliceTo(union_obj.owner_decl.name, 0));
+                return try ally.dupeZ(u8, std.mem.sliceTo(union_obj.owner_decl.name, 0));
             },
             .enum_full, .enum_nonexhaustive => {
                 const enum_full = ty.cast(Payload.EnumFull).?.data;
-                return try arena.dupeZ(u8, std.mem.sliceTo(enum_full.owner_decl.name, 0));
+                return try ally.dupeZ(u8, std.mem.sliceTo(enum_full.owner_decl.name, 0));
             },
             .enum_simple => {
                 const enum_simple = ty.castTag(.enum_simple).?.data;
-                return try arena.dupeZ(u8, std.mem.sliceTo(enum_simple.owner_decl.name, 0));
+                return try ally.dupeZ(u8, std.mem.sliceTo(enum_simple.owner_decl.name, 0));
             },
             .enum_numbered => {
                 const enum_numbered = ty.castTag(.enum_numbered).?.data;
-                return try arena.dupeZ(u8, std.mem.sliceTo(enum_numbered.owner_decl.name, 0));
+                return try ally.dupeZ(u8, std.mem.sliceTo(enum_numbered.owner_decl.name, 0));
             },
             .@"opaque" => {
-                // TODO use declaration name
-                return "opaque {}";
-            },
-
-            .anyerror_void_error_union => return "anyerror!void",
-            .const_slice_u8 => return "[]const u8",
-            .const_slice_u8_sentinel_0 => return "[:0]const u8",
-            .fn_noreturn_no_args => return "fn() noreturn",
-            .fn_void_no_args => return "fn() void",
-            .fn_naked_noreturn_no_args => return "fn() callconv(.Naked) noreturn",
-            .fn_ccc_void_no_args => return "fn() callconv(.C) void",
-            .single_const_pointer_to_comptime_int => return "*const comptime_int",
-            .manyptr_u8 => return "[*]u8",
-            .manyptr_const_u8 => return "[*]const u8",
-            .manyptr_const_u8_sentinel_0 => return "[*:0]const u8",
-            .atomic_order => return "AtomicOrder",
-            .atomic_rmw_op => return "AtomicRmwOp",
-            .calling_convention => return "CallingConvention",
-            .address_space => return "AddressSpace",
-            .float_mode => return "FloatMode",
-            .reduce_op => return "ReduceOp",
-            .call_options => return "CallOptions",
-            .prefetch_options => return "PrefetchOptions",
-            .export_options => return "ExportOptions",
-            .extern_options => return "ExternOptions",
-            .type_info => return "Type",
+                const opaque_obj = ty.cast(Payload.Opaque).?.data;
+                return try ally.dupeZ(u8, std.mem.sliceTo(opaque_obj.owner_decl.name, 0));
+            },
+
+            .anyerror_void_error_union => return maybeDupe("anyerror!void", ally, is_arena),
+            .const_slice_u8 => return maybeDupe("[]const u8", ally, is_arena),
+            .const_slice_u8_sentinel_0 => return maybeDupe("[:0]const u8", ally, is_arena),
+            .fn_noreturn_no_args => return maybeDupe("fn() noreturn", ally, is_arena),
+            .fn_void_no_args => return maybeDupe("fn() void", ally, is_arena),
+            .fn_naked_noreturn_no_args => return maybeDupe("fn() callconv(.Naked) noreturn", ally, is_arena),
+            .fn_ccc_void_no_args => return maybeDupe("fn() callconv(.C) void", ally, is_arena),
+            .single_const_pointer_to_comptime_int => return maybeDupe("*const comptime_int", ally, is_arena),
+            .manyptr_u8 => return maybeDupe("[*]u8", ally, is_arena),
+            .manyptr_const_u8 => return maybeDupe("[*]const u8", ally, is_arena),
+            .manyptr_const_u8_sentinel_0 => return maybeDupe("[*:0]const u8", ally, is_arena),
+            .atomic_order => return maybeDupe("AtomicOrder", ally, is_arena),
+            .atomic_rmw_op => return maybeDupe("AtomicRmwOp", ally, is_arena),
+            .calling_convention => return maybeDupe("CallingConvention", ally, is_arena),
+            .address_space => return maybeDupe("AddressSpace", ally, is_arena),
+            .float_mode => return maybeDupe("FloatMode", ally, is_arena),
+            .reduce_op => return maybeDupe("ReduceOp", ally, is_arena),
+            .call_options => return maybeDupe("CallOptions", ally, is_arena),
+            .prefetch_options => return maybeDupe("PrefetchOptions", ally, is_arena),
+            .export_options => return maybeDupe("ExportOptions", ally, is_arena),
+            .extern_options => return maybeDupe("ExternOptions", ally, is_arena),
+            .type_info => return maybeDupe("Type", ally, is_arena),
 
             else => {
                 // TODO this is wasteful and also an incorrect implementation of `@typeName`
-                var buf = std.ArrayList(u8).init(arena);
+                var buf = std.ArrayList(u8).init(ally);
                 try buf.writer().print("{}", .{ty});
                 return try buf.toOwnedSliceSentinel(0);
             },
         }
     }
 
+    fn maybeDupe(s: [:0]const u8, ally: Allocator, is_arena: bool) Allocator.Error![:0]const u8 {
+        if (is_arena) {
+            return s;
+        } else {
+            return try ally.dupeZ(u8, s);
+        }
+    }
+
     pub fn toValue(self: Type, allocator: Allocator) Allocator.Error!Value {
         switch (self.tag()) {
             .u1 => return Value.initTag(.u1_type),