Commit 28ca203b7132ba0513a3854bd3bcbd0ee9bca067 | zig

author	Jakub Konka <kubkon@jakubkonka.com> 2022-04-21 10:46:04+02:00
committer	Jakub Konka <kubkon@jakubkonka.com> 2022-04-21 11:48:15+02:00
log	28ca203b7132ba0513a3854bd3bcbd0ee9bca067
tree	eabc9d3522d89fe8d050197bd6dc1c8a14c349a5
parent	96c1314443bdf26442a2c9fdffa03f2afffbcb8e

debug: fix resource (de)allocation for Elf and Coff targets

With this change, it is now possible to safely call
`var di = std.debug.openSelfDebugInfo(gpa)`. Calling then
`di.deinit()` on the object will correctly free all allocated
resources.

Ensure we store the result of `mmap` with correct alignment.

3 files changed, 40 insertions(+), 18 deletions(-)

lib
- std

lib/std/coff.zig+6-5

 @@ -4,8 +4,6 @@ const mem = std.mem;
 const os = std.os;
 const File = std.fs.File;
-const ArrayList = std.ArrayList;
 // CoffHeader.machine values
 // see https://msdn.microsoft.com/en-us/library/windows/desktop/ms680313(v=vs.85).aspx
 const IMAGE_FILE_MACHINE_I386 = 0x014c;
 @@ -117,7 +115,7 @@ pub const Coff = struct {
     coff_header: CoffHeader,
     pe_header: OptionalHeader,
-    sections: ArrayList(Section),
+    sections: std.ArrayListUnmanaged(Section) = .{},
     guid: [16]u8,
     age: u32,
 @@ -128,12 +126,15 @@ pub const Coff = struct {
             .allocator = allocator,
             .coff_header = undefined,
             .pe_header = undefined,
-            .sections = ArrayList(Section).init(allocator),
             .guid = undefined,
             .age = undefined,
         };
     }
