Remove state from std.debug.MemoryAccessor

Author: GrayHatter

lib/std/debug.zig

@@ -767,7 +767,6 @@ pub const StackIterator = struct {
     first_address: ?usize,
     // Last known value of the frame pointer register.
     fp: usize,
-    ma: MemoryAccessor = MemoryAccessor.init,
 
     // When SelfInfo and a register context is available, this iterator can unwind
     // stacks with frames that don't use a frame pointer (ie. -fomit-frame-pointer),
@@ -819,7 +818,6 @@ pub const StackIterator = struct {
     }
 
     pub fn deinit(it: *StackIterator) void {
-        it.ma.deinit();
         if (have_ucontext and it.unwind_state != null) it.unwind_state.?.dwarf_context.deinit();
     }
 
@@ -890,7 +888,6 @@ pub const StackIterator = struct {
                         unwind_state.debug_info.allocator,
                         module.base_address,
                         &unwind_state.dwarf_context,
-                        &it.ma,
                         unwind_info,
                         module.eh_frame,
                     )) |return_address| {
@@ -909,7 +906,6 @@ pub const StackIterator = struct {
                 di,
                 module.base_address,
                 &unwind_state.dwarf_context,
-                &it.ma,
                 null,
             );
         } else return error.MissingDebugInfo;
@@ -945,15 +941,15 @@ pub const StackIterator = struct {
 
         // Sanity check.
         if (fp == 0 or !mem.isAligned(fp, @alignOf(usize))) return null;
-        const new_fp = math.add(usize, it.ma.load(usize, fp) orelse return null, fp_bias) catch
+        const new_fp = math.add(usize, MemoryAccessor.load(usize, fp) orelse return null, fp_bias) catch
             return null;
 
         // Sanity check: the stack grows down thus all the parent frames must be
         // be at addresses that are greater (or equal) than the previous one.
         // A zero frame pointer often signals this is the last frame, that case
         // is gracefully handled by the next call to next_internal.
         if (new_fp != 0 and new_fp < it.fp) return null;
-        const new_pc = it.ma.load(usize, math.add(usize, fp, pc_offset) catch return null) orelse
+        const new_pc = MemoryAccessor.load(usize, math.add(usize, fp, pc_offset) catch return null) orelse
             return null;
 
         it.fp = new_fp;

lib/std/debug/Dwarf.zig

@@ -348,17 +348,11 @@ pub const ExceptionFrameHeader = struct {
         };
     }
 
