PenaDevops/delve
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Miscellaneous fixes for Windows native backend (#2736)

Browse Source 
* proc/native: always stop after RequestManualStop on Windows

On Windows RequestManualStop will generate an exception on a special
DbgUiRemoteBreakin thread, sometimes this thread will die before we
finish stopping the process. We need to account for that and still stop
even if the thread is gone and no other thread hit a breakpoint.

Fixes flakiness of TestIssue419.

* proc/native: fix watchpoints with new threads on Windows

When a new thread is created we must reapply all watchpoints to it,
like we do on linux.

* tests: be lenient on goroutinestackprog tests on Windows

We can not guarantee that we find all goroutines stopped in a good
place and sometimes the stacktrace fails on Windows.
This commit is contained in:
Alessandro Arzilli 2021-10-13 17:43:54 +02:00 committed by GitHub
parent d1c888f22a
commit 1893c9769b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 99 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
_fixtures/databpeasy.go
View File

@ -3,6 +3,8 @@ package main
import (
"fmt"
"runtime"
"sync"
"time"
)
var globalvar1 = 0
@ -23,6 +25,12 @@ func main() { // Position 0
fmt.Printf("%d %d\n", globalvar1, globalvar2)
done := make(chan struct{}) // Position 4
var wg sync.WaitGroup
for i := 0; i < 20; i++ {
wg.Add(1)
go waitfunc(i, &wg)
}
wg.Wait()
go f(done)
<-done
}
@ -32,3 +40,9 @@ func f(done chan struct{}) {
globalvar1 = globalvar2 + 2
close(done) // Position 5
}
func waitfunc(i int, wg *sync.WaitGroup) {
runtime.LockOSThread()
wg.Done()
time.Sleep(50 * time.Second)
}

18
pkg/proc/native/proc_windows.go
View File

@ -222,6 +222,16 @@ func (dbp *nativeProcess) addThread(hThread syscall.Handle, threadID int, attach
return nil, err
}
}
for _, bp := range dbp.Breakpoints().M {
if bp.WatchType != 0 {
err := thread.writeHardwareBreakpoint(bp.Addr, bp.WatchType, bp.HWBreakIndex)
if err != nil {
return nil, err
}
}
}
return thread, nil
}
@ -489,6 +499,7 @@ func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error)
}
if !trapthreadFound {
wasDbgUiRemoteBreakIn := trapthread.os.dbgUiRemoteBreakIn
// trapthread exited during stop, pick another one
trapthread = nil
for _, thread := range dbp.threads {
@ -497,6 +508,13 @@ func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error)
break
}
}
if trapthread == nil && wasDbgUiRemoteBreakIn {
// If this was triggered by a manual stop request we should stop
// regardless, pick a thread.
for _, thread := range dbp.threads {
return thread, nil
}
}
}
return trapthread, nil

37
pkg/proc/proc_test.go
View File