+    pub fn deinit(self: *Coff) void {
+        self.sections.deinit(self.allocator);
+    }
     pub fn loadHeader(self: *Coff) !void {
         const pe_pointer_offset = 0x3C;
 @@ -291,7 +292,7 @@ pub const Coff = struct {
         if (self.sections.items.len == self.coff_header.number_of_sections)
             return;
-        try self.sections.ensureTotalCapacityPrecise(self.coff_header.number_of_sections);
+        try self.sections.ensureTotalCapacityPrecise(self.allocator, self.coff_header.number_of_sections);
         const in = self.in_file.reader();

lib/std/debug.zig+34-12

 @@ -41,7 +41,7 @@ pub const SymbolInfo = struct {
     compile_unit_name: []const u8 = "???",
     line_info: ?LineInfo = null,
-    pub fn deinit(self: @This(), allocator: mem.Allocator) void {
+    pub fn deinit(self: SymbolInfo, allocator: mem.Allocator) void {
         if (self.line_info) |li| {
             li.deinit(allocator);
         }
 @@ -50,6 +50,13 @@ pub const SymbolInfo = struct {
 const PdbOrDwarf = union(enum) {
     pdb: pdb.Pdb,
     dwarf: DW.DwarfInfo,
+    fn deinit(self: *PdbOrDwarf, allocator: mem.Allocator) void {
+        switch (self.*) {
+            .pdb => |*inner| inner.deinit(),
+            .dwarf => |*inner| inner.deinit(allocator),
+        }
+    }
 };
 var stderr_mutex = std.Thread.Mutex{};
 @@ -793,6 +800,7 @@ fn readCoffDebugInfo(allocator: mem.Allocator, coff_file: File) !ModuleDebugInfo
         errdefer coff_file.close();
         const coff_obj = try allocator.create(coff.Coff);
+        errdefer allocator.destroy(coff_obj);
         coff_obj.* = coff.Coff.init(allocator, coff_file);
         var di = ModuleDebugInfo{
 @@ -1386,7 +1394,7 @@ pub const DebugInfo = struct {
 pub const ModuleDebugInfo = switch (native_os) {
     .macos, .ios, .watchos, .tvos => struct {
         base_address: usize,
-        mapped_memory: []const u8,
+        mapped_memory: []align(mem.page_size) const u8,
         symbols: []const MachoSymbol,
         strings: [:0]const u8,
         ofiles: OFileTable,
 @@ -1406,6 +1414,7 @@ pub const ModuleDebugInfo = switch (native_os) {
             }
             self.ofiles.deinit();
             allocator.free(self.symbols);
+            os.munmap(self.mapped_memory);
         }
         fn loadOFile(self: *@This(), allocator: mem.Allocator, o_file_path: []const u8) !OFileInfo {
 @@ -1609,18 +1618,20 @@ pub const ModuleDebugInfo = switch (native_os) {
         debug_data: PdbOrDwarf,
         coff: *coff.Coff,
-        pub fn allocator(self: @This()) mem.Allocator {
+        fn deinit(self: *@This(), allocator: mem.Allocator) void {
-            return self.coff.allocator;
+            self.debug_data.deinit(allocator);
+            self.coff.deinit();
+            allocator.destroy(self.coff);
         }
-        pub fn getSymbolAtAddress(self: *@This(), address: usize) !SymbolInfo {
+        pub fn getSymbolAtAddress(self: *@This(), allocator: mem.Allocator, address: usize) !SymbolInfo {
             // Translate the VA into an address into this object
             const relocated_address = address - self.base_address;
             switch (self.debug_data) {
                 .dwarf => |*dwarf| {
                     const dwarf_address = relocated_address + self.coff.pe_header.image_base;
-                    return getSymbolFromDwarf(dwarf_address, dwarf);
+                    return getSymbolFromDwarf(allocator, dwarf_address, dwarf);
                 },
                 .pdb => {
                     // fallthrough to pdb handling
 @@ -1666,17 +1677,28 @@ pub const ModuleDebugInfo = switch (native_os) {
     .linux, .netbsd, .freebsd, .dragonfly, .openbsd, .haiku, .solaris => struct {
         base_address: usize,
         dwarf: DW.DwarfInfo,
-        mapped_memory: []const u8,
+        mapped_memory: []align(mem.page_size) const u8,
+        fn deinit(self: *@This(), allocator: mem.Allocator) void {
+            self.dwarf.deinit(allocator);
+            os.munmap(self.mapped_memory);
+        }
-        pub fn getSymbolAtAddress(self: *@This(), address: usize) !SymbolInfo {
+        pub fn getSymbolAtAddress(self: *@This(), allocator: mem.Allocator, address: usize) !SymbolInfo {
             // Translate the VA into an address into this object
             const relocated_address = address - self.base_address;
-            return getSymbolFromDwarf(relocated_address, &self.dwarf);
+            return getSymbolFromDwarf(allocator, relocated_address, &self.dwarf);
         }
     },
     .wasi => struct {
-        pub fn getSymbolAtAddress(self: *@This(), address: usize) !SymbolInfo {
+        fn deinit(self: *@This(), allocator: mem.Allocator) void {
+            _ = self;
+            _ = allocator;
+        }
+        pub fn getSymbolAtAddress(self: *@This(), allocator: mem.Allocator, address: usize) !SymbolInfo {
             _ = self;
+            _ = allocator;
             _ = address;
             return SymbolInfo{};
         }
 @@ -1684,14 +1706,14 @@ pub const ModuleDebugInfo = switch (native_os) {
     else => DW.DwarfInfo,
 };
-fn getSymbolFromDwarf(address: u64, di: *DW.DwarfInfo) !SymbolInfo {
+fn getSymbolFromDwarf(allocator: mem.Allocator, address: u64, di: *DW.DwarfInfo) !SymbolInfo {
     if (nosuspend di.findCompileUnit(address)) |compile_unit| {
         return SymbolInfo{
             .symbol_name = nosuspend di.getSymbolName(address) orelse "???",
             .compile_unit_name = compile_unit.die.getAttrString(di, DW.AT.name) catch |err| switch (err) {
                 error.MissingDebugInfo, error.InvalidDebugInfo => "???",
             },
-            .line_info = nosuspend di.getLineNumberInfo(compile_unit.*, address) catch |err| switch (err) {
+            .line_info = nosuspend di.getLineNumberInfo(allocator, compile_unit.*, address) catch |err| switch (err) {
                 error.MissingDebugInfo, error.InvalidDebugInfo => null,
                 else => return err,
             },

lib/std/pdb.zig-1

...	@@ -764,7 +764,6 @@ pub const Pdb = struct {	...	@@ -764,7 +764,6 @@ pub const Pdb = struct {
764	const flags = @ptrCast(*LineNumberEntry.Flags, &line_num_entry.Flags);	764	const flags = @ptrCast(*LineNumberEntry.Flags, &line_num_entry.Flags);
765		765
766	return debug.LineInfo{	766	return debug.LineInfo{
767	.allocator = self.allocator,
768	.file_name = source_file_name,	767	.file_name = source_file_name,
769	.line = flags.Start,	768	.line = flags.Start,
770	.column = column,	769	.column = column,