From 00df758d572f403d556e9da512043661715a0a28 Mon Sep 17 00:00:00 2001 From: Alessandro Arzilli Date: Tue, 20 Dec 2022 18:54:16 +0100 Subject: [PATCH] proc/native: fix FreeBSD backend (#3224) - use PT_SUSPEND/PT_RESUME to control running threads in resume/stop/singleStep - change manual stop signal from SIGTRAP to SIGSTOP to make manual stop handling simpler - change (*nativeProcess).trapWaitInternal to suspend newly created threads when we are stepping a thread - change (*nativeProcess).trapWaitInternal to handle some unhandled stop events - remove misleading (*nativeProcess).waitFast which does not do anything different from the normal wait variant - rewrite (*nativeProcess).stop to only set breakpoints for threads of which we have received SIGTRAP - rewrite (*nativeThread).singleStep to actually execute a single instruction and to properly route signals --- Documentation/backend_test_health.md | 3 +- cmd/dlv/dlv_test.go | 5 - pkg/proc/native/proc.go | 4 +- pkg/proc/native/proc_freebsd.go | 160 ++++++++++++++++++++++----- pkg/proc/native/ptrace_freebsd.go | 33 ++++++ pkg/proc/native/threads.go | 20 ---- pkg/proc/native/threads_darwin.go | 20 ++++ pkg/proc/native/threads_freebsd.go | 98 +++++++++------- pkg/proc/native/threads_linux.go | 20 ++++ pkg/proc/native/threads_windows.go | 16 --- pkg/proc/proc_test.go | 13 +-- pkg/terminal/command_test.go | 6 - service/dap/server_test.go | 27 +---- service/test/integration1_test.go | 3 - service/test/integration2_test.go | 3 - 15 files changed, 266 insertions(+), 165 deletions(-) diff --git a/Documentation/backend_test_health.md b/Documentation/backend_test_health.md index 6ee41512ea..4f16ef93bb 100644 --- a/Documentation/backend_test_health.md +++ b/Documentation/backend_test_health.md @@ -11,8 +11,7 @@ Tests skipped by each supported backend: * 1 broken - cgo stacktraces * darwin/lldb skipped = 1 * 1 upstream issue -* freebsd skipped = 16 - * 12 broken +* freebsd skipped = 4 * 4 not implemented * linux/386/pie skipped = 1 * 1 broken diff --git a/cmd/dlv/dlv_test.go b/cmd/dlv/dlv_test.go index cfec1439fd..753dd3835e 100644 --- a/cmd/dlv/dlv_test.go +++ b/cmd/dlv/dlv_test.go @@ -1036,11 +1036,6 @@ func TestTrace(t *testing.T) { } func TestTraceMultipleGoroutines(t *testing.T) { - if runtime.GOOS == "freebsd" { - //TODO(aarzilli): investigate further when the FreeBSD backend is more stable. - t.Skip("temporarily disabled due to issues with FreeBSD in Delve and Go") - } - dlvbin, tmpdir := getDlvBin(t) defer os.RemoveAll(tmpdir) diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go index 2a0bd5c8a4..b1c906a48e 100644 --- a/pkg/proc/native/proc.go +++ b/pkg/proc/native/proc.go @@ -246,14 +246,12 @@ func (dbp *nativeProcess) initialize(path string, debugInfoDirs []string) (*proc // We disable asyncpreempt for the following reasons: // - on Windows asyncpreempt is incompatible with debuggers, see: // https://github.com/golang/go/issues/36494 - // - freebsd's backend is generally broken and asyncpreempt makes it even more so, see: - // https://github.com/go-delve/delve/issues/1754 // - on linux/arm64 asyncpreempt can sometimes restart a sequence of // instructions, if the sequence happens to contain a breakpoint it will // look like the breakpoint was hit twice when it was "logically" only // executed once. // See: https://go-review.googlesource.com/c/go/+/208126 - DisableAsyncPreempt: runtime.GOOS == "windows" || runtime.GOOS == "freebsd" || (runtime.GOOS == "linux" && runtime.GOARCH == "arm64"), + DisableAsyncPreempt: runtime.GOOS == "windows" || (runtime.GOOS == "linux" && runtime.GOARCH == "arm64"), StopReason: stopReason, CanDump: runtime.GOOS == "linux" || (runtime.GOOS == "windows" && runtime.GOARCH == "amd64"), diff --git a/pkg/proc/native/proc_freebsd.go b/pkg/proc/native/proc_freebsd.go index f4fb6334c8..be659065f4 100644 --- a/pkg/proc/native/proc_freebsd.go +++ b/pkg/proc/native/proc_freebsd.go @@ -42,6 +42,10 @@ const ( type osProcessDetails struct { comm string tid int + + delayedSignal syscall.Signal + trapThreads []int + selectedThread *nativeThread } func (os *osProcessDetails) Close() {} @@ -83,7 +87,6 @@ func Launch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []str process.Stdout = stdout process.Stderr = stderr process.SysProcAttr = &syscall.SysProcAttr{Ptrace: true, Setpgid: true, Foreground: foreground} - process.Env = proc.DisableAsyncPreemptEnv() if foreground { signal.Ignore(syscall.SIGTTOU, syscall.SIGTTIN) } @@ -152,7 +155,12 @@ func (dbp *nativeProcess) kill() (err error) { if dbp.exited { return nil } - dbp.execPtraceFunc(func() { err = ptraceCont(dbp.pid, int(sys.SIGKILL)) }) + dbp.execPtraceFunc(func() { + for _, th := range dbp.threads { + ptraceResume(th.ID) + } + err = ptraceCont(dbp.pid, int(sys.SIGKILL)) + }) if err != nil { return err } @@ -165,7 +173,7 @@ func (dbp *nativeProcess) kill() (err error) { // Used by RequestManualStop func (dbp *nativeProcess) requestManualStop() (err error) { - return sys.Kill(dbp.pid, sys.SIGTRAP) + return sys.Kill(dbp.pid, sys.SIGSTOP) } // Attach to a newly created thread, and store that thread in our list of @@ -178,7 +186,7 @@ func (dbp *nativeProcess) addThread(tid int, attach bool) (*nativeThread, error) var err error dbp.execPtraceFunc(func() { err = sys.PtraceLwpEvents(dbp.pid, 1) }) if err == syscall.ESRCH { - if _, _, err = dbp.waitFast(dbp.pid); err != nil { + if _, _, err = dbp.wait(dbp.pid, 0); err != nil { return nil, fmt.Errorf("error while waiting after adding process: %d %s", dbp.pid, err) } } @@ -220,13 +228,29 @@ func findExecutable(path string, pid int) string { } func (dbp *nativeProcess) trapWait(pid int) (*nativeThread, error) { - return dbp.trapWaitInternal(pid, false) + return dbp.trapWaitInternal(pid, trapWaitNormal) } +type trapWaitMode uint8 + +const ( + trapWaitNormal trapWaitMode = iota + trapWaitStepping +) + // Used by stop and trapWait -func (dbp *nativeProcess) trapWaitInternal(pid int, halt bool) (*nativeThread, error) { +func (dbp *nativeProcess) trapWaitInternal(pid int, mode trapWaitMode) (*nativeThread, error) { + if dbp.os.selectedThread != nil { + th := dbp.os.selectedThread + dbp.os.selectedThread = nil + return th, nil + } for { wpid, status, err := dbp.wait(pid, 0) + if wpid != dbp.pid { + // possibly a delayed notification from a process we just detached and killed, freebsd bug? + continue + } if err != nil { return nil, fmt.Errorf("wait err %s %d", err, pid) } @@ -239,6 +263,11 @@ func (dbp *nativeProcess) trapWaitInternal(pid int, halt bool) (*nativeThread, e dbp.postExit() return nil, proc.ErrProcessExited{Pid: wpid, Status: status.ExitStatus()} } + if status.Signaled() { + // Killed by a signal + dbp.postExit() + return nil, proc.ErrProcessExited{Pid: wpid, Status: -int(status.Signal())} + } var info sys.PtraceLwpInfoStruct dbp.execPtraceFunc(func() { info, err = ptraceGetLwpInfo(wpid) }) @@ -269,10 +298,10 @@ func (dbp *nativeProcess) trapWaitInternal(pid int, halt bool) (*nativeThread, e } return nil, err } - if halt { - return nil, nil + if mode == trapWaitStepping { + dbp.execPtraceFunc(func() { ptraceSuspend(tid) }) } - if err = th.Continue(); err != nil { + if err = dbp.ptraceCont(0); err != nil { if err == sys.ESRCH { // thread died while we were adding it delete(dbp.threads, int(tid)) @@ -288,12 +317,16 @@ func (dbp *nativeProcess) trapWaitInternal(pid int, halt bool) (*nativeThread, e continue } - if (halt && status.StopSignal() == sys.SIGSTOP) || (status.StopSignal() == sys.SIGTRAP) { + if mode == trapWaitStepping { + return th, nil + } + if status.StopSignal() == sys.SIGTRAP || status.Continued() { + // Continued in this case means we received the SIGSTOP signal return th, nil } // TODO(dp) alert user about unexpected signals here. - if err := th.resumeWithSig(int(status.StopSignal())); err != nil { + if err := dbp.ptraceCont(int(status.StopSignal())); err != nil { if err == sys.ESRCH { return nil, proc.ErrProcessExited{Pid: dbp.pid} } @@ -309,14 +342,6 @@ func status(pid int) rune { return status } -// Used by stop and singleStep -// waitFast is like wait but does not handle process-exit correctly -func (dbp *nativeProcess) waitFast(pid int) (int, *sys.WaitStatus, error) { - var s sys.WaitStatus - wpid, err := sys.Wait4(pid, &s, 0, nil) - return wpid, &s, err -} - // Only used in this file func (dbp *nativeProcess) wait(pid, options int) (int, *sys.WaitStatus, error) { var s sys.WaitStatus @@ -330,7 +355,7 @@ func (dbp *nativeProcess) exitGuard(err error) error { return err } if status(dbp.pid) == statusZombie { - _, err := dbp.trapWaitInternal(-1, false) + _, err := dbp.trapWaitInternal(-1, trapWaitNormal) return err } @@ -348,9 +373,31 @@ func (dbp *nativeProcess) resume() error { thread.CurrentBreakpoint.Clear() } } + if len(dbp.os.trapThreads) > 0 { + // On these threads we have already received a SIGTRAP while stepping on a different thread, + // do not resume the process, instead record the breakpoint hits on them. + tt := dbp.os.trapThreads + dbp.os.trapThreads = dbp.os.trapThreads[:0] + var err error + dbp.os.selectedThread, err = dbp.postStop(tt...) + return err + } // all threads are resumed var err error - dbp.execPtraceFunc(func() { err = ptraceCont(dbp.pid, 0) }) + dbp.execPtraceFunc(func() { + for _, th := range dbp.threads { + err = ptraceResume(th.ID) + if err != nil { + return + } + } + sig := int(dbp.os.delayedSignal) + dbp.os.delayedSignal = 0 + for _, thread := range dbp.threads { + thread.Status = nil + } + err = ptraceCont(dbp.pid, sig) + }) return err } @@ -360,19 +407,72 @@ func (dbp *nativeProcess) stop(cctx *proc.ContinueOnceContext, trapthread *nativ if dbp.exited { return nil, proc.ErrProcessExited{Pid: dbp.pid} } - // set breakpoints on all threads - for _, th := range dbp.threads { - if th.CurrentBreakpoint.Breakpoint == nil { - if err := th.SetCurrentBreakpoint(true); err != nil { - return nil, err + + var err error + dbp.execPtraceFunc(func() { + for _, th := range dbp.threads { + err = ptraceSuspend(th.ID) + if err != nil { + return } } + }) + if err != nil { + return nil, err } - return trapthread, nil + + if trapthread.Status == nil || (*sys.WaitStatus)(trapthread.Status).StopSignal() != sys.SIGTRAP { + return trapthread, nil + } + + return dbp.postStop(trapthread.ID) +} + +func (dbp *nativeProcess) postStop(tids ...int) (*nativeThread, error) { + var pickedTrapThread *nativeThread + for _, tid := range tids { + trapthread := dbp.threads[tid] + if trapthread == nil { + continue + } + + // Must either be a hardcoded breakpoint, a currently set breakpoint or a + // phantom breakpoint hit caused by a SIGTRAP that was delayed. + // If someone sent a SIGTRAP directly to the process this will fail, but we + // can't do better. + + err := trapthread.SetCurrentBreakpoint(true) + if err != nil { + return nil, err + } + + if trapthread.CurrentBreakpoint.Breakpoint == nil { + // hardcoded breakpoint or phantom breakpoint hit + if dbp.BinInfo().Arch.BreakInstrMovesPC() { + pc, _ := trapthread.PC() + if !trapthread.atHardcodedBreakpoint(pc) { + // phantom breakpoint hit + _ = trapthread.setPC(pc - uint64(len(dbp.BinInfo().Arch.BreakpointInstruction()))) + trapthread = nil + } + } + } + + if pickedTrapThread == nil && trapthread != nil { + pickedTrapThread = trapthread + } + } + + return pickedTrapThread, nil } // Used by Detach func (dbp *nativeProcess) detach(kill bool) error { + if !kill { + for _, th := range dbp.threads { + ptraceResume(th.ID) + } + } return ptraceDetach(dbp.pid) } @@ -395,6 +495,12 @@ func (dbp *nativeProcess) GetBufferedTracepoints() []ebpf.RawUProbeParams { panic("not implemented") } +func (dbp *nativeProcess) ptraceCont(sig int) error { + var err error + dbp.execPtraceFunc(func() { err = ptraceCont(dbp.pid, sig) }) + return err +} + // Usedy by Detach func killProcess(pid int) error { return sys.Kill(pid, sys.SIGINT) diff --git a/pkg/proc/native/ptrace_freebsd.go b/pkg/proc/native/ptrace_freebsd.go index fe7c5c371a..93cc90c22b 100644 --- a/pkg/proc/native/ptrace_freebsd.go +++ b/pkg/proc/native/ptrace_freebsd.go @@ -10,6 +10,7 @@ import "C" import ( "unsafe" + "syscall" sys "golang.org/x/sys/unix" ) @@ -66,3 +67,35 @@ func ptraceReadData(id int, addr uintptr, data []byte) (n int, err error) { func ptraceWriteData(id int, addr uintptr, data []byte) (n int, err error) { return sys.PtraceIO(sys.PIOD_WRITE_D, id, addr, data, len(data)) } + +func ptraceSuspend(id int) error { + _, _, e1 := sys.Syscall6(sys.SYS_PTRACE, uintptr(C.PT_SUSPEND), uintptr(id), uintptr(1), uintptr(0), 0, 0) + if e1 != 0 { + return syscall.Errno(e1) + } + return nil +} + +func ptraceResume(id int) error { + _, _, e1 := sys.Syscall6(sys.SYS_PTRACE, uintptr(C.PT_RESUME), uintptr(id), uintptr(1), uintptr(0), 0, 0) + if e1 != 0 { + return syscall.Errno(e1) + } + return nil +} + +func ptraceSetStep(id int) error { + _, _, e1 := sys.Syscall6(sys.SYS_PTRACE, uintptr(C.PT_SETSTEP), uintptr(id), uintptr(0), uintptr(0), 0, 0) + if e1 != 0 { + return syscall.Errno(e1) + } + return nil +} + +func ptraceClearStep(id int) error { + _, _, e1 := sys.Syscall6(sys.SYS_PTRACE, uintptr(C.PT_CLEARSTEP), uintptr(id), uintptr(0), uintptr(0), 0, 0) + if e1 != 0 { + return syscall.Errno(e1) + } + return nil +} diff --git a/pkg/proc/native/threads.go b/pkg/proc/native/threads.go index 3088545e22..5cf570d600 100644 --- a/pkg/proc/native/threads.go +++ b/pkg/proc/native/threads.go @@ -22,26 +22,6 @@ type nativeThread struct { common proc.CommonThread } -// Continue the execution of this thread. -// -// If we are currently at a breakpoint, we'll clear it -// first and then resume execution. Thread will continue until -// it hits a breakpoint or is signaled. -func (t *nativeThread) Continue() error { - pc, err := t.PC() - if err != nil { - return err - } - // Check whether we are stopped at a breakpoint, and - // if so, single step over it before continuing. - if _, ok := t.dbp.FindBreakpoint(pc, false); ok { - if err := t.StepInstruction(); err != nil { - return err - } - } - return t.resume() -} - // StepInstruction steps a single instruction. // // Executes exactly one instruction and then returns. diff --git a/pkg/proc/native/threads_darwin.go b/pkg/proc/native/threads_darwin.go index d4df60395c..b338ff3737 100644 --- a/pkg/proc/native/threads_darwin.go +++ b/pkg/proc/native/threads_darwin.go @@ -143,3 +143,23 @@ func (t *nativeThread) withDebugRegisters(f func(*amd64util.DebugRegisters) erro func (t *nativeThread) SoftExc() bool { return false } + +// Continue the execution of this thread. +// +// If we are currently at a breakpoint, we'll clear it +// first and then resume execution. Thread will continue until +// it hits a breakpoint or is signaled. +func (t *nativeThread) Continue() error { + pc, err := t.PC() + if err != nil { + return err + } + // Check whether we are stopped at a breakpoint, and + // if so, single step over it before continuing. + if _, ok := t.dbp.FindBreakpoint(pc, false); ok { + if err := t.StepInstruction(); err != nil { + return err + } + } + return t.resume() +} diff --git a/pkg/proc/native/threads_freebsd.go b/pkg/proc/native/threads_freebsd.go index f9163014b1..241455be57 100644 --- a/pkg/proc/native/threads_freebsd.go +++ b/pkg/proc/native/threads_freebsd.go @@ -1,16 +1,13 @@ package native -// #include -import "C" - import ( - "fmt" - "github.com/go-delve/delve/pkg/proc/fbsdutil" + "bytes" sys "golang.org/x/sys/unix" "github.com/go-delve/delve/pkg/proc" "github.com/go-delve/delve/pkg/proc/amd64util" + "github.com/go-delve/delve/pkg/proc/fbsdutil" ) type waitStatus sys.WaitStatus @@ -20,59 +17,58 @@ type osSpecificDetails struct { registers sys.Reg } -func (t *nativeThread) stop() (err error) { - _, err = C.thr_kill2(C.pid_t(t.dbp.pid), C.long(t.ID), C.int(sys.SIGSTOP)) - if err != nil { - err = fmt.Errorf("stop err %s on thread %d", err, t.ID) - return - } - // If the process is stopped, we must continue it so it can receive the - // signal - t.dbp.execPtraceFunc(func() { err = ptraceCont(t.dbp.pid, 0) }) +func (t *nativeThread) singleStep() (err error) { + t.dbp.execPtraceFunc(func() { err = ptraceSetStep(t.ID) }) if err != nil { return err } - _, _, err = t.dbp.waitFast(t.dbp.pid) - if err != nil { - err = fmt.Errorf("wait err %s on thread %d", err, t.ID) - return - } - return -} - -func (t *nativeThread) Stopped() bool { - state := status(t.dbp.pid) - return state == statusStopped -} + defer func() { + t.dbp.execPtraceFunc(func() { ptraceClearStep(t.ID) }) + }() -func (t *nativeThread) resume() error { - return t.resumeWithSig(0) -} - -func (t *nativeThread) resumeWithSig(sig int) (err error) { - t.dbp.execPtraceFunc(func() { err = ptraceCont(t.ID, sig) }) - return -} - -func (t *nativeThread) singleStep() (err error) { - t.dbp.execPtraceFunc(func() { err = ptraceSingleStep(t.ID) }) + t.dbp.execPtraceFunc(func() { err = ptraceResume(t.ID) }) if err != nil { return err } + defer func() { + t.dbp.execPtraceFunc(func() { ptraceSuspend(t.ID) }) + }() + + sig := 0 for { - th, err := t.dbp.trapWait(t.dbp.pid) + err = t.dbp.ptraceCont(sig) + sig = 0 if err != nil { return err } - if th.ID == t.ID { - break - } - t.dbp.execPtraceFunc(func() { err = ptraceCont(th.ID, 0) }) + + trapthread, err := t.dbp.trapWaitInternal(-1, trapWaitStepping) if err != nil { return err } + + status := ((*sys.WaitStatus)(trapthread.Status)) + + if trapthread.ID == t.ID { + switch s := status.StopSignal(); s { + case sys.SIGTRAP: + return nil + case sys.SIGSTOP: + // delayed SIGSTOP, ignore it + case sys.SIGILL, sys.SIGBUS, sys.SIGFPE, sys.SIGSEGV: + // propagate signals that can be caused by current instruction + sig = int(s) + default: + t.dbp.os.delayedSignal = s + } + } else { + if status.StopSignal() == sys.SIGTRAP { + t.dbp.os.trapThreads = append(t.dbp.os.trapThreads, trapthread.ID) + } else { + t.dbp.os.delayedSignal = status.StopSignal() + } + } } - return nil } func (t *nativeThread) restoreRegisters(savedRegs proc.Registers) error { @@ -110,3 +106,19 @@ func (t *nativeThread) withDebugRegisters(f func(*amd64util.DebugRegisters) erro func (t *nativeThread) SoftExc() bool { return false } + +func (t *nativeThread) atHardcodedBreakpoint(pc uint64) bool { + for _, bpinstr := range [][]byte{ + t.dbp.BinInfo().Arch.BreakpointInstruction(), + t.dbp.BinInfo().Arch.AltBreakpointInstruction()} { + if bpinstr == nil { + continue + } + buf := make([]byte, len(bpinstr)) + _, _ = t.ReadMemory(buf, pc-uint64(len(buf))) + if bytes.Equal(buf, bpinstr) { + return true + } + } + return false +} diff --git a/pkg/proc/native/threads_linux.go b/pkg/proc/native/threads_linux.go index 1b9f707e9e..4c25cc7b4f 100644 --- a/pkg/proc/native/threads_linux.go +++ b/pkg/proc/native/threads_linux.go @@ -120,3 +120,23 @@ func (t *nativeThread) ReadMemory(data []byte, addr uint64) (n int, err error) { func (t *nativeThread) SoftExc() bool { return t.os.setbp } + +// Continue the execution of this thread. +// +// If we are currently at a breakpoint, we'll clear it +// first and then resume execution. Thread will continue until +// it hits a breakpoint or is signaled. +func (t *nativeThread) Continue() error { + pc, err := t.PC() + if err != nil { + return err + } + // Check whether we are stopped at a breakpoint, and + // if so, single step over it before continuing. + if _, ok := t.dbp.FindBreakpoint(pc, false); ok { + if err := t.StepInstruction(); err != nil { + return err + } + } + return t.resume() +} diff --git a/pkg/proc/native/threads_windows.go b/pkg/proc/native/threads_windows.go index ecd6dacd74..8bcd6f6192 100644 --- a/pkg/proc/native/threads_windows.go +++ b/pkg/proc/native/threads_windows.go @@ -107,22 +107,6 @@ func (t *nativeThread) singleStep() error { return t.setContext(context) } -func (t *nativeThread) resume() error { - var err error - t.dbp.execPtraceFunc(func() { - //TODO: Note that we are ignoring the thread we were asked to continue and are continuing the - //thread that we last broke on. - err = _ContinueDebugEvent(uint32(t.dbp.pid), uint32(t.ID), _DBG_CONTINUE) - }) - return err -} - -// Stopped returns whether the thread is stopped at the operating system -// level. On windows this always returns true. -func (t *nativeThread) Stopped() bool { - return true -} - func (t *nativeThread) WriteMemory(addr uint64, data []byte) (int, error) { if t.dbp.exited { return 0, proc.ErrProcessExited{Pid: t.dbp.pid} diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 2af4e67207..8ee2d8aed1 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -586,7 +586,6 @@ func TestNextGeneral(t *testing.T) { } func TestNextConcurrent(t *testing.T) { - skipOn(t, "broken", "freebsd") testcases := []nextTest{ {8, 9}, {9, 10}, @@ -622,7 +621,6 @@ func TestNextConcurrent(t *testing.T) { } func TestNextConcurrentVariant2(t *testing.T) { - skipOn(t, "broken", "freebsd") // Just like TestNextConcurrent but instead of removing the initial breakpoint we check that when it happens is for other goroutines testcases := []nextTest{ {8, 9}, @@ -1472,7 +1470,6 @@ func TestIssue325(t *testing.T) { } func TestBreakpointCounts(t *testing.T) { - skipOn(t, "broken", "freebsd") protest.AllowRecording(t) withTestProcess("bpcountstest", t, func(p *proc.Target, fixture protest.Fixture) { bp := setFileBreakpoint(p, t, fixture.Source, 12) @@ -1504,7 +1501,6 @@ func TestBreakpointCounts(t *testing.T) { } func TestHardcodedBreakpointCounts(t *testing.T) { - skipOn(t, "broken", "freebsd") withTestProcess("hcbpcountstest", t, func(p *proc.Target, fixture protest.Fixture) { counts := map[int64]int{} for { @@ -1716,7 +1712,6 @@ func BenchmarkLocalVariables(b *testing.B) { } func TestCondBreakpoint(t *testing.T) { - skipOn(t, "broken", "freebsd") protest.AllowRecording(t) withTestProcess("parallel_next", t, func(p *proc.Target, fixture protest.Fixture) { bp := setFileBreakpoint(p, t, fixture.Source, 9) @@ -1738,7 +1733,6 @@ func TestCondBreakpoint(t *testing.T) { } func TestCondBreakpointError(t *testing.T) { - skipOn(t, "broken", "freebsd") protest.AllowRecording(t) withTestProcess("parallel_next", t, func(p *proc.Target, fixture protest.Fixture) { bp := setFileBreakpoint(p, t, fixture.Source, 9) @@ -2101,7 +2095,6 @@ func TestIssue462(t *testing.T) { } func TestNextParked(t *testing.T) { - skipOn(t, "broken", "freebsd") protest.AllowRecording(t) withTestProcess("parallel_next", t, func(p *proc.Target, fixture protest.Fixture) { bp := setFunctionBreakpoint(p, t, "main.sayhi") @@ -2152,7 +2145,6 @@ func TestNextParked(t *testing.T) { } func TestStepParked(t *testing.T) { - skipOn(t, "broken", "freebsd") protest.AllowRecording(t) withTestProcess("parallel_next", t, func(p *proc.Target, fixture protest.Fixture) { bp := setFunctionBreakpoint(p, t, "main.sayhi") @@ -2481,7 +2473,6 @@ func TestStepOut(t *testing.T) { } func TestStepConcurrentDirect(t *testing.T) { - skipOn(t, "broken", "freebsd") skipOn(t, "broken - step concurrent", "windows", "arm64") protest.AllowRecording(t) withTestProcess("teststepconcurrent", t, func(p *proc.Target, fixture protest.Fixture) { @@ -2546,7 +2537,6 @@ func TestStepConcurrentDirect(t *testing.T) { } func TestStepConcurrentPtr(t *testing.T) { - skipOn(t, "broken", "freebsd") protest.AllowRecording(t) withTestProcess("teststepconcurrent", t, func(p *proc.Target, fixture protest.Fixture) { setFileBreakpoint(p, t, fixture.Source, 24) @@ -4633,7 +4623,6 @@ func testCallConcurrentCheckReturns(p *proc.Target, t *testing.T, gid1, gid2 int } func TestCallConcurrent(t *testing.T) { - skipOn(t, "broken", "freebsd") protest.MustSupportFunctionCalls(t, testBackend) withTestProcess("teststepconcurrent", t, func(p *proc.Target, fixture protest.Fixture) { grp := proc.NewGroup(p) @@ -5152,7 +5141,6 @@ func TestRequestManualStopWhileStopped(t *testing.T) { func TestStepOutPreservesGoroutine(t *testing.T) { // Checks that StepOut preserves the currently selected goroutine. - skipOn(t, "broken", "freebsd") rand.Seed(time.Now().Unix()) withTestProcess("issue2113", t, func(p *proc.Target, fixture protest.Fixture) { assertNoError(p.Continue(), t, "Continue()") @@ -5894,6 +5882,7 @@ func TestNilPtrDerefInBreakInstr(t *testing.T) { // this is also ok return } + t.Logf("third continue") assertNoError(err, t, "Continue()") bp := p.CurrentThread().Breakpoint() if bp != nil { diff --git a/pkg/terminal/command_test.go b/pkg/terminal/command_test.go index 706a83db08..e6b3c2cc23 100644 --- a/pkg/terminal/command_test.go +++ b/pkg/terminal/command_test.go @@ -456,9 +456,6 @@ func TestScopePrefix(t *testing.T) { } func TestOnPrefix(t *testing.T) { - if runtime.GOOS == "freebsd" { - t.Skip("test is not valid on FreeBSD") - } const prefix = "\ti: " test.AllowRecording(t) lenient := false @@ -520,9 +517,6 @@ func TestNoVars(t *testing.T) { } func TestOnPrefixLocals(t *testing.T) { - if runtime.GOOS == "freebsd" { - t.Skip("test is not valid on FreeBSD") - } const prefix = "\ti: " test.AllowRecording(t) withTestTerminal("goroutinestackprog", t, func(term *FakeTerminal) { diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 159392855d..e7469e06f3 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -485,9 +485,6 @@ func TestLaunchStopOnEntry(t *testing.T) { // TestAttachStopOnEntry is like TestLaunchStopOnEntry, but with attach request. func TestAttachStopOnEntry(t *testing.T) { - if runtime.GOOS == "freebsd" { - t.SkipNow() - } runTest(t, "loopprog", func(client *daptest.Client, fixture protest.Fixture) { // Start the program to attach to cmd := exec.Command(fixture.Path) @@ -3435,9 +3432,6 @@ func TestHaltPreventsAutoResume(t *testing.T) { // goroutine is hit the correct number of times and log points set in the // children goroutines produce the correct number of output events. func TestConcurrentBreakpointsLogPoints(t *testing.T) { - if runtime.GOOS == "freebsd" { - t.SkipNow() - } tests := []struct { name string fixture string @@ -3685,9 +3679,6 @@ func TestLaunchSubstitutePath(t *testing.T) { // that does not exist and expects the substitutePath attribute // in the launch configuration to take care of the mapping. func TestAttachSubstitutePath(t *testing.T) { - if runtime.GOOS == "freebsd" { - t.SkipNow() - } if runtime.GOOS == "windows" { t.Skip("test skipped on windows, see https://delve.beta.teamcity.com/project/Delve_windows for details") } @@ -4616,9 +4607,6 @@ func getPC(t *testing.T, client *daptest.Client, threadId int) (uint64, error) { // TestNextParked tests that we can switched selected goroutine to a parked one // and perform next operation on it. func TestNextParked(t *testing.T) { - if runtime.GOOS == "freebsd" { - t.SkipNow() - } runTest(t, "parallel_next", func(client *daptest.Client, fixture protest.Fixture) { runDebugSessionWithBPs(t, client, "launch", // Launch @@ -4681,7 +4669,8 @@ func testNextParkedHelper(t *testing.T, client *daptest.Client, fixture protest. // 2. hasn't called wg.Done yet // 3. is not the currently selected goroutine for _, g := range threads.Body.Threads { - if g.Id == se.Body.ThreadId { // Skip selected goroutine + if g.Id == se.Body.ThreadId || g.Id == 0 { + // Skip selected goroutine and goroutine 0 continue } client.StackTraceRequest(g.Id, 0, 5) @@ -4709,9 +4698,6 @@ func testNextParkedHelper(t *testing.T, client *daptest.Client, fixture protest. // and checks that StepOut preserves the currently selected goroutine. func TestStepOutPreservesGoroutine(t *testing.T) { // Checks that StepOut preserves the currently selected goroutine. - if runtime.GOOS == "freebsd" { - t.SkipNow() - } rand.Seed(time.Now().Unix()) runTest(t, "issue2113", func(client *daptest.Client, fixture protest.Fixture) { runDebugSessionWithBPs(t, client, "launch", @@ -4862,9 +4848,6 @@ func TestBadAccess(t *testing.T) { // again will produce an error with a helpful message, and 'continue' // will resume the program. func TestNextWhileNexting(t *testing.T) { - if runtime.GOOS == "freebsd" { - t.Skip("test is not valid on FreeBSD") - } // a breakpoint triggering during a 'next' operation will interrupt 'next'' // Unlike the test for the terminal package, we cannot be certain // of the number of breakpoints we expect to hit, since multiple @@ -5710,9 +5693,6 @@ func TestLaunchRequestWithEnv(t *testing.T) { } func TestAttachRequest(t *testing.T) { - if runtime.GOOS == "freebsd" { - t.SkipNow() - } if runtime.GOOS == "windows" { t.Skip("test skipped on windows, see https://delve.beta.teamcity.com/project/Delve_windows for details") } @@ -6638,9 +6618,6 @@ func TestAttachRemoteToDlvLaunchHaltedStopOnEntry(t *testing.T) { } func TestAttachRemoteToDlvAttachHaltedStopOnEntry(t *testing.T) { - if runtime.GOOS == "freebsd" || runtime.GOOS == "windows" { - t.SkipNow() - } cmd, dbg := attachDebuggerWithTargetHalted(t, "http_server") runTestWithDebugger(t, dbg, func(client *daptest.Client) { client.AttachRequest(map[string]interface{}{"mode": "remote", "stopOnEntry": true}) diff --git a/service/test/integration1_test.go b/service/test/integration1_test.go index a362ce5282..464a99736d 100644 --- a/service/test/integration1_test.go +++ b/service/test/integration1_test.go @@ -985,9 +985,6 @@ func Test1NegativeStackDepthBug(t *testing.T) { } func Test1ClientServer_CondBreakpoint(t *testing.T) { - if runtime.GOOS == "freebsd" { - t.Skip("test is not valid on FreeBSD") - } withTestClient1("parallel_next", t, func(c *rpc1.RPCClient) { bp, err := c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.sayhi", Line: 1}) assertNoError(err, t, "CreateBreakpoint()") diff --git a/service/test/integration2_test.go b/service/test/integration2_test.go index 69c81ef585..a66d8dd132 100644 --- a/service/test/integration2_test.go +++ b/service/test/integration2_test.go @@ -1496,9 +1496,6 @@ func TestNegativeStackDepthBug(t *testing.T) { } func TestClientServer_CondBreakpoint(t *testing.T) { - if runtime.GOOS == "freebsd" { - t.Skip("test is not valid on FreeBSD") - } protest.AllowRecording(t) withTestClient2("parallel_next", t, func(c service.Client) { bp, err := c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.sayhi", Line: 1})