Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supports sending output to clients when running programs remotely #3253

Merged
merged 61 commits into from
Jul 5, 2023
Merged
Show file tree
Hide file tree
Changes from 47 commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
4a06698
wip: Support sending output when remote debug
tttoad Jan 17, 2023
6968a79
wip: Support local output and remote output
tttoad Jan 19, 2023
105696d
wip: fix stderr and stdout assignment error
tttoad Jan 19, 2023
444eeb1
wip: optimize code
tttoad Jan 20, 2023
3869956
wip: Only if outputMode is "remote" is the redirected console output
tttoad Jan 21, 2023
8a43523
wip: Redirected debugMode output(Not tested on windows)
tttoad Jan 31, 2023
4e47c69
wip: support remote debugging output redirection of windows
tttoad Jan 31, 2023
8350405
wip: real-time write back output
tttoad Feb 1, 2023
9c96fbe
wip: support for windows
tttoad Feb 2, 2023
c4b8ce2
wip: fix windows remote debug not output
tttoad Feb 2, 2023
a605ff9
wip: fix truncated output redirection
tttoad Feb 2, 2023
d56ac9f
Merge branch 'master' of github.com:tttoad/delve into feat-console
tttoad Feb 2, 2023
984e012
wip: delete printfln
tttoad Feb 3, 2023
63341e7
wip: use debugger.Config to pass redirect(macOS)
tttoad Feb 9, 2023
d5f5fb3
wip: use debugger.Config to pass redirect(linux)
tttoad Feb 11, 2023
4621f4c
wip: Change redirect to a concrete type
tttoad Feb 11, 2023
f433b23
wip: s.wg.wait before sending "terminated"
tttoad Feb 11, 2023
ffad8a4
wip: add proc/redirect test(darwin and linux)
tttoad Feb 17, 2023
a1467d6
Merge branch 'master' of github.com:tttoad/delve into feat-console
tttoad Feb 18, 2023
70216a5
Merge branch 'master' of github.com:tttoad/delve into feat-console
tttoad Feb 19, 2023
902235d
wip: Fix test failure on windows
tttoad Feb 20, 2023
9e08369
fix: undefined: proc.Redirects
tttoad Feb 20, 2023
0b3c066
fix: compile failure
tttoad Feb 20, 2023
38958bc
wip: Remove useless code
tttoad Feb 20, 2023
fc89882
fix: filename error
tttoad Feb 23, 2023
e6a0c78
Merge branch 'master' of github.com:tttoad/delve into feat-console
tttoad Feb 26, 2023
359ee2c
fix: os.file not close
tttoad Feb 26, 2023
ad0dec1
test: add server_test.redirect
tttoad Mar 1, 2023
73c7076
fix: Remove 'eol' from end of file
tttoad Mar 2, 2023
61092e2
fix: gdbserial: File not closed in file mode.
tttoad Mar 2, 2023
bdd13c9
feat: Remove "only-remote". Fix spelling mistakes.
tttoad Mar 3, 2023
f43b3d3
fix: spelling mistakes
tttoad Mar 3, 2023
28cea47
refactor: redirect
tttoad Mar 8, 2023
c0b0d07
fix: stdout and stderr are not set to default values
tttoad Mar 9, 2023
4617c1f
fix: Restore code logic for rr.openRedirects()
tttoad Mar 9, 2023
123de13
fix: Optimization Code
tttoad Mar 15, 2023
980d430
fix: utiltest
tttoad Mar 15, 2023
72c6718
fix: execpt out
tttoad Mar 15, 2023
f83ec15
fix: Resource release for redirects
tttoad Mar 18, 2023
31dba49
fix: build failure
tttoad Mar 18, 2023
8ebdb9d
fix: clean->clear
tttoad Mar 18, 2023
7d441c2
fix: build failure
tttoad Mar 19, 2023
dfa96ba
Merge branch 'master' of github.com:tttoad/delve into feat-console
tttoad Mar 19, 2023
ca76f9d
fix: test failure
tttoad Mar 19, 2023
36d0e92
fix: Optimization Code
Mar 20, 2023
6e33d4a
style: remove useless code
tttoad Mar 27, 2023
702341d
refactor: namedpipe
tttoad Apr 1, 2023
5aeb3f8
refactor: namedpipe, launch ...
tttoad Apr 4, 2023
10e0a66
fix: freebsd compile failure
tttoad Apr 4, 2023
1e3701b
fix: proc_darwin compile failure
tttoad Apr 4, 2023
4c8f86b
style: remove useless code
tttoad Apr 4, 2023
c016bc1
Merge branch 'master' of github.com:tttoad/delve into feat-console
tttoad Apr 5, 2023
a4f0491
feat: add d.config.Stdxx check on debug.Restart
tttoad Apr 8, 2023
0c2f3ad
style: formatting and adding comments
tttoad May 1, 2023
02d96ec
style: formatting and adding comments
tttoad May 1, 2023
44fa283
feat: add d.config.Stdxx check on debug.Restart
tttoad Apr 8, 2023
768fad8
style: namedpipe->redirector
tttoad May 12, 2023
f27bfcf
Merge branch 'feat-console' of github.com:tttoad/delve into feat-console
tttoad May 12, 2023
7dd6f49
style: namedPipe->redirector
tttoad May 12, 2023
7725eae
Merge branch 'master' into feat-console
tttoad Jun 13, 2023
38969ab
Merge branch 'master' of github.com:tttoad/delve into feat-console
tttoad Jun 28, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions _fixtures/out_redirect.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package main