@ -957,6 +957,11 @@ func TestStacktraceGoroutine(t *testing.T) {
{{10, "main.agoroutine"}},
}
lenient := 0
if runtime.GOOS == "windows" {
lenient = 1
}
protest.AllowRecording(t)
withTestProcess("goroutinestackprog", t, func(p *proc.Target, fixture protest.Fixture) {
bp := setFunctionBreakpoint(p, t, "main.stacktraceme")
@ -1006,7 +1011,7 @@ func TestStacktraceGoroutine(t *testing.T) {
t.Fatalf("Main goroutine stack not found %d", mainCount)
}
if agoroutineCount < 10 {
if agoroutineCount < 10-lenient {
t.Fatalf("Goroutine stacks not found (%d)", agoroutineCount)
}
@ -1266,6 +1271,10 @@ func TestVariableEvaluation(t *testing.T) {
func TestFrameEvaluation(t *testing.T) {
protest.AllowRecording(t)
lenient := false
if runtime.GOOS == "windows" {
lenient = true
}
withTestProcess("goroutinestackprog", t, func(p *proc.Target, fixture protest.Fixture) {
setFunctionBreakpoint(p, t, "main.stacktraceme")
assertNoError(p.Continue(), t, "Continue()")
@ -1312,7 +1321,11 @@ func TestFrameEvaluation(t *testing.T) {
for i := range found {
if !found[i] {
t.Fatalf("Goroutine %d not found\n", i)
if lenient {
lenient = false
} else {
t.Fatalf("Goroutine %d not found\n", i)
}
}
}
@ -5368,20 +5381,20 @@ func TestWatchpointsBasic(t *testing.T) {
skipOn(t, "not implemented", "linux", "arm64")
protest.AllowRecording(t)
position1 := 17
position5 := 33
position1 := 19
position5 := 41
if runtime.GOARCH == "arm64" {
position1 = 16
position5 = 32
position1 = 18
position5 = 40
}
withTestProcess("databpeasy", t, func(p *proc.Target, fixture protest.Fixture) {
setFunctionBreakpoint(p, t, "main.main")
setFileBreakpoint(p, t, fixture.Source, 19) // Position 2 breakpoint
setFileBreakpoint(p, t, fixture.Source, 25) // Position 4 breakpoint
setFileBreakpoint(p, t, fixture.Source, 21) // Position 2 breakpoint
setFileBreakpoint(p, t, fixture.Source, 27) // Position 4 breakpoint
assertNoError(p.Continue(), t, "Continue 0")
assertLineNumber(p, t, 11, "Continue 0") // Position 0
assertLineNumber(p, t, 13, "Continue 0") // Position 0
scope, err := proc.GoroutineScope(p, p.CurrentThread())
assertNoError(err, t, "GoroutineScope")
@ -5399,18 +5412,18 @@ func TestWatchpointsBasic(t *testing.T) {
p.ClearBreakpoint(bp.Addr)
assertNoError(p.Continue(), t, "Continue 2")
assertLineNumber(p, t, 19, "Continue 2") // Position 2
assertLineNumber(p, t, 21, "Continue 2") // Position 2
_, err = p.SetWatchpoint(scope, "globalvar1", proc.WatchWrite|proc.WatchRead, nil)
assertNoError(err, t, "SetDataBreakpoint(read-write)")
assertNoError(p.Continue(), t, "Continue 3")
assertLineNumber(p, t, 20, "Continue 3") // Position 3
assertLineNumber(p, t, 22, "Continue 3") // Position 3
p.ClearBreakpoint(bp.Addr)
assertNoError(p.Continue(), t, "Continue 4")
assertLineNumber(p, t, 25, "Continue 4") // Position 4
assertLineNumber(p, t, 27, "Continue 4") // Position 4
t.Logf("setting final breakpoint")
_, err = p.SetWatchpoint(scope, "globalvar1", proc.WatchWrite, nil)

23
pkg/terminal/command_test.go
View File

@ -331,6 +331,11 @@ func TestScopePrefix(t *testing.T) {
const goroutinesCurLinePrefix = "* Goroutine "
test.AllowRecording(t)
lenient := 0
if runtime.GOOS == "windows" {
lenient = 1
}
withTestTerminal("goroutinestackprog", t, func(term *FakeTerminal) {
term.MustExec("b stacktraceme")
term.MustExec("continue")
@ -383,7 +388,7 @@ func TestScopePrefix(t *testing.T) {
}
}
}
if len(agoroutines)+extraAgoroutines < 10 {
if len(agoroutines)+extraAgoroutines < 10-lenient {
t.Fatalf("Output of goroutines did not have 10 goroutines stopped on main.agoroutine (%d+%d found): %q", len(agoroutines), extraAgoroutines, goroutinesOut)
}
}
@ -429,7 +434,11 @@ func TestScopePrefix(t *testing.T) {
for i := range seen {
if !seen[i] {
t.Fatalf("goroutine %d not found", i)
if lenient > 0 {
lenient--
} else {
t.Fatalf("goroutine %d not found", i)
}
}
}
@ -474,6 +483,10 @@ func TestOnPrefix(t *testing.T) {
}
const prefix = "\ti: "
test.AllowRecording(t)
lenient := false
if runtime.GOOS == "windows" {
lenient = true
}
withTestTerminal("goroutinestackprog", t, func(term *FakeTerminal) {
term.MustExec("b agobp main.agoroutine")
term.MustExec("on agobp print i")
@ -507,7 +520,11 @@ func TestOnPrefix(t *testing.T) {
for i := range seen {
if !seen[i] {
t.Fatalf("Goroutine %d not seen\n", i)
if lenient {
lenient = false
} else {
t.Fatalf("Goroutine %d not seen\n", i)
}
}
}
})

12
service/test/integration1_test.go
View File

@ -719,6 +719,12 @@ func Test1ClientServer_FullStacktrace(t *testing.T) {
if runtime.GOOS == "darwin" && runtime.GOARCH == "arm64" {
t.Skip("cgo doesn't work on darwin/arm64")
}
lenient := false
if runtime.GOOS == "windows" {
lenient = true
}
withTestClient1("goroutinestackprog", t, func(c *rpc1.RPCClient) {
_, err := c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.stacktraceme", Line: -1})
assertNoError(err, t, "CreateBreakpoint()")
@ -757,7 +763,11 @@ func Test1ClientServer_FullStacktrace(t *testing.T) {
for i := range found {
if !found[i] {
t.Fatalf("Goroutine %d not found", i)
if lenient {
lenient = false
} else {
t.Fatalf("Goroutine %d not found", i)
}
}
}

12
service/test/integration2_test.go
View File

@ -1022,6 +1022,12 @@ func TestClientServer_FullStacktrace(t *testing.T) {
if runtime.GOOS == "darwin" && runtime.GOARCH == "arm64" {
t.Skip("cgo doesn't work on darwin/arm64")
}
lenient := false
if runtime.GOOS == "windows" {
lenient = true
}
withTestClient2("goroutinestackprog", t, func(c service.Client) {
_, err := c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.stacktraceme", Line: -1})
assertNoError(err, t, "CreateBreakpoint()")
@ -1060,7 +1066,11 @@ func TestClientServer_FullStacktrace(t *testing.T) {
for i := range found {
if !found[i] {
t.Fatalf("Goroutine %d not found", i)
if lenient {
lenient = false
} else {
t.Fatalf("Goroutine %d not found", i)
}
}
}