-    fn isValidPtr(
-        self: ExceptionFrameHeader,
-        comptime T: type,
-        ptr: usize,
-        ma: *MemoryAccessor,
-        eh_frame_len: ?usize,
-    ) bool {
+    fn isValidPtr(self: ExceptionFrameHeader, comptime T: type, ptr: usize, eh_frame_len: ?usize) bool {
         if (eh_frame_len) |len| {
             return ptr >= self.eh_frame_ptr and ptr <= self.eh_frame_ptr + len - @sizeOf(T);
         } else {
-            return ma.load(T, ptr) != null;
+            return MemoryAccessor.load(T, ptr) != null;
         }
     }
 
@@ -369,7 +363,6 @@ pub const ExceptionFrameHeader = struct {
     /// If `eh_frame_len` is provided, then these checks can be skipped.
     pub fn findEntry(
         self: ExceptionFrameHeader,
-        ma: *MemoryAccessor,
         eh_frame_len: ?usize,
         eh_frame_hdr_ptr: usize,
         pc: usize,
@@ -430,15 +423,15 @@ pub const ExceptionFrameHeader = struct {
             .endian = native_endian,
         };
 
-        const fde_entry_header = try EntryHeader.read(&eh_frame_fbr, if (eh_frame_len == null) ma else null, .eh_frame);
-        if (fde_entry_header.entry_bytes.len > 0 and !self.isValidPtr(u8, @intFromPtr(&fde_entry_header.entry_bytes[fde_entry_header.entry_bytes.len - 1]), ma, eh_frame_len)) return bad();
+        const fde_entry_header = try EntryHeader.read(&eh_frame_fbr, eh_frame_len == null, .eh_frame);
+        if (fde_entry_header.entry_bytes.len > 0 and !self.isValidPtr(u8, @intFromPtr(&fde_entry_header.entry_bytes[fde_entry_header.entry_bytes.len - 1]), true, eh_frame_len)) return bad();
         if (fde_entry_header.type != .fde) return bad();
 
         // CIEs always come before FDEs (the offset is a subtraction), so we can assume this memory is readable
         const cie_offset = fde_entry_header.type.fde;
         try eh_frame_fbr.seekTo(cie_offset);
-        const cie_entry_header = try EntryHeader.read(&eh_frame_fbr, if (eh_frame_len == null) ma else null, .eh_frame);
-        if (cie_entry_header.entry_bytes.len > 0 and !self.isValidPtr(u8, @intFromPtr(&cie_entry_header.entry_bytes[cie_entry_header.entry_bytes.len - 1]), ma, eh_frame_len)) return bad();
+        const cie_entry_header = try EntryHeader.read(&eh_frame_fbr, eh_frame_len == null, .eh_frame);
+        if (cie_entry_header.entry_bytes.len > 0 and !self.isValidPtr(u8, @intFromPtr(&cie_entry_header.entry_bytes[cie_entry_header.entry_bytes.len - 1]), true, eh_frame_len)) return bad();
         if (cie_entry_header.type != .cie) return bad();
 
         cie.* = try CommonInformationEntry.parse(
@@ -485,15 +478,11 @@ pub const EntryHeader = struct {
 
     /// Reads a header for either an FDE or a CIE, then advances the fbr to the position after the trailing structure.
     /// `fbr` must be a FixedBufferReader backed by either the .eh_frame or .debug_frame sections.
-    pub fn read(
-        fbr: *FixedBufferReader,
-        opt_ma: ?*MemoryAccessor,
-        dwarf_section: Section.Id,
-    ) !EntryHeader {
+    pub fn read(fbr: *FixedBufferReader, checked: bool, dwarf_section: Section.Id) !EntryHeader {
         assert(dwarf_section == .eh_frame or dwarf_section == .debug_frame);
 
         const length_offset = fbr.pos;
-        const unit_header = try readUnitHeader(fbr, opt_ma);
+        const unit_header = try readUnitHeader(fbr, checked);
         const unit_length = cast(usize, unit_header.unit_length) orelse return bad();
         if (unit_length == 0) return .{
             .length_offset = length_offset,
@@ -505,8 +494,8 @@ pub const EntryHeader = struct {
         const end_offset = start_offset + unit_length;
         defer fbr.pos = end_offset;
 
-        const id = try if (opt_ma) |ma|
-            fbr.readAddressChecked(unit_header.format, ma)
+        const id = try if (checked)
+            fbr.readAddressChecked(unit_header.format)
         else
             fbr.readAddress(unit_header.format);
         const entry_bytes = fbr.buf[fbr.pos..end_offset];
@@ -855,7 +844,7 @@ fn scanAllFunctions(di: *Dwarf, allocator: Allocator) ScanError!void {
     while (this_unit_offset < fbr.buf.len) {
         try fbr.seekTo(this_unit_offset);
 
-        const unit_header = try readUnitHeader(&fbr, null);
+        const unit_header = try readUnitHeader(&fbr, false);
         if (unit_header.unit_length == 0) return;
         const next_offset = unit_header.header_length + unit_header.unit_length;
 
@@ -1044,7 +1033,7 @@ fn scanAllCompileUnits(di: *Dwarf, allocator: Allocator) ScanError!void {
     while (this_unit_offset < fbr.buf.len) {
         try fbr.seekTo(this_unit_offset);
 
-        const unit_header = try readUnitHeader(&fbr, null);
+        const unit_header = try readUnitHeader(&fbr, false);
         if (unit_header.unit_length == 0) return;
         const next_offset = unit_header.header_length + unit_header.unit_length;
 
@@ -1426,7 +1415,7 @@ fn runLineNumberProgram(d: *Dwarf, gpa: Allocator, compile_unit: *CompileUnit) !
     };
     try fbr.seekTo(line_info_offset);
 
-    const unit_header = try readUnitHeader(&fbr, null);
+    const unit_header = try readUnitHeader(&fbr, false);
     if (unit_header.unit_length == 0) return missing();
 
     const next_offset = unit_header.header_length + unit_header.unit_length;
@@ -1987,8 +1976,8 @@ const UnitHeader = struct {
     unit_length: u64,
 };
 
-fn readUnitHeader(fbr: *FixedBufferReader, opt_ma: ?*MemoryAccessor) ScanError!UnitHeader {
-    return switch (try if (opt_ma) |ma| fbr.readIntChecked(u32, ma) else fbr.readInt(u32)) {
+fn readUnitHeader(fbr: *FixedBufferReader, checked: bool) ScanError!UnitHeader {
+    return switch (try if (checked) fbr.readIntChecked(u32) else fbr.readInt(u32)) {
         0...0xfffffff0 - 1 => |unit_length| .{
             .format = .@"32",
             .header_length = 4,
@@ -1998,7 +1987,7 @@ fn readUnitHeader(fbr: *FixedBufferReader, opt_ma: ?*MemoryAccessor) ScanError!U
         0xffffffff => .{
             .format = .@"64",
             .header_length = 12,
-            .unit_length = try if (opt_ma) |ma| fbr.readIntChecked(u64, ma) else fbr.readInt(u64),
+            .unit_length = try if (checked) fbr.readIntChecked(u64) else fbr.readInt(u64),
         },
     };
 }

lib/std/debug/Dwarf/expression.zig

@@ -15,8 +15,8 @@ const assert = std.debug.assert;
 pub const Context = struct {
     /// The dwarf format of the section this expression is in
     format: std.dwarf.Format = .@"32",
-    /// If specified, any addresses will pass through before being accessed
-    memory_accessor: ?*std.debug.MemoryAccessor = null,
+    /// When true, addresses will be validated before being accessed
+    check_memory: bool = false,
     /// The compilation unit this expression relates to, if any
     compile_unit: ?*const std.debug.Dwarf.CompileUnit = null,
     /// When evaluating a user-presented expression, this is the address of the object being evaluated
@@ -465,12 +465,12 @@ pub fn StackMachine(comptime options: Options) type {
                         else => unreachable,
                     };
 
-                    if (context.memory_accessor) |memory_accessor| {
+                    if (context.check_memory) {
                         if (!switch (size) {
-                            1 => memory_accessor.load(u8, addr) != null,
-                            2 => memory_accessor.load(u16, addr) != null,
-                            4 => memory_accessor.load(u32, addr) != null,
-                            8 => memory_accessor.load(u64, addr) != null,
+                            1 => std.debug.MemoryAccessor.load(u8, addr) != null,
+                            2 => std.debug.MemoryAccessor.load(u16, addr) != null,
+                            4 => std.debug.MemoryAccessor.load(u32, addr) != null,
+                            8 => std.debug.MemoryAccessor.load(u64, addr) != null,
                             else => return error.InvalidExpression,
                         }) return error.InvalidExpression;
                     }

lib/std/debug/FixedBufferReader.zig

@@ -38,12 +38,8 @@ pub fn readInt(fbr: *FixedBufferReader, comptime T: type) Error!T {
     return std.mem.readInt(T, fbr.buf[fbr.pos..][0..size], fbr.endian);
 }
 
-pub fn readIntChecked(
-    fbr: *FixedBufferReader,
-    comptime T: type,
-    ma: *MemoryAccessor,
-) Error!T {
-    if (ma.load(T, @intFromPtr(fbr.buf[fbr.pos..].ptr)) == null)
+pub fn readIntChecked(fbr: *FixedBufferReader, comptime T: type) Error!T {
+    if (MemoryAccessor.load(T, @intFromPtr(fbr.buf[fbr.pos..].ptr)) == null)
         return error.InvalidBuffer;
 
     return fbr.readInt(T);
@@ -64,14 +60,10 @@ pub fn readAddress(fbr: *FixedBufferReader, format: std.dwarf.Format) Error!u64
     };
 }
 
-pub fn readAddressChecked(
-    fbr: *FixedBufferReader,
-    format: std.dwarf.Format,
-    ma: *MemoryAccessor,
-) Error!u64 {
+pub fn readAddressChecked(fbr: *FixedBufferReader, format: std.dwarf.Format) Error!u64 {
     return switch (format) {
-        .@"32" => try fbr.readIntChecked(u32, ma),
-        .@"64" => try fbr.readIntChecked(u64, ma),
+        .@"32" => try fbr.readIntChecked(u32),
+        .@"64" => try fbr.readIntChecked(u64),
     };
 }
 

lib/std/debug/MemoryAccessor.zig

@@ -11,90 +11,11 @@ const page_size_min = std.heap.page_size_min;
 
 const MemoryAccessor = @This();
 
-var cached_pid: posix.pid_t = -1;
-
-mem: switch (native_os) {
-    .linux => File,
-    else => void,
-},
-
-pub const init: MemoryAccessor = .{
-    .mem = switch (native_os) {
-        .linux => .{ .handle = -1 },
-        else => {},
-    },
-};
-
-pub fn deinit(ma: *MemoryAccessor) void {
-    switch (native_os) {
-        .linux => switch (ma.mem.handle) {
-            -2, -1 => {},
-            else => ma.mem.close(),
-        },
-        else => {},
-    }
-    ma.* = undefined;
-}
-
-fn read(ma: *MemoryAccessor, address: usize, buf: []u8) bool {
-    switch (native_os) {
-        .linux => linux: switch (ma.mem.handle) {
-            -2 => break :linux,
-            -1 => {
-                const linux = std.os.linux;
-                const pid = switch (@atomicLoad(posix.pid_t, &cached_pid, .monotonic)) {
-                    -1 => pid: {
-                        const pid = linux.getpid();
-                        @atomicStore(posix.pid_t, &cached_pid, pid, .monotonic);
-                        break :pid pid;
-                    },
-                    else => |pid| pid,
-                };
-                const bytes_read = linux.process_vm_readv(
-                    pid,
-                    &.{.{ .base = buf.ptr, .len = buf.len }},
-                    &.{.{ .base = @ptrFromInt(address), .len = buf.len }},
-                    0,
-                );
-                switch (linux.E.init(bytes_read)) {
-                    .SUCCESS => return bytes_read == buf.len,
-                    .FAULT => return false,
-                    .INVAL => unreachable, // EINVAL is returned when flags to process_vm_readv is non-zero
-                    .SRCH => {
-                        // process with pid does not exist, but it should be our pid, so we'll try again.
-                        // This can happen after a call to fork() without a corresponding exec()
-                        const newpid = linux.getpid();
-                        std.debug.assert(newpid != pid);
-                        @atomicStore(posix.pid_t, &cached_pid, newpid, .monotonic);
-                        continue :linux -1;
-                    },
-                    .PERM => {}, // Known to happen in containers.
-                    .NOMEM => {},
-                    .NOSYS => {}, // QEMU is known not to implement this syscall.
-                    else => unreachable, // unexpected
-                }
-                var path_buf: [
-                    std.fmt.count("/proc/{d}/mem", .{std.math.minInt(posix.pid_t)})
-                ]u8 = undefined;
-                const path = std.fmt.bufPrint(&path_buf, "/proc/{d}/mem", .{pid}) catch
-                    unreachable;
-                ma.mem = std.fs.openFileAbsolute(path, .{}) catch {
-                    ma.mem.handle = -2;
-                    break :linux;
-                };
-            },
-            else => return (ma.mem.pread(buf, address) catch return false) == buf.len,
-        },
-        else => {},
-    }
-    if (!isValidMemory(address)) return false;
-    @memcpy(buf, @as([*]const u8, @ptrFromInt(address)));
-    return true;
-}
-
-pub fn load(ma: *MemoryAccessor, comptime Type: type, address: usize) ?Type {
+pub fn load(comptime Type: type, address: usize) ?Type {
+    if (!isValidMemory(address)) return null;
     var result: Type = undefined;
-    return if (ma.read(address, std.mem.asBytes(&result))) result else null;
+    @memcpy(std.mem.asBytes(&result), @as([*]const u8, @ptrFromInt(address)));
+    return result;
 }
 
 pub fn isValidMemory(address: usize) bool {