Skip to content

fix fork cache bug in MemoryAccessor.zig #24376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions lib/std/debug.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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| {
Expand All @@ -909,7 +906,6 @@ pub const StackIterator = struct {
di,
module.base_address,
&unwind_state.dwarf_context,
&it.ma,
null,
);
} else return error.MissingDebugInfo;
Expand Down Expand Up @@ -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;
Expand Down
45 changes: 17 additions & 28 deletions lib/std/debug/Dwarf.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand All @@ -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,
Expand Down Expand Up @@ -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]), 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]), eh_frame_len)) return bad();
if (cie_entry_header.type != .cie) return bad();

cie.* = try CommonInformationEntry.parse(
Expand Down Expand Up @@ -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,
Expand All @@ -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];
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1814,7 +1803,7 @@ pub fn scanCieFdeInfo(di: *Dwarf, allocator: Allocator, base_address: usize) !vo
if (di.section(frame_section)) |section_data| {
var fbr: FixedBufferReader = .{ .buf = section_data, .endian = di.endian };
while (fbr.pos < fbr.buf.len) {
const entry_header = try EntryHeader.read(&fbr, null, frame_section);
const entry_header = try EntryHeader.read(&fbr, false, frame_section);
switch (entry_header.type) {
.cie => {
const cie = try CommonInformationEntry.parse(
Expand Down Expand Up @@ -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,
Expand All @@ -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),
},
};
}
Expand Down
14 changes: 7 additions & 7 deletions lib/std/debug/Dwarf/expression.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
18 changes: 5 additions & 13 deletions lib/std/debug/FixedBufferReader.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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),
};
}

Expand Down
79 changes: 4 additions & 75 deletions lib/std/debug/MemoryAccessor.zig
Original file line number Diff line number Diff line change
Expand Up @@ -11,82 +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 => while (true) switch (ma.mem.handle) {
-2 => break,
-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, .SRCH => unreachable, // own pid is always valid
.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;
};
},
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 {
Expand Down
Loading