import (
"fmt"
"os"
)

func main() {
fmt.Println("hello world!")
fmt.Fprintf(os.Stdout, "hello world!")
fmt.Fprintf(os.Stderr, "hello world!\n")
fmt.Fprintf(os.Stderr, "hello world! error!")
}
3 changes: 2 additions & 1 deletion cmd/dlv/cmds/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/go-delve/delve/pkg/gobuild"
"github.com/go-delve/delve/pkg/goversion"
"github.com/go-delve/delve/pkg/logflags"
"github.com/go-delve/delve/pkg/proc"
"github.com/go-delve/delve/pkg/terminal"
"github.com/go-delve/delve/pkg/version"
"github.com/go-delve/delve/service"
Expand Down Expand Up @@ -985,7 +986,7 @@ func execute(attachPid int, processArgs []string, conf *config.Config, coreFile
DebugInfoDirectories: conf.DebugInfoDirectories,
CheckGoVersion: checkGoVersion,
TTY: tty,
Redirects: redirects,
Redirect: proc.NewRedirectByPath(redirects),
DisableASLR: disableASLR,
RrOnProcessPid: rrOnProcessPid,
},
Expand Down
42 changes: 25 additions & 17 deletions pkg/proc/gdbserial/rr.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
// program. Returns a run function which will actually record the program, a
// stop function which will prematurely terminate the recording of the
// program.
func RecordAsync(cmd []string, wd string, quiet bool, redirects [3]string) (run func() (string, error), stop func() error, err error) {
func RecordAsync(cmd []string, wd string, quiet bool, redirects [3]proc.OutputRedirect) (run func() (string, error), stop func() error, err error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird that the stdin redirect is of type OutputRedirect, and that it can never be anything but a path. I'd say this should be changed to stdinRedirect string, stdoutRedirect, stderrRedirect proc.OutputRedirect. The stdin redirect isn't treated like the other two at all, at this point.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the struct name should be changed back to Redirect.I want to keep stdin passed in as *os.File.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? Nothing is using the *os.File version of stdin. Looking at the DAP specification I don't see anything that could use it, but maybe I'm missing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went through the Dap specification and didn't find definition of remote input event. Maybe there will be a specification to support remote input events.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think that this and other should be replaced by stdin string, stdout, stderr proc.OutputRedirect. They are not three of the same thing anymore, stdin is treated differently and there is no path for it becoming similar to the other two. Also stdin is definitely not an output.

if err := checkRRAvailable(); err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -63,11 +63,13 @@ func RecordAsync(cmd []string, wd string, quiet bool, redirects [3]string) (run
return run, stop, nil
}

func openRedirects(redirects [3]string, quiet bool) (stdin, stdout, stderr *os.File, closefn func(), err error) {
toclose := []*os.File{}
func openRedirects(redirects [3]proc.OutputRedirect, quiet bool) (stdin, stdout, stderr *os.File, closefn func(), err error) {
var (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this var () block.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

toclose = []*os.File{}
)

if redirects[0] != "" {
stdin, err = os.Open(redirects[0])
if redirects[0].Path != "" {
stdin, err = os.Open(redirects[0].Path)
if err != nil {
return nil, nil, nil, nil, err
}
Expand All @@ -76,19 +78,25 @@ func openRedirects(redirects [3]string, quiet bool) (stdin, stdout, stderr *os.F
stdin = os.Stdin
}

create := func(path string, dflt *os.File) *os.File {
if path == "" {
if quiet {
return nil
create := func(redirect proc.OutputRedirect, dflt *os.File) (f *os.File) {
if redirect.Path != "" {
f, err = os.Create(redirect.Path)
if f != nil {
toclose = append(toclose, f)
}
return dflt

return f
} else if redirect.File != nil {
toclose = append(toclose, redirect.File)

return redirect.File
}
var f *os.File
f, err = os.Create(path)
if f != nil {
toclose = append(toclose, f)

if quiet {
return nil
}
return f

return dflt
}

stdout = create(redirects[1], os.Stdout)
Expand All @@ -112,7 +120,7 @@ func openRedirects(redirects [3]string, quiet bool) (stdin, stdout, stderr *os.F

// Record uses rr to record the execution of the specified program and
// returns the trace directory's path.
func Record(cmd []string, wd string, quiet bool, redirects [3]string) (tracedir string, err error) {
func Record(cmd []string, wd string, quiet bool, redirects [3]proc.OutputRedirect) (tracedir string, err error) {
run, _, err := RecordAsync(cmd, wd, quiet, redirects)
if err != nil {
return "", err
Expand Down Expand Up @@ -288,7 +296,7 @@ func rrParseGdbCommand(line string) rrInit {
}

// RecordAndReplay acts like calling Record and then Replay.
func RecordAndReplay(cmd []string, wd string, quiet bool, debugInfoDirs []string, redirects [3]string) (*proc.TargetGroup, string, error) {
func RecordAndReplay(cmd []string, wd string, quiet bool, debugInfoDirs []string, redirects [3]proc.OutputRedirect) (*proc.TargetGroup, string, error) {
tracedir, err := Record(cmd, wd, quiet, redirects)
if tracedir == "" {
return nil, "", err
Expand Down
2 changes: 1 addition & 1 deletion pkg/proc/gdbserial/rr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func withTestRecording(name string, t testing.TB, fn func(grp *proc.TargetGroup,
t.Skip("test skipped, rr not found")
}
t.Log("recording")
grp, tracedir, err := gdbserial.RecordAndReplay([]string{fixture.Path}, ".", true, []string{}, [3]string{})
grp, tracedir, err := gdbserial.RecordAndReplay([]string{fixture.Path}, ".", true, []string{}, proc.NewEmptyRedirect())
if err != nil {
t.Fatal("Launch():", err)
}
Expand Down
84 changes: 84 additions & 0 deletions pkg/proc/namedpipe_other.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
//go:build !windows
derekparker marked this conversation as resolved.
Show resolved Hide resolved
// +build !windows

package proc

import (
"crypto/rand"
"encoding/hex"
"io"
"os"
"path/filepath"
"sync/atomic"
"syscall"
)

type openOnRead struct {
p string
rd io.ReadCloser
status int32 // 0: initialization 1: file is opened 2: file is closed
}

func (oor *openOnRead) Read(p []byte) (n int, err error) {
if oor.rd != nil {
return oor.rd.Read(p)
}

// try from "into" to "open"
if !atomic.CompareAndSwapInt32(&oor.status, 0, 1) {
return 0, io.EOF
}

fh, err := os.OpenFile(oor.p, os.O_RDONLY, os.ModeNamedPipe)
if err != nil {
return 0, err
}

oor.rd = fh
return oor.rd.Read(p)
}

func (oor *openOnRead) Close() error {
defer os.Remove(oor.p)

// try from "init" to "close".
if !atomic.CompareAndSwapInt32(&oor.status, 0, 2) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awful, there has to be a better way. At minimum it should use a mutex instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be enough to start the readers only after the target process has started.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can not work because rr.go (at minimum) would deadlock. However all we need to know is that the fifo has been opened for writing at least once, which we can do by unconditionally opening the fifo with os.O_WRONLY|syscall.O_NONBLOCK. This should work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.O_WRONLY|syscall.O_NONBLOCK can solve the blocking problem. But, until the pipe is opened in write mode, the read pipe will return null.

https://go.dev/play/p/hKYuUNNg868

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant this:

func (oor *openOnRead) Close() error {
	defer os.Remove(oor.p)
	fh, _ := os.OpenFile(oor.p, os.O_WRONLY|syscall.O_NONBLOCK, 0)
	if fh != nil {
		fh.Close()
	}
	return oor.rd.Close()
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

// try from "open" to "close"
if atomic.CompareAndSwapInt32(&oor.status, 1, 2) {
// Make the "oor.Read()" function stop blocking
_, err := os.OpenFile(oor.p, os.O_WRONLY, os.ModeNamedPipe)
if err != nil {
return err
}
}
}

return oor.rd.Close()
}

func NamedPipe() (reader [2]io.ReadCloser, output [3]OutputRedirect, err error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be NamedPipe() (io.ReadClsoer, OutputRedirect, error). It's called NamedPipe not TwoNamedPipes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

r := make([]byte, 4)
if _, err = rand.Read(r); err != nil {
return reader, output, err
}

var (
prefix = filepath.Join(os.TempDir(), hex.EncodeToString(r))
stdoutPath = prefix + "stdout"
stderrPath = prefix + "stderr"
)

if err = syscall.Mkfifo(stdoutPath, 0o600); err != nil {
return reader, output, err
}

if err := syscall.Mkfifo(stderrPath, 0o600); err != nil {
_ = os.Remove(stdoutPath)
return reader, output, err
}

reader[0] = &openOnRead{p: stdoutPath}
reader[1] = &openOnRead{p: stderrPath}

return reader, NewRedirectByPath([3]string{"", stdoutPath, stderrPath}), nil
}
23 changes: 23 additions & 0 deletions pkg/proc/namedpipe_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//go:build windows
// +build windows

package proc

import (
"io"
"os"
)

func NamedPipe() (reader [2]io.ReadCloser, output [3]OutputRedirect, err error) {
reader[0], output[1].File, err = os.Pipe()
if err != nil {
return reader, output, err
}

reader[1], output[2].File, err = os.Pipe()
if err != nil {
return reader, output, err
}

return reader, output, nil
}
2 changes: 1 addition & 1 deletion pkg/proc/native/nonative_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
var ErrNativeBackendDisabled = errors.New("native backend disabled during compilation")

// Launch returns ErrNativeBackendDisabled.
func Launch(_ []string, _ string, _ proc.LaunchFlags, _ []string, _ string, _ [3]string) (*proc.TargetGroup, error) {
func Launch(_ []string, _ string, _ proc.LaunchFlags, _ []string, _ string, _ [3]proc.OutputRedirect) (*proc.TargetGroup, error) {
return nil, ErrNativeBackendDisabled
}

Expand Down
33 changes: 20 additions & 13 deletions pkg/proc/native/proc.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,11 +370,13 @@ func (dbp *nativeProcess) writeSoftwareBreakpoint(thread *nativeThread, addr uin
return err
}

func openRedirects(redirects [3]string, foreground bool) (stdin, stdout, stderr *os.File, closefn func(), err error) {
toclose := []*os.File{}
func openRedirects(redirects [3]proc.OutputRedirect, foreground bool) (stdin, stdout, stderr *os.File, closefn func(), err error) {
var (
toclose = []*os.File{}
)

if redirects[0] != "" {
stdin, err = os.Open(redirects[0])
if redirects[0].Path != "" {
stdin, err = os.Open(redirects[0].Path)
if err != nil {
return nil, nil, nil, nil, err
}
Expand All @@ -383,16 +385,21 @@ func openRedirects(redirects [3]string, foreground bool) (stdin, stdout, stderr
stdin = os.Stdin
}

create := func(path string, dflt *os.File) *os.File {
if path == "" {
return dflt
}
var f *os.File
f, err = os.Create(path)
if f != nil {
toclose = append(toclose, f)
create := func(redirect proc.OutputRedirect, dflt *os.File) (f *os.File) {
if redirect.Path != "" {
f, err = os.Create(redirect.Path)
if f != nil {
toclose = append(toclose, f)
}

return f
} else if redirect.File != nil {
toclose = append(toclose, redirect.File)

return redirect.File
}
return f

return dflt
}

stdout = create(redirects[1], os.Stdout)
Expand Down
2 changes: 1 addition & 1 deletion pkg/proc/native/proc_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (os *osProcessDetails) Close() {}
// custom fork/exec process in order to take advantage of
// PT_SIGEXC on Darwin which will turn Unix signals into
// Mach exceptions.
func Launch(cmd []string, wd string, flags proc.LaunchFlags, _ []string, _ string, _ [3]string) (*proc.TargetGroup, error) {
func Launch(cmd []string, wd string, flags proc.LaunchFlags, _ []string, _ string, _ [3]proc.OutputRedirect) (*proc.TargetGroup, error) {
argv0Go, err := filepath.Abs(cmd[0])
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/proc/native/proc_freebsd.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (os *osProcessDetails) Close() {}
// to be supplied to that process. `wd` is working directory of the program.
// If the DWARF information cannot be found in the binary, Delve will look
// for external debug files in the directories passed in.
func Launch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []string, tty string, redirects [3]string) (*proc.TargetGroup, error) {
func Launch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []string, tty string, redirects [3]proc.OutputRedirect) (*proc.TargetGroup, error) {
var (
process *exec.Cmd
err error
Expand Down
4 changes: 2 additions & 2 deletions pkg/proc/native/proc_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,15 @@ func (os *osProcessDetails) Close() {
// to be supplied to that process. `wd` is working directory of the program.
// If the DWARF information cannot be found in the binary, Delve will look
// for external debug files in the directories passed in.
func Launch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []string, tty string, redirects [3]string) (*proc.TargetGroup, error) {
func Launch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []string, tty string, redirect [3]proc.OutputRedirect) (*proc.TargetGroup, error) {
var (
process *exec.Cmd
err error
)

foreground := flags&proc.LaunchForeground != 0

stdin, stdout, stderr, closefn, err := openRedirects(redirects, foreground)
stdin, stdout, stderr, closefn, err := openRedirects(redirect, foreground)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/proc/native/proc_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type osProcessDetails struct {
func (os *osProcessDetails) Close() {}

// Launch creates and begins debugging a new process.
func Launch(cmd []string, wd string, flags proc.LaunchFlags, _ []string, _ string, redirects [3]string) (*proc.TargetGroup, error) {
func Launch(cmd []string, wd string, flags proc.LaunchFlags, _ []string, _ string, redirects [3]proc.OutputRedirect) (*proc.TargetGroup, error) {
argv0Go := cmd[0]

env := proc.DisableAsyncPreemptEnv()
Expand Down
3 changes: 2 additions & 1 deletion pkg/proc/proc_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"path/filepath"
"testing"

"github.com/go-delve/delve/pkg/proc"
"github.com/go-delve/delve/pkg/proc/native"
protest "github.com/go-delve/delve/pkg/proc/test"
)
Expand All @@ -14,7 +15,7 @@ func TestLoadingExternalDebugInfo(t *testing.T) {
fixture := protest.BuildFixture("locationsprog", 0)
defer os.Remove(fixture.Path)
stripAndCopyDebugInfo(fixture, t)
p, err := native.Launch(append([]string{fixture.Path}, ""), "", 0, []string{filepath.Dir(fixture.Path)}, "", [3]string{})
p, err := native.Launch(append([]string{fixture.Path}, ""), "", 0, []string{filepath.Dir(fixture.Path)}, "", proc.NewEmptyRedirect())
if err != nil {
t.Fatal(err)
}
Expand Down
Loading