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

proc: fix bad cached goroutines after a call injection (#1926)

Browse Source 
Inserts a call to ClearAllGCache into stepInstructionOut so that cached
goroutine state is not inconsistent after an injected function call.\

Fixes #1925
This commit is contained in:
Alessandro Arzilli 2020-03-19 20:27:31 +01:00 committed by GitHub
parent c6de961be8
commit e1cfd72795
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 21 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
pkg/proc/proc.go
View File

@ -330,6 +330,7 @@ func disassembleCurrentInstruction(p Process, thread Thread) ([]AsmInstruction,
// This function is used to step out of runtime.Breakpoint as well as // This function is used to step out of runtime.Breakpoint as well as
// runtime.debugCallV1. // runtime.debugCallV1.
func stepInstructionOut(dbp *Target, curthread Thread, fnname1, fnname2 string) error { func stepInstructionOut(dbp *Target, curthread Thread, fnname1, fnname2 string) error {
defer dbp.ClearAllGCache()
for { for {
if err := curthread.StepInstruction(); err != nil { if err := curthread.StepInstruction(); err != nil {
return err return err

23
pkg/proc/proc_test.go
View File

@ -4174,9 +4174,6 @@ func TestReadDeferArgs(t *testing.T) {
} }
func TestIssue1374(t *testing.T) { func TestIssue1374(t *testing.T) {
if runtime.GOARCH == "arm64" || runtime.GOARCH == "386" {
t.Skip(fmt.Errorf("%s does not support FunctionCall for now", runtime.GOARCH))
}
// Continue did not work when stopped at a breakpoint immediately after calling CallFunction. // Continue did not work when stopped at a breakpoint immediately after calling CallFunction.
protest.MustSupportFunctionCalls(t, testBackend) protest.MustSupportFunctionCalls(t, testBackend)
withTestProcess("issue1374", t, func(p *proc.Target, fixture protest.Fixture) { withTestProcess("issue1374", t, func(p *proc.Target, fixture protest.Fixture) {
@ -4396,9 +4393,6 @@ func TestCallConcurrent(t *testing.T) {
if runtime.GOOS == "freebsd" { if runtime.GOOS == "freebsd" {
t.Skip("test is not valid on FreeBSD") t.Skip("test is not valid on FreeBSD")
} }
if runtime.GOARCH == "arm64" || runtime.GOARCH == "386" {
t.Skip(fmt.Sprintf("%s does not support FunctionCall for now", runtime.GOARCH))
}
protest.MustSupportFunctionCalls(t, testBackend) protest.MustSupportFunctionCalls(t, testBackend)
withTestProcess("teststepconcurrent", t, func(p *proc.Target, fixture protest.Fixture) { withTestProcess("teststepconcurrent", t, func(p *proc.Target, fixture protest.Fixture) {
@ -4748,3 +4742,20 @@ func TestBackwardNextDeferPanic(t *testing.T) {
{contReverseNext, 28}, {contReverseNext, 28},
}) })
} }
func TestIssue1925(t *testing.T) {
// Calling a function should not leave cached goroutine information in an
// inconsistent state.
// In particular the stepInstructionOut function called at the end of a
// 'call' procedure should clean the G cache like every other function
// altering the state of the target process.
protest.MustSupportFunctionCalls(t, testBackend)
withTestProcess("testvariables2", t, func(p *proc.Target, fixture protest.Fixture) {
assertNoError(proc.Continue(p), t, "Continue()")
assertNoError(proc.EvalExpressionWithCalls(p, p.SelectedGoroutine(), "afunc(2)", normalLoadConfig, true), t, "Call")
t.Logf("%v\n", p.SelectedGoroutine().CurrentLoc)
if loc := p.SelectedGoroutine().CurrentLoc; loc.File != fixture.Source {
t.Errorf("wrong location for selected goroutine after call: %s:%d", loc.File, loc.Line)
}
})
}

3
pkg/proc/test/support.go
View File

@ -315,6 +315,9 @@ func MustSupportFunctionCalls(t *testing.T, testBackend string) {
if runtime.GOOS == "darwin" && os.Getenv("TRAVIS") == "true" { if runtime.GOOS == "darwin" && os.Getenv("TRAVIS") == "true" {
t.Skip("function call injection tests are failing on macOS on Travis-CI (see #1802)") t.Skip("function call injection tests are failing on macOS on Travis-CI (see #1802)")
} }
if runtime.GOARCH == "arm64" || runtime.GOARCH == "386" {
t.Skip(fmt.Errorf("%s does not support FunctionCall for now", runtime.GOARCH))
}
} }
// DefaultTestBackend changes the value of testBackend to be the default // DefaultTestBackend changes the value of testBackend to be the default