From 55e99f5312d25f51b0e08571b2a15251316138fc Mon Sep 17 00:00:00 2001 From: aarzilli Date: Tue, 31 Oct 2023 14:59:52 +0100 Subject: [PATCH] proc/gdbserver: clean up rr directory on detach We used to autoremove the trace recorded by rr but as a result of various refactorings done to implement follow exec mode this broke. Restore the functionality. Also remove the _fixtures/testfnpos.go file which is autogenerated during testing. --- _fixtures/testfnpos.go | 16 ---------------- pkg/proc/core/core.go | 4 ++++ pkg/proc/gdbserial/gdbserver.go | 4 ++++ pkg/proc/interface.go | 1 + pkg/proc/native/proc.go | 4 ++++ pkg/proc/target_exec.go | 3 ++- pkg/proc/target_group.go | 2 +- service/debugger/debugger.go | 3 --- service/test/integration2_test.go | 3 ++- 9 files changed, 18 insertions(+), 22 deletions(-) delete mode 100644 _fixtures/testfnpos.go diff --git a/_fixtures/testfnpos.go b/_fixtures/testfnpos.go deleted file mode 100644 index 9572603fc8..0000000000 --- a/_fixtures/testfnpos.go +++ /dev/null @@ -1,16 +0,0 @@ -package main - -import "fmt" - -func f2() { - fmt.Printf("f2\n") -} - -func f1() { - fmt.Printf("f1\n") -} - -func main() { - f1() - f2() -} diff --git a/pkg/proc/core/core.go b/pkg/proc/core/core.go index dfab1b137f..3cb72e92d9 100644 --- a/pkg/proc/core/core.go +++ b/pkg/proc/core/core.go @@ -452,6 +452,10 @@ func (p *process) Detach(int, bool) error { return nil } +func (p *process) Close() error { + return nil +} + // Valid returns whether the process is active. Always returns true // for core files as it cannot exit or be otherwise detached from. func (p *process) Valid() (bool, error) { diff --git a/pkg/proc/gdbserial/gdbserver.go b/pkg/proc/gdbserial/gdbserver.go index 381a45577d..23fdbafd8f 100644 --- a/pkg/proc/gdbserial/gdbserver.go +++ b/pkg/proc/gdbserial/gdbserver.go @@ -1082,6 +1082,10 @@ func (p *gdbProcess) Detach(_pid int, kill bool) error { p.process = nil } p.detached = true + return nil +} + +func (p *gdbProcess) Close() error { if p.onDetach != nil { p.onDetach() } diff --git a/pkg/proc/interface.go b/pkg/proc/interface.go index 55536d0f01..4b17cd6833 100644 --- a/pkg/proc/interface.go +++ b/pkg/proc/interface.go @@ -13,6 +13,7 @@ type ProcessGroup interface { ContinueOnce(*ContinueOnceContext) (Thread, StopReason, error) StepInstruction(int) error Detach(int, bool) error + Close() error } // Process represents the target of the debugger. This diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go index c3086904b9..e209d9f81e 100644 --- a/pkg/proc/native/proc.go +++ b/pkg/proc/native/proc.go @@ -130,6 +130,10 @@ func (procgrp *processGroup) Detach(pid int, kill bool) (err error) { return } +func (procgrp *processGroup) Close() error { + return nil +} + // Valid returns whether the process is still attached to and // has not exited. func (dbp *nativeProcess) Valid() (bool, error) { diff --git a/pkg/proc/target_exec.go b/pkg/proc/target_exec.go index 29fc6e50c6..e54aaf2c63 100644 --- a/pkg/proc/target_exec.go +++ b/pkg/proc/target_exec.go @@ -1098,12 +1098,13 @@ func skipAutogeneratedWrappersOut(tgt *Target, g *G, thread Thread, startTopfram if err != nil { return } + bi := thread.BinInfo() for i := 1; i < len(frames); i++ { frame := frames[i] if frame.Current.Fn == nil { return } - file, line := g.Thread.BinInfo().EntryLineForFunc(frame.Current.Fn) + file, line := bi.EntryLineForFunc(frame.Current.Fn) if !isAutogeneratedOrDeferReturn(Location{File: file, Line: line, Fn: frame.Current.Fn}) { return &frames[i-1], &frames[i] } diff --git a/pkg/proc/target_group.go b/pkg/proc/target_group.go index b391986d12..1e64dbddae 100644 --- a/pkg/proc/target_group.go +++ b/pkg/proc/target_group.go @@ -189,7 +189,7 @@ func (grp *TargetGroup) Detach(kill bool) error { if len(errs) > 0 { return fmt.Errorf("%s", strings.Join(errs, "\n")) } - return nil + return grp.procgrp.Close() } // detachTarget will detach the target from the underlying process. diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index 229c032adb..a4f8ed36fd 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -459,9 +459,6 @@ func (d *Debugger) Detach(kill bool) error { d.log.Debug("detaching") d.targetMutex.Lock() defer d.targetMutex.Unlock() - if ok, _ := d.target.Valid(); !ok { - return nil - } return d.detach(kill) } diff --git a/service/test/integration2_test.go b/service/test/integration2_test.go index 7159529dd6..7ab9665232 100644 --- a/service/test/integration2_test.go +++ b/service/test/integration2_test.go @@ -2831,11 +2831,12 @@ func TestRestart_PreserveFunctionBreakpoint(t *testing.T) { // even if the function changed position in the source file. dir := protest.FindFixturesDir() + outpath := filepath.Join(dir, "testfnpos.go") + defer os.Remove(outpath) copy := func(inpath string) { buf, err := os.ReadFile(inpath) assertNoError(err, t, fmt.Sprintf("Reading %q", inpath)) - outpath := filepath.Join(dir, "testfnpos.go") assertNoError(os.WriteFile(outpath, buf, 0o666), t, fmt.Sprintf("Creating %q", outpath)) }