From f57377abb96f871857f26f66887c0cdf77bfdf87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Balv=C3=ADn?= Date: Fri, 2 Oct 2026 00:40:54 +0200 Subject: [PATCH] fix(debug): handle mapping edges, stray traps and dying debuggees Assisted-by: GLM 5.3 --- debug/breakpoint.go | 36 +++- debug/debug_arch_test.go | 44 ++++ debug/debug_audit_test.go | 325 ++++++++++++++++++++++++++++++ debug/disasm_freebsd_amd64.go | 27 ++- debug/disasm_freebsd_riscv64.go | 26 ++- debug/disasm_linux_amd64.go | 27 ++- debug/disasm_linux_riscv64.go | 26 ++- debug/ptrace_freebsd.go | 22 ++ debug/ptrace_linux.go | 77 ++++--- debug/repl.go | 76 +++++-- debug/target_freebsd.go | 13 +- debug/target_linux_amd64.go | 13 +- debug/target_linux_arm64.go | 13 +- debug/target_linux_loong64.go | 13 +- debug/target_linux_riscv64.go | 13 +- debug/tracer.go | 16 ++ debug/watchpoint_freebsd_amd64.go | 22 +- debug/watchpoint_freebsd_arm64.go | 15 +- debug/watchpoint_linux_amd64.go | 19 +- 19 files changed, 726 insertions(+), 97 deletions(-) create mode 100644 debug/debug_audit_test.go diff --git a/debug/breakpoint.go b/debug/breakpoint.go index a1f3cf1..64e097a 100644 --- a/debug/breakpoint.go +++ b/debug/breakpoint.go @@ -7,7 +7,11 @@ package debug import "strings" -import "fmt" +import ( + "cmp" + "fmt" + "slices" +) // Breakpoint is one software breakpoint in the debuggee. type Breakpoint struct { @@ -149,15 +153,16 @@ func (bm *Breakpoints) SetWithCond(addr uint64, label string, cond *Condition) ( return bp, nil } -// Info returns a formatted list of all breakpoints. +// Info returns a formatted list of all breakpoints, ordered by address so +// the numbering is stable across calls (map iteration order is not). func (bm *Breakpoints) Info() string { if len(bm.bps) == 0 { return "no breakpoints set\n" } var result strings.Builder - i := 0 - for _, bp := range bm.bps { - i++ + bps := bm.All() + slices.SortFunc(bps, func(a, b *Breakpoint) int { return cmp.Compare(a.Addr, b.Addr) }) + for i, bp := range bps { status := "enabled" if !bp.Enabled { status = "disabled" @@ -170,7 +175,7 @@ func (bm *Breakpoints) Info() string { if bp.Cond != nil { cond = " if " + bp.Cond.String() } - result.WriteString(fmt.Sprintf(" %d: %s at %#x [%s, %d hits]%s\n", i, label, bp.Addr, status, bp.hits, cond)) + result.WriteString(fmt.Sprintf(" %d: %s at %#x [%s, %d hits]%s\n", i+1, label, bp.Addr, status, bp.hits, cond)) } return result.String() } @@ -238,6 +243,25 @@ func (bm *Breakpoints) All() []*Breakpoint { // Hits returns how many times the breakpoint has been hit. func (bp *Breakpoint) Hits() int { return bp.hits } +// TrapStray reports whether a stop is a breakpoint-class trap that matches +// no breakpoint of ours and cannot be resumed: the PC still stands on the +// trapping instruction (the kernel's own BRK, EBREAK or break, on an +// architecture that reports the trap in place), so the next resume would +// re-execute it and trap forever. trapPC is the PC the stop reported, +// before any HandleTrap rewinding; reason is the stop's StopInfo class. +// The debuggee's SIGSTOP barriers also stop without PC movement, and they +// never carry the breakpoint class, so they are unaffected. +func TrapStray(s *Session, reason StopReason, trapPC uint64) bool { + if reason != StopBreakpoint { + return false + } + after, err := s.GetRegs() + if err != nil { + return false + } + return after.GetPC() <= trapPC-uint64(breakpointPCAdjust) +} + func (bm *Breakpoints) HandleTrap(regs *Regs) *Breakpoint { // On amd64 the kernel reports the trap with RIP past the INT3; on the // other supported architectures the PC still stands on the trap diff --git a/debug/debug_arch_test.go b/debug/debug_arch_test.go index 505bd22..335ba0d 100644 --- a/debug/debug_arch_test.go +++ b/debug/debug_arch_test.go @@ -10,6 +10,8 @@ package debug // build on every supported linux architecture. import ( + "fmt" + "slices" "strings" "testing" ) @@ -263,3 +265,45 @@ func TestConditionString(t *testing.T) { } } } + +// TestBreakpointsInfoOrdered proves the listing is ordered by address: the +// numbers it prints are map keys rendered in iteration order otherwise, so +// the same set of breakpoints would renumber itself between calls. +func TestBreakpointsInfoOrdered(t *testing.T) { + tr := newMockTracer() + bm := NewBreakpoints(tr) + addrs := []uint64{0x9000, 0x1000, 0x7000, 0x3000, 0x8000, 0x2000, + 0x6000, 0x4000, 0x5000, 0xa000} + for i, a := range addrs { + if _, err := bm.Set(a, fmt.Sprintf("bp%d", i)); err != nil { + t.Fatalf("Set(%#x): %v", a, err) + } + } + sorted := append([]uint64(nil), addrs...) + slices.Sort(sorted) + info := bm.Info() + for i, a := range sorted { + want := fmt.Sprintf(" %d: bp%d at %#x", i+1, indexOf(addrs, a), a) + if !strings.Contains(info, want) { + t.Errorf("Info() missing %q; listing:\n%s", want, info) + } + } + // The numbers themselves must ascend: "1:" before "2" ... "10". + pos := 0 + for i := range len(addrs) { + next := strings.Index(info[pos:], fmt.Sprintf(" %d: ", i+1)) + if next < 0 { + t.Fatalf("Info() has no entry %d; listing:\n%s", i+1, info) + } + pos += next + } +} + +func indexOf(addrs []uint64, a uint64) int { + for i, v := range addrs { + if v == a { + return i + } + } + return -1 +} diff --git a/debug/debug_audit_test.go b/debug/debug_audit_test.go new file mode 100644 index 0000000..436d6f1 --- /dev/null +++ b/debug/debug_audit_test.go @@ -0,0 +1,325 @@ +// Copyright (c) 2026 Petr Balvín (https://petrbalvin.org) +// SPDX-License-Identifier: BSD-3-Clause + +//go:build linux && amd64 + +package debug + +import ( + "fmt" + "os" + "runtime" + "strings" + "testing" + "time" +) + +// Regression tests for the debugger audit: memory access at mapping +// boundaries, watchpoint slot attribution, launch failure latency, stray +// trap instructions and the REPL's argument validation. All drive a real +// ptrace session, so they run on amd64 hosts only. + +// memMap is one line of /proc/pid/maps. +type memMap struct { + lo, hi uint64 + perms string + name string +} + +// readMaps parses the debuggee's memory map. +func readMaps(t *testing.T, pid int) []memMap { + t.Helper() + data, err := os.ReadFile(fmt.Sprintf("/proc/%d/maps", pid)) + if err != nil { + t.Fatalf("read maps: %v", err) + } + var out []memMap + for line := range strings.SplitSeq(string(data), "\n") { + fields := strings.Fields(line) + if len(fields) < 2 { + continue + } + var lo, hi uint64 + if _, err := fmt.Sscanf(fields[0], "%x-%x", &lo, &hi); err != nil { + continue + } + m := memMap{lo: lo, hi: hi, perms: fields[1]} + if len(fields) >= 6 { + m.name = fields[5] + } + out = append(out, m) + } + return out +} + +// boundaryByte returns the last byte of a writable, ordinary mapping that is +// followed by an unmapped gap: an access there is inside the mapping, while +// the 8-byte word starting at it crosses into unmapped memory. +func boundaryByte(t *testing.T, pid int) uint64 { + t.Helper() + maps := readMaps(t, pid) + for i, m := range maps { + if !strings.Contains(m.perms, "rw") || + strings.Contains(m.name, "vvar") || strings.Contains(m.name, "vdso") || + strings.Contains(m.name, "vsyscall") { + continue + } + gap := uint64(1) << 62 + if i+1 < len(maps) { + gap = maps[i+1].lo - m.hi + } + if gap >= 4096 { + return m.hi - 1 + } + } + t.Skip("no writable mapping followed by a hole; cannot construct the boundary") + return 0 +} + +// TestReadMemoryPageBoundary proves ReadMemory never reads past the requested +// range: one byte at the end of a mapping followed by a hole must be +// readable, which the old word-at-a-time tail read failed because its final +// 8-byte Peek crossed into the unmapped page. +func TestReadMemoryPageBoundary(t *testing.T) { + sess, _, _ := launchKernel(t, buildGasm(t), boundaryKernel(t), "boundary", nil) + addr := boundaryByte(t, sess.Pid()) + mem, err := sess.ReadMemory(addr, 1) + if err != nil { + t.Fatalf("ReadMemory(%#x, 1): %v (the read must not cross into the unmapped page)", addr, err) + } + if len(mem) != 1 { + t.Fatalf("ReadMemory returned %d bytes, want 1", len(mem)) + } + // A request whose own range crosses into the hole must still fail. + if _, err := sess.ReadMemory(addr, 8); err == nil { + t.Fatal("ReadMemory past the mapping end should fail") + } +} + +// TestDisassemblePageBoundary proves the disassembler shrinks its read +// window at a mapping end instead of failing: the instruction stream cannot +// be decoded at all when the fixed 15-byte read crosses into the hole. +func TestDisassemblePageBoundary(t *testing.T) { + sess, _, _ := launchKernel(t, buildGasm(t), boundaryKernel(t), "boundary", nil) + addr := boundaryByte(t, sess.Pid()) + if _, _, err := sess.Disassemble(addr); err != nil { + t.Fatalf("Disassemble(%#x): %v (the read window must shrink at the mapping end)", addr, err) + } +} + +// TestWriteMemoryPageBoundary proves WriteMemory writes exactly the bytes it +// is given: one byte at the end of a mapping followed by a hole must be +// writable, which the old read-modify-write of the final partial word failed +// because its Peek crossed into the unmapped page. +func TestWriteMemoryPageBoundary(t *testing.T) { + sess, _, _ := launchKernel(t, buildGasm(t), boundaryKernel(t), "boundary", nil) + addr := boundaryByte(t, sess.Pid()) + orig, err := sess.ReadMemory(addr, 1) + if err != nil { + t.Fatalf("ReadMemory(%#x, 1): %v", addr, err) + } + if err := sess.WriteMemory(addr, []byte{orig[0]}); err != nil { + t.Fatalf("WriteMemory(%#x, 1): %v (the write must not read past the range)", addr, err) + } +} + +// TestWatchpointSlotAttribution proves a hit is attributed to the slot that +// fired, not to an earlier one whose DR6 status bit is still set: the B0-B3 +// bits are sticky, so they must be acknowledged when read. +func TestWatchpointSlotAttribution(t *testing.T) { + bin := buildGasm(t) + const kernel = `#include "textflag.h" + +// func wptwo(x, y int64) (a, b int64) +TEXT ·wptwo(SB), NOSPLIT, $0-32 + MOVQ $0x1111, AX + MOVQ AX, a+16(FP) + MOVQ $0x2222, BX + MOVQ BX, b+24(FP) + RET +` + path := writeKernel(t, kernel) + sess, bm, fl := launchKernel(t, bin, path, "wptwo", nil) + + entry := sess.CodeBase() + uint64(fl.Offset) + if _, err := bm.Set(entry, "entry"); err != nil { + t.Fatalf("Set: %v", err) + } + runToEntry(t, sess, bm, entry) + regs, err := sess.GetRegs() + if err != nil { + t.Fatalf("GetRegs: %v", err) + } + // FP sits one word above the entry stack pointer (the return address + // occupies [RSP]), so a+16(FP) = RSP+24 and b+24(FP) = RSP+32. + watchA := regs.RSP + 24 + watchB := regs.RSP + 32 + + if err := sess.SetWatchpoint(0, watchA, WatchWrite, 8); err != nil { + t.Fatalf("SetWatchpoint(0): %v", err) + } + if err := sess.SetWatchpoint(1, watchB, WatchWrite, 8); err != nil { + t.Fatalf("SetWatchpoint(1): %v", err) + } + + for i, want := range []uint64{watchA, watchB} { + if err := sess.Continue(); err != nil { + t.Fatalf("Continue (hit %d): %v", i+1, err) + } + reason, addr := sess.StopInfo() + if reason != StopWatchpoint { + t.Fatalf("hit %d: stop reason = %v, want StopWatchpoint", i+1, reason) + } + if addr != want { + t.Fatalf("hit %d reported %#x, want %#x (the sticky DR6 bit misattributes the slot)", i+1, addr, want) + } + } + + // Clearing a watchpoint must zero its address register: a stale + // address in a disabled slot turns any sticky status bit into a + // misattributed report later. + if err := sess.ClearWatchpoint(0); err != nil { + t.Fatalf("ClearWatchpoint(0): %v", err) + } + dr0, err := ptracePeekUser(sess.Pid(), drOffset) + if err != nil { + t.Fatalf("read DR0: %v", err) + } + if dr0 != 0 { + t.Fatalf("DR0 = %#x after ClearWatchpoint, want 0 (the address register must be cleared)", dr0) + } +} + +// TestLaunchFailsFastOnDeadDebuggee proves a debuggee that dies before +// signalling readiness surfaces promptly: the ready poll used to run its +// full 2.5 seconds before the wait discovered the exit. +func TestLaunchFailsFastOnDeadDebuggee(t *testing.T) { + runtime.LockOSThread() + defer runtime.UnlockOSThread() + bin := buildGasm(t) + path := boundaryKernel(t) + start := time.Now() + sess, err := Launch(bin, path, "nosuchfunction", nil) + elapsed := time.Since(start) + if err == nil { + sess.Kill() + t.Fatal("Launch with an unknown function should fail") + } + if !strings.Contains(err.Error(), "before signalling readiness") && + !strings.Contains(err.Error(), "debuggee exited") { + t.Errorf("error does not name the dead debuggee: %v", err) + } + if elapsed >= 1500*time.Millisecond { + t.Fatalf("Launch took %v to report the dead debuggee; the readiness poll must detect the exit, not time out", elapsed) + } +} + +// TestStrayTrapRunsThrough proves the continue loop survives a trap +// instruction planted in the kernel itself (BYTE $0xCC, the same byte the +// debugger patches in): on architectures that report the trap in place the +// loop must surface the stop, and on amd64 it runs through to the exit. A +// regression here hangs, so a watchdog fails the run. +func TestStrayTrapRunsThrough(t *testing.T) { + bin := buildGasm(t) + const kernel = `#include "textflag.h" + +// func stray() int64 +TEXT ·stray(SB), NOSPLIT, $0-8 + MOVQ $7, AX + BYTE $0xCC + MOVQ AX, ret+0(FP) + RET +` + path := writeKernel(t, kernel) + sess, bm, _ := launchKernel(t, bin, path, "stray", nil) + + timer := time.AfterFunc(time.Minute, func() { + panic("watchdog: the continue loop hung on the stray trap instruction") + }) + defer timer.Stop() + + out := captureStdout(t, func() { + REPL(sess, bm, sess.CodeBase(), 0, 0, 0, nil, nil, + strings.NewReader("continue\nquit\n")) + }) + if !strings.Contains(out, "debuggee exited") { + t.Errorf("the stray trap wedged the continue loop; output:\n%s", out) + } +} + +// TestStepIntoFaultReportsSignal proves the step command reports a genuine +// signal-delivery-stop instead of silently printing the faulting +// instruction as if the step had succeeded. +func TestStepIntoFaultReportsSignal(t *testing.T) { + bin := buildGasm(t) + const kernel = `#include "textflag.h" + +// func crash() int64 +TEXT ·crash(SB), NOSPLIT, $0-8 + XORQ AX, AX + MOVQ (AX), AX + MOVQ AX, ret+0(FP) + RET +` + path := writeKernel(t, kernel) + sess, bm, fl := launchKernel(t, bin, path, "crash", nil) + entry := sess.CodeBase() + uint64(fl.Offset) + if _, err := bm.Set(entry, "entry"); err != nil { + t.Fatalf("Set: %v", err) + } + runToEntry(t, sess, bm, entry) + + out := captureStdout(t, func() { + REPL(sess, bm, sess.CodeBase(), fl.Offset, fl.Size, fl.Args, nil, nil, + strings.NewReader("step 2\nquit\n")) + }) + if !strings.Contains(out, "stopped on signal") { + t.Errorf("stepping into the fault did not report the signal; output:\n%s", out) + } +} + +// TestREPLRejectsBadArguments proves the command loop reports malformed +// input instead of silently defaulting: an unknown label for x would read +// address 0, and a malformed count would silently step one instruction. +func TestREPLRejectsBadArguments(t *testing.T) { + bin := buildGasm(t) + path := boundaryKernel(t) + sess, bm, fl := launchKernel(t, bin, path, "boundary", nil) + entry := sess.CodeBase() + uint64(fl.Offset) + if _, err := bm.Set(entry, "entry"); err != nil { + t.Fatalf("Set: %v", err) + } + runToEntry(t, sess, bm, entry) + + out := captureStdout(t, func() { + REPL(sess, bm, sess.CodeBase(), fl.Offset, fl.Size, fl.Args, nil, nil, + strings.NewReader("x nosuchlabel\nstep abc\ndisas abc\nwatch 0x1000 q 8\nquit\n")) + }) + for _, want := range []string{ + "unknown address: nosuchlabel", + "invalid count: abc", + "unknown watchpoint type: q", + } { + if !strings.Contains(out, want) { + t.Errorf("output missing %q:\n%s", want, out) + } + } + if got := strings.Count(out, "invalid count: abc"); got != 2 { + t.Errorf("invalid count reported %d times, want 2 (step and disas):\n%s", got, out) + } +} + +// boundaryKernel is a minimal kernel for the boundary tests, which only need +// a live, stopped debuggee. +func boundaryKernel(t *testing.T) string { + t.Helper() + const kernel = `#include "textflag.h" + +// func boundary() int64 +TEXT ·boundary(SB), NOSPLIT, $0-8 + MOVQ $1, AX + MOVQ AX, ret+0(FP) + RET +` + return writeKernel(t, kernel) +} diff --git a/debug/disasm_freebsd_amd64.go b/debug/disasm_freebsd_amd64.go index e8df9e5..a9f314c 100644 --- a/debug/disasm_freebsd_amd64.go +++ b/debug/disasm_freebsd_amd64.go @@ -14,17 +14,26 @@ import ( ) // Disassemble decodes the instruction at the given address in the debuggee's -// memory and returns its text representation and length in bytes. +// memory and returns its text representation and length in bytes. An amd64 +// instruction is up to 15 bytes long, but the read must not reach past the +// end of the mapping: when the full 15-byte window crosses into unmapped +// memory the window shrinks, because an instruction at the mapping's end is +// by construction no longer than the readable bytes that hold it. func (s *Session) Disassemble(addr uint64) (string, int, error) { - mem, err := s.ReadMemory(addr, 15) - if err != nil { - return "", 0, err + var lastErr error + for _, n := range []int{15, 8, 4, 2, 1} { + mem, err := s.ReadMemory(addr, n) + if err != nil { + lastErr = err + continue + } + ins, derr := disasm.Decode(arch.AMD64, mem, addr) + if derr != nil { + return "", 0, derr + } + return ins.Text, ins.Len, nil } - ins, err := disasm.Decode(arch.AMD64, mem, addr) - if err != nil { - return "", 0, err - } - return ins.Text, ins.Len, nil + return "", 0, lastErr } // DisassembleN decodes up to n instructions starting at addr and returns diff --git a/debug/disasm_freebsd_riscv64.go b/debug/disasm_freebsd_riscv64.go index 0207101..7230f53 100644 --- a/debug/disasm_freebsd_riscv64.go +++ b/debug/disasm_freebsd_riscv64.go @@ -14,17 +14,25 @@ import ( ) // Disassemble decodes the instruction at the given address in the debuggee's -// memory and returns its text representation and length in bytes. +// memory and returns its text representation and length in bytes. The read +// shrinks from 4 to 2 bytes when the full word crosses into unmapped memory: +// a compressed instruction at the mapping's end still fits the shorter +// window, and an instruction can never extend past the mapping that holds it. func (s *Session) Disassemble(addr uint64) (string, int, error) { - mem, err := s.ReadMemory(addr, 4) - if err != nil { - return "", 0, err + var lastErr error + for _, n := range []int{4, 2} { + mem, err := s.ReadMemory(addr, n) + if err != nil { + lastErr = err + continue + } + ins, derr := disasm.Decode(arch.RISCV, mem, addr) + if derr != nil { + return "", 0, derr + } + return ins.Text, ins.Len, nil } - ins, err := disasm.Decode(arch.RISCV, mem, addr) - if err != nil { - return "", 0, err - } - return ins.Text, ins.Len, nil + return "", 0, lastErr } // DisassembleN decodes up to n instructions starting at addr and returns diff --git a/debug/disasm_linux_amd64.go b/debug/disasm_linux_amd64.go index aec0b8f..4a5693c 100644 --- a/debug/disasm_linux_amd64.go +++ b/debug/disasm_linux_amd64.go @@ -14,17 +14,26 @@ import ( ) // Disassemble decodes the instruction at the given address in the debuggee's -// memory and returns its text representation and length in bytes. +// memory and returns its text representation and length in bytes. An amd64 +// instruction is up to 15 bytes long, but the read must not reach past the +// end of the mapping: when the full 15-byte window crosses into unmapped +// memory the window shrinks, because an instruction at the mapping's end is +// by construction no longer than the readable bytes that hold it. func (s *Session) Disassemble(addr uint64) (string, int, error) { - mem, err := s.ReadMemory(addr, 15) - if err != nil { - return "", 0, err + var lastErr error + for _, n := range []int{15, 8, 4, 2, 1} { + mem, err := s.ReadMemory(addr, n) + if err != nil { + lastErr = err + continue + } + ins, derr := disasm.Decode(arch.AMD64, mem, addr) + if derr != nil { + return "", 0, derr + } + return ins.Text, ins.Len, nil } - ins, err := disasm.Decode(arch.AMD64, mem, addr) - if err != nil { - return "", 0, err - } - return ins.Text, ins.Len, nil + return "", 0, lastErr } // DisassembleN decodes up to n instructions starting at addr and returns diff --git a/debug/disasm_linux_riscv64.go b/debug/disasm_linux_riscv64.go index 39cf3d5..6a42b8b 100644 --- a/debug/disasm_linux_riscv64.go +++ b/debug/disasm_linux_riscv64.go @@ -14,17 +14,25 @@ import ( ) // Disassemble decodes the instruction at the given address in the debuggee's -// memory and returns its text representation and length in bytes. +// memory and returns its text representation and length in bytes. The read +// shrinks from 4 to 2 bytes when the full word crosses into unmapped memory: +// a compressed instruction at the mapping's end still fits the shorter +// window, and an instruction can never extend past the mapping that holds it. func (s *Session) Disassemble(addr uint64) (string, int, error) { - mem, err := s.ReadMemory(addr, 4) - if err != nil { - return "", 0, err + var lastErr error + for _, n := range []int{4, 2} { + mem, err := s.ReadMemory(addr, n) + if err != nil { + lastErr = err + continue + } + ins, derr := disasm.Decode(arch.RISCV, mem, addr) + if derr != nil { + return "", 0, derr + } + return ins.Text, ins.Len, nil } - ins, err := disasm.Decode(arch.RISCV, mem, addr) - if err != nil { - return "", 0, err - } - return ins.Text, ins.Len, nil + return "", 0, lastErr } // DisassembleN decodes up to n instructions starting at addr. diff --git a/debug/ptrace_freebsd.go b/debug/ptrace_freebsd.go index cd7dc93..32c5680 100644 --- a/debug/ptrace_freebsd.go +++ b/debug/ptrace_freebsd.go @@ -97,6 +97,16 @@ func LaunchWithBuffers(gasmBin, asmPath, funcName string, args []byte, bufSpec s if _, err := os.Stat(readyFile); err == nil { break } + // A debuggee that died before signalling readiness (unknown + // function, unparseable source) writes its failure notice to the + // handshake directory; read it and fail fast. The poll never + // waits on the child: a wait here could consume the SIGSTOP park + // that waitStopped below must receive, hanging the launch. + if err := s.deadReason(); err != nil { + cmd.Wait() + os.RemoveAll(tmpDir) + return nil, nil, err + } time.Sleep(5 * time.Millisecond) } @@ -134,6 +144,18 @@ func LaunchWithBuffers(gasmBin, asmPath, funcName string, args []byte, bufSpec s return s, bufAddrs, nil } +// deadReason reports the debuggee's own failure notice, the file its +// failure paths write before exiting. A debuggee killed without a notice +// (a crash, SIGKILL) surfaces through waitStopped after the poll instead, +// which is why the poll's budget stays finite. +func (s *Session) deadReason() error { + data, err := os.ReadFile(filepath.Join(s.tmpDir, "dead")) + if err != nil { + return nil + } + return fmt.Errorf("debug: debuggee failed before signalling readiness: %s", strings.TrimSpace(string(data))) +} + // waitStopped consumes ptrace-stop events until one the debugger cares // about arrives: SIGTRAP (a breakpoint or a completed single-step), the // debuggee's own SIGSTOP, or a genuine signal-delivery-stop. A Go tracee's diff --git a/debug/ptrace_linux.go b/debug/ptrace_linux.go index e7148be..3369823 100644 --- a/debug/ptrace_linux.go +++ b/debug/ptrace_linux.go @@ -91,6 +91,16 @@ func LaunchWithBuffers(gasmBin, asmPath, funcName string, args []byte, bufSpec s if _, err := os.Stat(readyFile); err == nil { break } + // A debuggee that died before signalling readiness (unknown + // function, unparseable source) writes its failure notice to the + // handshake directory; read it and fail fast. The poll never + // waits on the child: a wait here could consume the SIGSTOP park + // that waitStopped below must receive, hanging the launch. + if err := s.deadReason(); err != nil { + cmd.Wait() + os.RemoveAll(tmpDir) + return nil, nil, err + } time.Sleep(5 * time.Millisecond) } @@ -131,6 +141,18 @@ func LaunchWithBuffers(gasmBin, asmPath, funcName string, args []byte, bufSpec s return s, bufAddrs, nil } +// deadReason reports the debuggee's own failure notice, the file its +// failure paths write before exiting. A debuggee killed without a notice +// (a crash, SIGKILL) surfaces through waitStopped after the poll instead, +// which is why the poll's budget stays finite. +func (s *Session) deadReason() error { + data, err := os.ReadFile(filepath.Join(s.tmpDir, "dead")) + if err != nil { + return nil + } + return fmt.Errorf("debug: debuggee failed before signalling readiness: %s", strings.TrimSpace(string(data))) +} + // waitStopped consumes ptrace-stop events until one the debugger cares // about arrives: SIGTRAP (a breakpoint or a completed single-step), the // debuggee's own SIGSTOP, or a genuine signal-delivery-stop. A Go tracee's @@ -219,40 +241,41 @@ func (s *Session) Poke(addr, val uint64) error { return nil } -// ReadMemory reads len bytes from the debuggee's memory at addr. +// ReadMemory reads len bytes from the debuggee's memory at addr. The read +// covers exactly the requested range: the old word-at-a-time loop read a +// whole 8-byte word for the final partial word, so a request that ended +// inside the last mapped page failed whenever the following page was +// unmapped, even though every requested byte was readable. func (s *Session) ReadMemory(addr uint64, length int) ([]byte, error) { out := make([]byte, length) - for i := 0; i < length; i += 8 { - word, err := s.Peek(addr + uint64(i)) - if err != nil { - return out[:i], err - } - for j := 0; j < 8 && i+j < length; j++ { - out[i+j] = byte(word >> (8 * j)) - } + mem, err := os.OpenFile(fmt.Sprintf("/proc/%d/mem", s.pid), os.O_RDONLY, 0) + if err != nil { + return out, fmt.Errorf("debug: open /proc/%d/mem: %w", s.pid, err) + } + defer mem.Close() + n, err := mem.ReadAt(out, int64(addr)) + if err != nil { + return out[:n], fmt.Errorf("debug: read mem %#x: %w", addr, err) } return out, nil } -// WriteMemory writes bytes to the debuggee's memory at addr. +// WriteMemory writes bytes to the debuggee's memory at addr. The write +// covers exactly the given bytes: /proc/pid/mem accepts writes of any +// length at any offset, so the word loop's read-modify-write of the final +// partial word (which read past the requested range and failed on an +// unmapped following page) is unnecessary. func (s *Session) WriteMemory(addr uint64, data []byte) error { - for i := 0; i < len(data); i += 8 { - end := min(i+8, len(data)) - var word uint64 - for j := 0; j < end-i; j++ { - word |= uint64(data[i+j]) << (8 * j) - } - if end-i < 8 { - existing, err := s.Peek(addr + uint64(i)) - if err != nil { - return err - } - mask := ^((uint64(1) << (8 * (end - i))) - 1) - word = (existing & mask) | word - } - if err := s.Poke(addr+uint64(i), word); err != nil { - return err - } + if len(data) == 0 { + return nil + } + mem, err := os.OpenFile(fmt.Sprintf("/proc/%d/mem", s.pid), os.O_WRONLY, 0) + if err != nil { + return fmt.Errorf("debug: open /proc/%d/mem: %w", s.pid, err) + } + defer mem.Close() + if _, err := mem.WriteAt(data, int64(addr)); err != nil { + return fmt.Errorf("debug: write mem %#x: %w", addr, err) } return nil } diff --git a/debug/repl.go b/debug/repl.go index 05a3c6d..418c09a 100644 --- a/debug/repl.go +++ b/debug/repl.go @@ -71,7 +71,12 @@ func REPL(s *Session, bm *Breakpoints, codeBase uint64, funcOffset, funcSize, ar case "step", "s": n := 1 if len(parts) > 1 { - n, _ = strconv.Atoi(parts[1]) + v, err := strconv.Atoi(parts[1]) + if err != nil || v < 0 { + fmt.Printf("invalid count: %s\n", parts[1]) + continue + } + n = v } for range n { if s.Exited() { @@ -82,8 +87,13 @@ func REPL(s *Session, bm *Breakpoints, codeBase uint64, funcOffset, funcSize, ar fmt.Println(err) break } + if sig := s.LastSignal(); sig != 0 { + regs, _ := s.GetRegs() + fmt.Printf("stopped on signal %v at %#x\n", sig, regs.GetPC()) + break + } } - if !s.Exited() { + if !s.Exited() && s.LastSignal() == 0 { regs, _ := s.GetRegs() pc := regs.GetPC() text, _, _ := s.Disassemble(pc) @@ -106,16 +116,16 @@ func REPL(s *Session, bm *Breakpoints, codeBase uint64, funcOffset, funcSize, ar } if err := s.Continue(); err != nil { fmt.Println(err) - bm.Clear(afterAddr) + clearNextBp(bm, afterAddr) continue } if s.Exited() { - bm.Clear(afterAddr) + clearNextBp(bm, afterAddr) fmt.Println("debuggee exited") continue } if sig := s.LastSignal(); sig != 0 { - bm.Clear(afterAddr) + clearNextBp(bm, afterAddr) regs, _ := s.GetRegs() fmt.Printf("stopped on signal %v at %#x\n", sig, regs.GetPC()) continue @@ -125,12 +135,17 @@ func REPL(s *Session, bm *Breakpoints, codeBase uint64, funcOffset, funcSize, ar // snapshot, and a stale SetRegs would clobber live state. regs, _ = s.GetRegs() bm.HandleTrap(®s) - bm.Clear(afterAddr) + clearNextBp(bm, afterAddr) } else { if err := s.Step(); err != nil { fmt.Println(err) continue } + if sig := s.LastSignal(); sig != 0 { + regs, _ := s.GetRegs() + fmt.Printf("stopped on signal %v at %#x\n", sig, regs.GetPC()) + continue + } } if !s.Exited() { regs, _ := s.GetRegs() @@ -156,16 +171,16 @@ func REPL(s *Session, bm *Breakpoints, codeBase uint64, funcOffset, funcSize, ar } if err := s.Continue(); err != nil { fmt.Println(err) - bm.Clear(retAddr) + clearNextBp(bm, retAddr) continue } if s.Exited() { - bm.Clear(retAddr) + clearNextBp(bm, retAddr) fmt.Println("debuggee exited") continue } if sig := s.LastSignal(); sig != 0 { - bm.Clear(retAddr) + clearNextBp(bm, retAddr) regs, _ := s.GetRegs() fmt.Printf("stopped on signal %v at %#x\n", sig, regs.GetPC()) continue @@ -174,7 +189,7 @@ func REPL(s *Session, bm *Breakpoints, codeBase uint64, funcOffset, funcSize, ar // does: HandleTrap must see the PC the trap left behind. regs, _ = s.GetRegs() bm.HandleTrap(®s) - bm.Clear(retAddr) + clearNextBp(bm, retAddr) if s.Exited() { fmt.Println("debuggee exited") } else { @@ -214,6 +229,7 @@ func REPL(s *Session, bm *Breakpoints, codeBase uint64, funcOffset, funcSize, ar break } regs, _ := s.GetRegs() + trapPC := regs.GetPC() if bp := bm.HandleTrap(®s); bp != nil { // Execute the instruction under the restored breakpoint // so the next continue cannot re-trap on the same @@ -229,6 +245,13 @@ func REPL(s *Session, bm *Breakpoints, codeBase uint64, funcOffset, funcSize, ar fmt.Printf("breakpoint hit: %s (func+%#x)\n", name, bp.Addr-codeBase-uint64(funcOffset)) break } + // The trap matched no breakpoint of ours. When the PC still + // stands on the trapping instruction, resuming would re-execute + // it and trap forever, so surface the stop instead of spinning. + if TrapStray(s, reason, trapPC) { + fmt.Printf("SIGTRAP at %#x matches no breakpoint; the PC did not advance\n", trapPC-uint64(breakpointPCAdjust)) + break + } } case "break", "b": @@ -326,6 +349,10 @@ func REPL(s *Session, bm *Breakpoints, codeBase uint64, funcOffset, funcSize, ar length := 64 if len(parts) > 1 { addr, _ = resolveAddr(parts[1], codeBase, uint64(funcOffset), labels) + if addr == 0 { + fmt.Printf("unknown address: %s\n", parts[1]) + continue + } } if len(parts) > 2 { // A malformed or non-positive length would panic @@ -399,9 +426,13 @@ func REPL(s *Session, bm *Breakpoints, codeBase uint64, funcOffset, funcSize, ar case "disas", "u": n := 5 if len(parts) > 1 { - n, _ = strconv.Atoi(parts[1]) - if n <= 0 { - n = 5 + v, err := strconv.Atoi(parts[1]) + if err != nil { + fmt.Printf("invalid count: %s\n", parts[1]) + continue + } + if v > 0 { + n = v } } regs, _ := s.GetRegs() @@ -496,10 +527,18 @@ func REPL(s *Session, bm *Breakpoints, codeBase uint64, funcOffset, funcSize, ar typ = WatchRead case "w": typ = WatchWrite + default: + fmt.Printf("unknown watchpoint type: %s (want r or w)\n", parts[2]) + continue } } if len(parts) > 3 { - size, _ = strconv.Atoi(parts[3]) + v, err := strconv.Atoi(parts[3]) + if err != nil || v <= 0 { + fmt.Printf("invalid size: %s\n", parts[3]) + continue + } + size = v } slot := s.FindFreeWatchpointSlot() if slot < 0 { @@ -543,6 +582,15 @@ func REPL(s *Session, bm *Breakpoints, codeBase uint64, funcOffset, funcSize, ar s.Kill() } +// clearNextBp removes one of the temporary breakpoints the next and finish +// commands plant, reporting a failure instead of silently leaving the trap +// instruction behind in the debuggee. +func clearNextBp(bm *Breakpoints, addr uint64) { + if err := bm.Clear(addr); err != nil { + fmt.Printf("cannot remove temporary breakpoint at %#x: %v\n", addr, err) + } +} + func hexDump(addr uint64, data []byte) { for i := 0; i < len(data); i += 16 { end := min(i+16, len(data)) diff --git a/debug/target_freebsd.go b/debug/target_freebsd.go index 51e5c47..702ca3d 100644 --- a/debug/target_freebsd.go +++ b/debug/target_freebsd.go @@ -105,8 +105,19 @@ func fillBuffer(buf []byte, pattern string) { } } -// RunTarget is the debuggee entry point (gasm debug --target). +// RunTarget is the debuggee entry point (gasm debug --target). A failure +// is marked in the handshake directory before the process exits, so the +// debugger's readiness poll fails fast on a dead debuggee instead of +// waiting out its whole budget. func RunTarget(asmPath, funcName, argsFile, tmpDir string) error { + err := runTarget(asmPath, funcName, argsFile, tmpDir) + if err != nil { + markDead(tmpDir, err.Error()) + } + return err +} + +func runTarget(asmPath, funcName, argsFile, tmpDir string) error { src, err := os.ReadFile(asmPath) if err != nil { return fmt.Errorf("debug target: %w", err) diff --git a/debug/target_linux_amd64.go b/debug/target_linux_amd64.go index 0a86241..da14e95 100644 --- a/debug/target_linux_amd64.go +++ b/debug/target_linux_amd64.go @@ -17,8 +17,19 @@ import ( "sourcedock.dev/petrbalvin/gasm-sdk/verify" ) -// RunTarget is the debuggee entry point (gasm debug --target). +// RunTarget is the debuggee entry point (gasm debug --target). A failure +// is marked in the handshake directory before the process exits, so the +// debugger's readiness poll fails fast on a dead debuggee instead of +// waiting out its whole budget. func RunTarget(asmPath, funcName, argsFile, tmpDir string) error { + err := runTarget(asmPath, funcName, argsFile, tmpDir) + if err != nil { + markDead(tmpDir, err.Error()) + } + return err +} + +func runTarget(asmPath, funcName, argsFile, tmpDir string) error { src, err := os.ReadFile(asmPath) if err != nil { return fmt.Errorf("debug target: %w", err) diff --git a/debug/target_linux_arm64.go b/debug/target_linux_arm64.go index 97a4cbb..2be53e2 100644 --- a/debug/target_linux_arm64.go +++ b/debug/target_linux_arm64.go @@ -17,8 +17,19 @@ import ( "sourcedock.dev/petrbalvin/gasm-sdk/verify" ) -// RunTarget is the debuggee entry point (gasm debug --target). +// RunTarget is the debuggee entry point (gasm debug --target). A failure +// is marked in the handshake directory before the process exits, so the +// debugger's readiness poll fails fast on a dead debuggee instead of +// waiting out its whole budget. func RunTarget(asmPath, funcName, argsFile, tmpDir string) error { + err := runTarget(asmPath, funcName, argsFile, tmpDir) + if err != nil { + markDead(tmpDir, err.Error()) + } + return err +} + +func runTarget(asmPath, funcName, argsFile, tmpDir string) error { src, err := os.ReadFile(asmPath) if err != nil { return fmt.Errorf("debug target: %w", err) diff --git a/debug/target_linux_loong64.go b/debug/target_linux_loong64.go index 2a07e20..a044ead 100644 --- a/debug/target_linux_loong64.go +++ b/debug/target_linux_loong64.go @@ -17,8 +17,19 @@ import ( "sourcedock.dev/petrbalvin/gasm-sdk/verify" ) -// RunTarget is the debuggee entry point (gasm debug --target). +// RunTarget is the debuggee entry point (gasm debug --target). A failure +// is marked in the handshake directory before the process exits, so the +// debugger's readiness poll fails fast on a dead debuggee instead of +// waiting out its whole budget. func RunTarget(asmPath, funcName, argsFile, tmpDir string) error { + err := runTarget(asmPath, funcName, argsFile, tmpDir) + if err != nil { + markDead(tmpDir, err.Error()) + } + return err +} + +func runTarget(asmPath, funcName, argsFile, tmpDir string) error { src, err := os.ReadFile(asmPath) if err != nil { return fmt.Errorf("debug target: %w", err) diff --git a/debug/target_linux_riscv64.go b/debug/target_linux_riscv64.go index e2320c1..46cd728 100644 --- a/debug/target_linux_riscv64.go +++ b/debug/target_linux_riscv64.go @@ -17,8 +17,19 @@ import ( "sourcedock.dev/petrbalvin/gasm-sdk/verify" ) -// RunTarget is the debuggee entry point (gasm debug --target). +// RunTarget is the debuggee entry point (gasm debug --target). A failure +// is marked in the handshake directory before the process exits, so the +// debugger's readiness poll fails fast on a dead debuggee instead of +// waiting out its whole budget. func RunTarget(asmPath, funcName, argsFile, tmpDir string) error { + err := runTarget(asmPath, funcName, argsFile, tmpDir) + if err != nil { + markDead(tmpDir, err.Error()) + } + return err +} + +func runTarget(asmPath, funcName, argsFile, tmpDir string) error { src, err := os.ReadFile(asmPath) if err != nil { return fmt.Errorf("debug target: %w", err) diff --git a/debug/tracer.go b/debug/tracer.go index 096c39f..e617520 100644 --- a/debug/tracer.go +++ b/debug/tracer.go @@ -5,6 +5,22 @@ package debug +import ( + "os" + "path/filepath" +) + +// markDead records the target's failure reason in the handshake directory, +// the death notice the debugger's readiness poll reads. The poll must not +// wait on the child while it polls: a wait there could consume the SIGSTOP +// park the debugger's own waitStopped must receive, hanging the launch, so +// the file is the only fast death notice that is safe to read. A debuggee +// killed without a notice (a crash, SIGKILL) surfaces through the ordinary +// wait after the poll instead. +func markDead(tmpDir, reason string) { + os.WriteFile(filepath.Join(tmpDir, "dead"), []byte(reason), 0o644) +} + // tracer abstracts the minimal ptrace operations needed by the breakpoint // manager and the stop-information helpers. The live implementation is // *Session (ptrace_linux_amd64.go); tests supply a mock. diff --git a/debug/watchpoint_freebsd_amd64.go b/debug/watchpoint_freebsd_amd64.go index 0a997c0..ba7e398 100644 --- a/debug/watchpoint_freebsd_amd64.go +++ b/debug/watchpoint_freebsd_amd64.go @@ -75,17 +75,27 @@ func (s *Session) setDbRegs(dr *dbreg) error { // archStopTrace classifies a TRAP_TRACE stop. On amd64 the kernel // delivers both the completed single-step and the debug-register hit // through T_TRCTRAP with TRAP_TRACE (sys/amd64/amd64/trap.c), and DR6's -// B0-B3 bits name the watchpoint that fired. +// B0-B3 bits name the watchpoint that fired. The bits are sticky ("the +// processor never clears DR6", Intel SDM vol 3, "Debug Registers"), so +// they are acknowledged here: cleared once read, or the next hit on a +// different slot would still see this slot's bit set and report this +// slot's address again. func archStopTrace(s *Session, siAddr uint64) (StopReason, uint64) { dr, err := s.getDbRegs() if err != nil { return StopSingleStep, 0 } - if status := dr.Dr[drStatus]; status&0xF != 0 { - for slot := range 4 { - if status&(1<_EL1): +// bit i watches the address plus i, so the range spans the lowest set bit +// to the highest set bit inclusive. func archStopTrace(s *Session, siAddr uint64) (StopReason, uint64) { dr, err := s.getDbRegs() if err != nil { @@ -94,7 +98,14 @@ func archStopTrace(s *Session, siAddr uint64) (StopReason, uint64) { if ctrl&1 == 0 || dr.DbWatchregs[slot].Addr == 0 { continue } - if bas := (ctrl >> 5) & 0xFF; bas != 0 && siAddr >= dr.DbWatchregs[slot].Addr && siAddr < dr.DbWatchregs[slot].Addr+8 { + bas := uint8((ctrl >> 5) & 0xFF) + if bas == 0 { + continue + } + lo := bits.TrailingZeros8(bas) + hi := 7 - bits.LeadingZeros8(bas) + addr := dr.DbWatchregs[slot].Addr + if siAddr >= addr+uint64(lo) && siAddr < addr+uint64(hi)+1 { return StopWatchpoint, siAddr } } diff --git a/debug/watchpoint_linux_amd64.go b/debug/watchpoint_linux_amd64.go index d6fb4de..384d9ee 100644 --- a/debug/watchpoint_linux_amd64.go +++ b/debug/watchpoint_linux_amd64.go @@ -30,13 +30,25 @@ const ( // (arch/x86/kernel/ptrace.c send_sigtrap passes regs->ip), so the watched // data address is recovered from DR6's slot bits (B0-B3, positive polarity // through PEEKUSER) and the matching DR0-DR3. +// +// The B0-B3 bits are sticky ("the processor never clears DR6", Intel SDM +// vol 3, "Debug Registers"), so they are acknowledged here: cleared once +// read, or the next hit on a different slot would still see this slot's bit +// set and report this slot's address again. func archWatchpointAddr(s *Session, siAddr uint64) uint64 { dr6, err := ptracePeekUser(s.pid, dr6Off) if err != nil { return siAddr } + status := dr6 & 0xF + if status == 0 { + return siAddr + } + // Best effort: the write-back only fails for a debuggee that died, in + // which case no further watchpoint can fire anyway. + _ = ptracePokeUser(s.pid, dr6Off, dr6&^0xF) for slot := range 4 { - if dr6&(1<