From 4a066980cf8834a58a2b2238eb95cd713d4c0f0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Tue, 17 Jan 2023 17:34:14 +0800 Subject: [PATCH 01/53] wip: Support sending output when remote debug --- service/dap/server.go | 41 +++++++++++++++++++++++++++++++++++++++-- service/dap/types.go | 1 + vendor/modules.txt | 2 -- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/service/dap/server.go b/service/dap/server.go index 3c953ddf72..1ada833bfa 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -159,6 +159,12 @@ type Session struct { // changeStateMu must be held for a request to protect itself from another goroutine // changing the state of the running process at the same time. changeStateMu sync.Mutex + + // + stdout io.ReadWriter + + // + stderr io.ReadWriter } // Config is all the information needed to start the debugger, handle @@ -974,7 +980,7 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { var cmd string var out []byte - var err error + switch args.Mode { case "debug": cmd, out, err = gobuild.GoBuildCombinedOutput(args.Output, []string{args.Program}, args.BuildFlags) @@ -1025,6 +1031,11 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { argsToLog.Cwd, _ = filepath.Abs(args.Cwd) s.config.log.Debugf("launching binary '%s' with config: %s", debugbinary, prettyPrint(argsToLog)) + if args.OutputModel == "remote" { + s.stderr = bytes.NewBufferString("") + s.stdout = bytes.NewBufferString("") + } + if args.NoDebug { s.mu.Lock() cmd, err := s.newNoDebugProcess(debugbinary, args.Args, s.config.Debugger.WorkingDir) @@ -1039,6 +1050,32 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { // Start the program on a different goroutine, so we can listen for disconnect request. go func() { + // TODO: Support remote stop program, if the program can not be stopped + if args.OutputModel == "remote" { + wg := &sync.WaitGroup{} + readerFunc := func(reader io.Reader, category string, wg *sync.WaitGroup) { + // Display one line back after outputting one line + var scanner = bufio.NewScanner(reader) + for scanner.Scan() { + s.send(&dap.OutputEvent{ + Event: *newEvent("output"), + Body: dap.OutputEventBody{ + Output: fmt.Sprintln(scanner.Text()), + Category: category, + }}) + } + + wg.Done() + } + + wg.Add(1) + go readerFunc(s.stdout, "stdout", wg) + wg.Add(1) + go readerFunc(s.stderr, "stderr", wg) + // Wait for the input and output to be read + defer wg.Wait() + } + if err := cmd.Wait(); err != nil { s.config.log.Debugf("program exited with error: %v", err) } @@ -1087,7 +1124,7 @@ func (s *Session) newNoDebugProcess(program string, targetArgs []string, wd stri return nil, fmt.Errorf("another launch request is in progress") } cmd := exec.Command(program, targetArgs...) - cmd.Stdout, cmd.Stderr, cmd.Stdin, cmd.Dir = os.Stdout, os.Stderr, os.Stdin, wd + cmd.Stdout, cmd.Stderr, cmd.Stdin, cmd.Dir = s.stdout, s.stderr, os.Stdin, wd if err := cmd.Start(); err != nil { return nil, err } diff --git a/service/dap/types.go b/service/dap/types.go index f67a03f66e..125f8ceda5 100644 --- a/service/dap/types.go +++ b/service/dap/types.go @@ -148,6 +148,7 @@ type LaunchConfig struct { // reference to other environment variables is not supported. Env map[string]*string `json:"env,omitempty"` + OutputModel string LaunchAttachCommonConfig } diff --git a/vendor/modules.txt b/vendor/modules.txt index ac1d2a3ab4..1dd56b7867 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -91,8 +91,6 @@ golang.org/x/tools/internal/gocommand golang.org/x/tools/internal/packagesinternal golang.org/x/tools/internal/typeparams golang.org/x/tools/internal/typesinternal -# golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 -## explicit # gopkg.in/yaml.v2 v2.4.0 ## explicit gopkg.in/yaml.v2 From 6968a79b0c29d537257b967b0dc6ecaec68606b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Thu, 19 Jan 2023 16:06:03 +0800 Subject: [PATCH 02/53] wip: Support local output and remote output --- service/dap/server.go | 84 +++++++++++++++++++++++++++++-------------- service/dap/types.go | 2 +- 2 files changed, 59 insertions(+), 27 deletions(-) diff --git a/service/dap/server.go b/service/dap/server.go index 1ada833bfa..0dd177197d 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -160,13 +160,25 @@ type Session struct { // changing the state of the running process at the same time. changeStateMu sync.Mutex - // - stdout io.ReadWriter + // outputModel specifies how to print the program's output. + outputModel outputModel - // - stderr io.ReadWriter + // stdoutReader the programs's stdout. + stdoutReader io.ReadCloser + + // stderrReader the program's stderr. + stderrReader io.ReadCloser } +type outputModel int8 + +const ( + // osStdMask os.Stdin and os.Stdout + osStdMask outputModel = 0x0001 + // remoteMask Sending program output to the client. + remoteMask outputModel = 0x0010 +) + // Config is all the information needed to start the debugger, handle // DAP connection traffic and signal to the server when it is time to stop. type Config struct { @@ -1031,9 +1043,9 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { argsToLog.Cwd, _ = filepath.Abs(args.Cwd) s.config.log.Debugf("launching binary '%s' with config: %s", debugbinary, prettyPrint(argsToLog)) + s.outputModel |= osStdMask if args.OutputModel == "remote" { - s.stderr = bytes.NewBufferString("") - s.stdout = bytes.NewBufferString("") + s.outputModel |= remoteMask } if args.NoDebug { @@ -1051,34 +1063,43 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { // Start the program on a different goroutine, so we can listen for disconnect request. go func() { // TODO: Support remote stop program, if the program can not be stopped - if args.OutputModel == "remote" { - wg := &sync.WaitGroup{} - readerFunc := func(reader io.Reader, category string, wg *sync.WaitGroup) { - // Display one line back after outputting one line - var scanner = bufio.NewScanner(reader) - for scanner.Scan() { + wg := &sync.WaitGroup{} + readerFunc := func(reader io.Reader, category string, wg *sync.WaitGroup) { + // Display one line back after outputting one line + var scanner = bufio.NewScanner(reader) + for scanner.Scan() { + out := scanner.Text() + if s.outputModel&0x0001 != 0 { + if category == "stdout" { + fmt.Fprintln(os.Stdout, out) + } else { + fmt.Fprintln(os.Stderr, out) + } + } + + if s.outputModel&0x0010 != 0 { s.send(&dap.OutputEvent{ Event: *newEvent("output"), Body: dap.OutputEventBody{ - Output: fmt.Sprintln(scanner.Text()), + Output: fmt.Sprintln(out), Category: category, }}) } - - wg.Done() } - - wg.Add(1) - go readerFunc(s.stdout, "stdout", wg) - wg.Add(1) - go readerFunc(s.stderr, "stderr", wg) - // Wait for the input and output to be read - defer wg.Wait() + wg.Done() } + wg.Add(1) + go readerFunc(s.stdoutReader, "stdout", wg) + wg.Add(1) + go readerFunc(s.stderrReader, "stderr", wg) + // Wait for the input and output to be read + if err := cmd.Wait(); err != nil { s.config.log.Debugf("program exited with error: %v", err) } + + wg.Wait() close(s.noDebugProcess.exited) s.logToConsole(proc.ErrProcessExited{Pid: cmd.ProcessState.Pid(), Status: cmd.ProcessState.ExitCode()}.Error()) s.send(&dap.TerminatedEvent{Event: *newEvent("terminated")}) @@ -1119,15 +1140,26 @@ func (s *Session) getPackageDir(pkg string) string { // newNoDebugProcess is called from onLaunchRequest (run goroutine) and // requires holding mu lock. It prepares process exec.Cmd to be started. -func (s *Session) newNoDebugProcess(program string, targetArgs []string, wd string) (*exec.Cmd, error) { +func (s *Session) newNoDebugProcess(program string, targetArgs []string, wd string) (cmd *exec.Cmd, err error) { if s.noDebugProcess != nil { return nil, fmt.Errorf("another launch request is in progress") } - cmd := exec.Command(program, targetArgs...) - cmd.Stdout, cmd.Stderr, cmd.Stdin, cmd.Dir = s.stdout, s.stderr, os.Stdin, wd - if err := cmd.Start(); err != nil { + + cmd = exec.Command(program, targetArgs...) + cmd.Stdin, cmd.Dir = os.Stdin, wd + + if s.stderrReader, err = cmd.StdoutPipe(); err != nil { return nil, err } + + if s.stdoutReader, err = cmd.StderrPipe(); err != nil { + return nil, err + } + + if err = cmd.Start(); err != nil { + return nil, err + } + s.noDebugProcess = &process{Cmd: cmd, exited: make(chan struct{})} return cmd, nil } diff --git a/service/dap/types.go b/service/dap/types.go index 125f8ceda5..ebdca1a2df 100644 --- a/service/dap/types.go +++ b/service/dap/types.go @@ -148,7 +148,7 @@ type LaunchConfig struct { // reference to other environment variables is not supported. Env map[string]*string `json:"env,omitempty"` - OutputModel string + OutputModel string `json:"outputModel,omitempty"` LaunchAttachCommonConfig } From 105696d0599f232961af6862c83e92b13476292f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Thu, 19 Jan 2023 16:49:07 +0800 Subject: [PATCH 03/53] wip: fix stderr and stdout assignment error --- service/dap/server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/service/dap/server.go b/service/dap/server.go index 0dd177197d..3745e58f43 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -1148,11 +1148,11 @@ func (s *Session) newNoDebugProcess(program string, targetArgs []string, wd stri cmd = exec.Command(program, targetArgs...) cmd.Stdin, cmd.Dir = os.Stdin, wd - if s.stderrReader, err = cmd.StdoutPipe(); err != nil { + if s.stderrReader, err = cmd.StderrPipe(); err != nil { return nil, err } - if s.stdoutReader, err = cmd.StderrPipe(); err != nil { + if s.stdoutReader, err = cmd.StdoutPipe(); err != nil { return nil, err } From 444eeb13aefd9026c899312c6a58f804043ff9cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Sat, 21 Jan 2023 01:38:51 +0800 Subject: [PATCH 04/53] wip: optimize code --- service/dap/server.go | 43 ++++++++++++++++++++++++++----------------- service/dap/types.go | 2 +- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/service/dap/server.go b/service/dap/server.go index 3745e58f43..a48723bae8 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -160,8 +160,8 @@ type Session struct { // changing the state of the running process at the same time. changeStateMu sync.Mutex - // outputModel specifies how to print the program's output. - outputModel outputModel + // outputMode specifies how to print the program's output. + outputMode outputMode // stdoutReader the programs's stdout. stdoutReader io.ReadCloser @@ -170,13 +170,13 @@ type Session struct { stderrReader io.ReadCloser } -type outputModel int8 +type outputMode int8 const ( - // osStdMask os.Stdin and os.Stdout - osStdMask outputModel = 0x0001 - // remoteMask Sending program output to the client. - remoteMask outputModel = 0x0010 + // outputToStd os.Stdin and os.Stdout + outputToStd outputMode = 1 << iota + // outputToDAP Sending program output to the client. + outputToDAP ) // Config is all the information needed to start the debugger, handle @@ -1043,9 +1043,15 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { argsToLog.Cwd, _ = filepath.Abs(args.Cwd) s.config.log.Debugf("launching binary '%s' with config: %s", debugbinary, prettyPrint(argsToLog)) - s.outputModel |= osStdMask - if args.OutputModel == "remote" { - s.outputModel |= remoteMask + s.outputMode |= outputToStd + switch args.OutputMode { + case "remote": + s.outputMode |= outputToDAP + case "local", "": + // noting + default: + s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", + fmt.Sprintf("invalid debug configuration - unsupported 'outputMode' attribute %q", args.OutputMode)) } if args.NoDebug { @@ -1067,17 +1073,20 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { readerFunc := func(reader io.Reader, category string, wg *sync.WaitGroup) { // Display one line back after outputting one line var scanner = bufio.NewScanner(reader) + var stdWriter io.Writer + if category == "stdout" { + stdWriter = os.Stdout + } else { + stdWriter = os.Stderr + } + for scanner.Scan() { out := scanner.Text() - if s.outputModel&0x0001 != 0 { - if category == "stdout" { - fmt.Fprintln(os.Stdout, out) - } else { - fmt.Fprintln(os.Stderr, out) - } + if s.outputMode&outputToStd != 0 { + fmt.Fprintln(stdWriter, out) } - if s.outputModel&0x0010 != 0 { + if s.outputMode&outputToDAP != 0 { s.send(&dap.OutputEvent{ Event: *newEvent("output"), Body: dap.OutputEventBody{ diff --git a/service/dap/types.go b/service/dap/types.go index ebdca1a2df..09b3825348 100644 --- a/service/dap/types.go +++ b/service/dap/types.go @@ -148,7 +148,7 @@ type LaunchConfig struct { // reference to other environment variables is not supported. Env map[string]*string `json:"env,omitempty"` - OutputModel string `json:"outputModel,omitempty"` + OutputMode string `json:"outputMode,omitempty"` LaunchAttachCommonConfig } From 38699560c32b0a61b0674c5c20c96889ec8433e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Sun, 22 Jan 2023 01:23:49 +0800 Subject: [PATCH 05/53] wip: Only if outputMode is "remote" is the redirected console output --- service/dap/server.go | 79 +++++++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 36 deletions(-) diff --git a/service/dap/server.go b/service/dap/server.go index a48723bae8..7dbf9f43f7 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -1043,10 +1043,12 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { argsToLog.Cwd, _ = filepath.Abs(args.Cwd) s.config.log.Debugf("launching binary '%s' with config: %s", debugbinary, prettyPrint(argsToLog)) + var redirected = false s.outputMode |= outputToStd switch args.OutputMode { case "remote": s.outputMode |= outputToDAP + redirected = true case "local", "": // noting default: @@ -1056,7 +1058,7 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { if args.NoDebug { s.mu.Lock() - cmd, err := s.newNoDebugProcess(debugbinary, args.Args, s.config.Debugger.WorkingDir) + cmd, err := s.newNoDebugProcess(debugbinary, args.Args, s.config.Debugger.WorkingDir, redirected) s.mu.Unlock() if err != nil { s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", err.Error()) @@ -1070,40 +1072,41 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { go func() { // TODO: Support remote stop program, if the program can not be stopped wg := &sync.WaitGroup{} - readerFunc := func(reader io.Reader, category string, wg *sync.WaitGroup) { - // Display one line back after outputting one line - var scanner = bufio.NewScanner(reader) - var stdWriter io.Writer - if category == "stdout" { - stdWriter = os.Stdout - } else { - stdWriter = os.Stderr - } - - for scanner.Scan() { - out := scanner.Text() - if s.outputMode&outputToStd != 0 { - fmt.Fprintln(stdWriter, out) + if redirected { + readerFunc := func(reader io.Reader, category string, wg *sync.WaitGroup) { + // Display one line back after outputting one line + var scanner = bufio.NewScanner(reader) + var stdWriter io.Writer + if category == "stdout" { + stdWriter = os.Stdout + } else { + stdWriter = os.Stderr } - if s.outputMode&outputToDAP != 0 { - s.send(&dap.OutputEvent{ - Event: *newEvent("output"), - Body: dap.OutputEventBody{ - Output: fmt.Sprintln(out), - Category: category, - }}) + for scanner.Scan() { + out := scanner.Text() + if s.outputMode&outputToStd != 0 { + fmt.Fprintln(stdWriter, out) + } + + if s.outputMode&outputToDAP != 0 { + s.send(&dap.OutputEvent{ + Event: *newEvent("output"), + Body: dap.OutputEventBody{ + Output: fmt.Sprintln(out), + Category: category, + }}) + } } + wg.Done() } - wg.Done() - } - - wg.Add(1) - go readerFunc(s.stdoutReader, "stdout", wg) - wg.Add(1) - go readerFunc(s.stderrReader, "stderr", wg) - // Wait for the input and output to be read + wg.Add(1) + go readerFunc(s.stdoutReader, "stdout", wg) + wg.Add(1) + go readerFunc(s.stderrReader, "stderr", wg) + // Wait for the input and output to be read + } if err := cmd.Wait(); err != nil { s.config.log.Debugf("program exited with error: %v", err) } @@ -1149,7 +1152,7 @@ func (s *Session) getPackageDir(pkg string) string { // newNoDebugProcess is called from onLaunchRequest (run goroutine) and // requires holding mu lock. It prepares process exec.Cmd to be started. -func (s *Session) newNoDebugProcess(program string, targetArgs []string, wd string) (cmd *exec.Cmd, err error) { +func (s *Session) newNoDebugProcess(program string, targetArgs []string, wd string, redirected bool) (cmd *exec.Cmd, err error) { if s.noDebugProcess != nil { return nil, fmt.Errorf("another launch request is in progress") } @@ -1157,12 +1160,16 @@ func (s *Session) newNoDebugProcess(program string, targetArgs []string, wd stri cmd = exec.Command(program, targetArgs...) cmd.Stdin, cmd.Dir = os.Stdin, wd - if s.stderrReader, err = cmd.StderrPipe(); err != nil { - return nil, err - } + if redirected { + if s.stderrReader, err = cmd.StderrPipe(); err != nil { + return nil, err + } - if s.stdoutReader, err = cmd.StdoutPipe(); err != nil { - return nil, err + if s.stdoutReader, err = cmd.StdoutPipe(); err != nil { + return nil, err + } + } else { + cmd.Stdout, cmd.Stderr = os.Stdin, os.Stderr } if err = cmd.Start(); err != nil { From 8a435239082bf82e08c3ee5d5cd7785aff9cd105 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Tue, 31 Jan 2023 11:08:15 +0800 Subject: [PATCH 06/53] wip: Redirected debugMode output(Not tested on windows) --- service/dap/server.go | 125 ++++++++++++++++++++++++++++++++---------- 1 file changed, 96 insertions(+), 29 deletions(-) diff --git a/service/dap/server.go b/service/dap/server.go index 7dbf9f43f7..23507c13c9 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -11,12 +11,15 @@ package dap import ( "bufio" "bytes" + "crypto/rand" + "encoding/hex" "encoding/json" "errors" "fmt" "go/constant" "go/parser" "io" + "log" "math" "net" "os" @@ -30,6 +33,7 @@ import ( "strconv" "strings" "sync" + "syscall" "time" "github.com/go-delve/delve/pkg/gobuild" @@ -1056,6 +1060,39 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { fmt.Sprintf("invalid debug configuration - unsupported 'outputMode' attribute %q", args.OutputMode)) } + readerFunc := func(reader io.Reader, category string) { + // Display one line back after outputting one line + var ( + scanner = bufio.NewScanner(reader) + stdWriter io.Writer + ) + + if category == "stdout" { + stdWriter = os.Stdout + } else { + stdWriter = os.Stderr + } + + fmt.Println(category, "scan") + for scanner.Scan() { + out := scanner.Text() + if s.outputMode&outputToStd != 0 { + fmt.Fprintln(stdWriter, out) + } + + if s.outputMode&outputToDAP != 0 { + s.send(&dap.OutputEvent{ + Event: *newEvent("output"), + Body: dap.OutputEventBody{ + Output: fmt.Sprintln(out), + Category: category, + }}) + } + } + + fmt.Println(category, "done") + } + if args.NoDebug { s.mu.Lock() cmd, err := s.newNoDebugProcess(debugbinary, args.Args, s.config.Debugger.WorkingDir, redirected) @@ -1073,38 +1110,17 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { // TODO: Support remote stop program, if the program can not be stopped wg := &sync.WaitGroup{} if redirected { - readerFunc := func(reader io.Reader, category string, wg *sync.WaitGroup) { - // Display one line back after outputting one line - var scanner = bufio.NewScanner(reader) - var stdWriter io.Writer - if category == "stdout" { - stdWriter = os.Stdout - } else { - stdWriter = os.Stderr - } - - for scanner.Scan() { - out := scanner.Text() - if s.outputMode&outputToStd != 0 { - fmt.Fprintln(stdWriter, out) - } - - if s.outputMode&outputToDAP != 0 { - s.send(&dap.OutputEvent{ - Event: *newEvent("output"), - Body: dap.OutputEventBody{ - Output: fmt.Sprintln(out), - Category: category, - }}) - } - } + wg.Add(1) + go func() { + readerFunc(s.stdoutReader, "stdout") wg.Done() - } + }() wg.Add(1) - go readerFunc(s.stdoutReader, "stdout", wg) - wg.Add(1) - go readerFunc(s.stderrReader, "stderr", wg) + go func() { + readerFunc(s.stderrReader, "stderr") + wg.Done() + }() // Wait for the input and output to be read } if err := cmd.Wait(); err != nil { @@ -1119,6 +1135,31 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { return } + if redirected { + redirects, err := generateStdioTempPipes() + if err != nil { + s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", + fmt.Sprintf("xxx", args.OutputMode)) + } + + s.config.Debugger.Redirects[1] = redirects[0] + s.config.Debugger.Redirects[2] = redirects[1] + go func() { + stdoutFile, err := os.OpenFile(redirects[0], os.O_RDONLY, os.ModeNamedPipe) + if err != nil { + log.Fatal(err) + } + + stderrFile, err := os.OpenFile(redirects[1], os.O_RDONLY, os.ModeNamedPipe) + if err != nil { + log.Fatal(err) + } + + go readerFunc(stdoutFile, "stdout") + go readerFunc(stderrFile, "stderr") + }() + } + func() { s.mu.Lock() defer s.mu.Unlock() // Make sure to unlock in case of panic that will become internal error @@ -3869,3 +3910,29 @@ func parseLogPoint(msg string) (bool, *logMessage, error) { args: args, }, nil } + +func generateStdioTempPipes() (res [2]string, err error) { + r := make([]byte, 4) + if _, err := rand.Read(r); err != nil { + return res, 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 res, err + } + + if err := syscall.Mkfifo(stderrPath, 0o600); err != nil { + os.Remove(stdoutPath) + return res, err + } + + res[0] = stdoutPath + res[1] = stderrPath + return res, nil +} From 4e47c69b80803272828e36b7d5ef686733947ad2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Wed, 1 Feb 2023 02:02:13 +0800 Subject: [PATCH 07/53] wip: support remote debugging output redirection of windows --- service/dap/server.go | 40 +++++++------------------------ service/dap/stdio_pipe.go | 38 +++++++++++++++++++++++++++++ service/dap/stdio_pipe_windows.go | 28 ++++++++++++++++++++++ 3 files changed, 75 insertions(+), 31 deletions(-) create mode 100644 service/dap/stdio_pipe.go create mode 100644 service/dap/stdio_pipe_windows.go diff --git a/service/dap/server.go b/service/dap/server.go index 23507c13c9..c47c68ce50 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -11,8 +11,6 @@ package dap import ( "bufio" "bytes" - "crypto/rand" - "encoding/hex" "encoding/json" "errors" "fmt" @@ -33,7 +31,6 @@ import ( "strconv" "strings" "sync" - "syscall" "time" "github.com/go-delve/delve/pkg/gobuild" @@ -1155,8 +1152,15 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { log.Fatal(err) } - go readerFunc(stdoutFile, "stdout") - go readerFunc(stderrFile, "stderr") + go func() { + readerFunc(stdoutFile, "stdout") + os.Remove(redirects[0]) + }() + + go func() { + readerFunc(stderrFile, "stderr") + os.Remove(redirects[1]) + }() }() } @@ -3910,29 +3914,3 @@ func parseLogPoint(msg string) (bool, *logMessage, error) { args: args, }, nil } - -func generateStdioTempPipes() (res [2]string, err error) { - r := make([]byte, 4) - if _, err := rand.Read(r); err != nil { - return res, 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 res, err - } - - if err := syscall.Mkfifo(stderrPath, 0o600); err != nil { - os.Remove(stdoutPath) - return res, err - } - - res[0] = stdoutPath - res[1] = stderrPath - return res, nil -} diff --git a/service/dap/stdio_pipe.go b/service/dap/stdio_pipe.go new file mode 100644 index 0000000000..9f17382544 --- /dev/null +++ b/service/dap/stdio_pipe.go @@ -0,0 +1,38 @@ +//go:build !windows +// +build !windows + +package dap + +import ( + "crypto/rand" + "encoding/hex" + "os" + "path/filepath" + "syscall" +) + +func generateStdioTempPipes() (res [2]string, err error) { + r := make([]byte, 4) + if _, err := rand.Read(r); err != nil { + return res, 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 res, err + } + + if err := syscall.Mkfifo(stderrPath, 0o600); err != nil { + os.Remove(stdoutPath) + return res, err + } + + res[0] = stdoutPath + res[1] = stderrPath + return res, nil +} diff --git a/service/dap/stdio_pipe_windows.go b/service/dap/stdio_pipe_windows.go new file mode 100644 index 0000000000..a71aa0467a --- /dev/null +++ b/service/dap/stdio_pipe_windows.go @@ -0,0 +1,28 @@ +//go:build windows +// +build windows + +package dap + +import ( + "crypto/rand" + "encoding/hex" + "os" + "path/filepath" +) + +func generateStdioTempPipes() (res [2]string, err error) { + r := make([]byte, 4) + if _, err := rand.Read(r); err != nil { + return res, err + } + + var ( + prefix = filepath.Join(os.TempDir(), hex.EncodeToString(r)) + stdoutPath = prefix + "stdout" + stderrPath = prefix + "stderr" + ) + + res[0] = stdoutPath + res[1] = stderrPath + return res, nil +} From 83504058b9b025d115ca47cde8da51aff2661313 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Wed, 1 Feb 2023 23:51:57 +0800 Subject: [PATCH 08/53] wip: real-time write back output --- service/dap/server.go | 48 ++++++++++++++++++++++----------------- service/dap/stdio_pipe.go | 2 +- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/service/dap/server.go b/service/dap/server.go index c47c68ce50..4554bd0cad 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -17,7 +17,6 @@ import ( "go/constant" "go/parser" "io" - "log" "math" "net" "os" @@ -1055,39 +1054,42 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { default: s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", fmt.Sprintf("invalid debug configuration - unsupported 'outputMode' attribute %q", args.OutputMode)) + return } readerFunc := func(reader io.Reader, category string) { - // Display one line back after outputting one line - var ( - scanner = bufio.NewScanner(reader) - stdWriter io.Writer - ) - + var stdWriter io.Writer if category == "stdout" { stdWriter = os.Stdout } else { stdWriter = os.Stderr } - fmt.Println(category, "scan") - for scanner.Scan() { - out := scanner.Text() + var out [1024]byte + for { + n, err := reader.Read(out[:]) + if err != nil { + if errors.Is(io.EOF, err) { + return + } + s.config.log.Errorf("failed read by %s - %v ", category, err) + return + } + outs := string(out[:n]) + if s.outputMode&outputToStd != 0 { - fmt.Fprintln(stdWriter, out) + fmt.Fprintf(stdWriter, outs) } if s.outputMode&outputToDAP != 0 { s.send(&dap.OutputEvent{ Event: *newEvent("output"), Body: dap.OutputEventBody{ - Output: fmt.Sprintln(out), + Output: outs, Category: category, }}) } } - - fmt.Println(category, "done") } if args.NoDebug { @@ -1104,7 +1106,6 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { // Start the program on a different goroutine, so we can listen for disconnect request. go func() { - // TODO: Support remote stop program, if the program can not be stopped wg := &sync.WaitGroup{} if redirected { wg.Add(1) @@ -1136,30 +1137,35 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { redirects, err := generateStdioTempPipes() if err != nil { s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", - fmt.Sprintf("xxx", args.OutputMode)) + fmt.Sprintf("failed to generate stdio pipes - %v", err)) + return } s.config.Debugger.Redirects[1] = redirects[0] s.config.Debugger.Redirects[2] = redirects[1] - go func() { + go func() { // os.OpenFile() will block stdoutFile, err := os.OpenFile(redirects[0], os.O_RDONLY, os.ModeNamedPipe) if err != nil { - log.Fatal(err) + s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", + fmt.Sprintf("failed to open stdout pipe - %v", err)) + return } stderrFile, err := os.OpenFile(redirects[1], os.O_RDONLY, os.ModeNamedPipe) if err != nil { - log.Fatal(err) + s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", + fmt.Sprintf("failed to open stderr pipe - %v", err)) + return } go func() { readerFunc(stdoutFile, "stdout") - os.Remove(redirects[0]) + _ = os.Remove(redirects[0]) }() go func() { readerFunc(stderrFile, "stderr") - os.Remove(redirects[1]) + _ = os.Remove(redirects[1]) }() }() } diff --git a/service/dap/stdio_pipe.go b/service/dap/stdio_pipe.go index 9f17382544..d4fe4a6be5 100644 --- a/service/dap/stdio_pipe.go +++ b/service/dap/stdio_pipe.go @@ -28,7 +28,7 @@ func generateStdioTempPipes() (res [2]string, err error) { } if err := syscall.Mkfifo(stderrPath, 0o600); err != nil { - os.Remove(stdoutPath) + _ = os.Remove(stdoutPath) return res, err } From 9c96fbe8e03a0a1edeee52c3df99f8cc4711cb24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Thu, 2 Feb 2023 11:12:15 +0800 Subject: [PATCH 09/53] wip: support for windows --- pkg/proc/gdbserial/rr.go | 11 ++++++++- pkg/util/redirect.go | 39 +++++++++++++++++++++++++++++++ service/dap/server.go | 29 ++++++++++------------- service/dap/stdio_pipe.go | 14 +++++++++++ service/dap/stdio_pipe_windows.go | 32 +++++++++++++++++++++++-- 5 files changed, 105 insertions(+), 20 deletions(-) create mode 100644 pkg/util/redirect.go diff --git a/pkg/proc/gdbserial/rr.go b/pkg/proc/gdbserial/rr.go index e133b12bd2..a578b3722b 100644 --- a/pkg/proc/gdbserial/rr.go +++ b/pkg/proc/gdbserial/rr.go @@ -14,6 +14,7 @@ import ( "github.com/go-delve/delve/pkg/config" "github.com/go-delve/delve/pkg/proc" + "github.com/go-delve/delve/pkg/util" ) // RecordAsync configures rr to record the execution of the specified @@ -83,11 +84,19 @@ func openRedirects(redirects [3]string, quiet bool) (stdin, stdout, stderr *os.F } return dflt } + var f *os.File - f, err = os.Create(path) + + if pipe, ok := util.GetRedirectStrore().Load(path); ok { + f = pipe.Writer + } else { + f, err = os.Create(path) + } + if f != nil { toclose = append(toclose, f) } + return f } diff --git a/pkg/util/redirect.go b/pkg/util/redirect.go new file mode 100644 index 0000000000..fdfc752412 --- /dev/null +++ b/pkg/util/redirect.go @@ -0,0 +1,39 @@ +package util + +import ( + "fmt" + "os" + "sync" +) + +var ( + redirectMap = &RedirectStroe{rs: sync.Map{}} +) + +type RedirectStroe struct { + rs sync.Map +} + +func GetRedirectStrore() *RedirectStroe { + return redirectMap +} + +func (r *RedirectStroe) Store(key string, val *Pipe) { + fmt.Println("store", key) + r.rs.Store(key, val) +} + +func (r *RedirectStroe) Load(key string) (*Pipe, bool) { + val, ok := r.rs.Load(key) + if ok { + return val.(*Pipe), true + } + + fmt.Println("Load not found", key) + return nil, false +} + +type Pipe struct { + Reader *os.File + Writer *os.File +} diff --git a/service/dap/server.go b/service/dap/server.go index 4554bd0cad..ed6d624794 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -1136,37 +1136,32 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { if redirected { redirects, err := generateStdioTempPipes() if err != nil { - s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", + s.sendShowUserErrorResponse(request.Request, InternalError, "Internal Error", fmt.Sprintf("failed to generate stdio pipes - %v", err)) return } s.config.Debugger.Redirects[1] = redirects[0] s.config.Debugger.Redirects[2] = redirects[1] - go func() { // os.OpenFile() will block - stdoutFile, err := os.OpenFile(redirects[0], os.O_RDONLY, os.ModeNamedPipe) - if err != nil { - s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", + go func() { + if err = ReadRedirect(redirects[0], func(reader io.Reader) { + readerFunc(reader, "stdout") + }); err != nil { + s.sendShowUserErrorResponse(request.Request, InternalError, "Internal Error", fmt.Sprintf("failed to open stdout pipe - %v", err)) return } - stderrFile, err := os.OpenFile(redirects[1], os.O_RDONLY, os.ModeNamedPipe) - if err != nil { + }() + + go func() { + if err = ReadRedirect(redirects[1], func(reader io.Reader) { + readerFunc(reader, "stderr") + }); err != nil { s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", fmt.Sprintf("failed to open stderr pipe - %v", err)) return } - - go func() { - readerFunc(stdoutFile, "stdout") - _ = os.Remove(redirects[0]) - }() - - go func() { - readerFunc(stderrFile, "stderr") - _ = os.Remove(redirects[1]) - }() }() } diff --git a/service/dap/stdio_pipe.go b/service/dap/stdio_pipe.go index d4fe4a6be5..0dbed2a282 100644 --- a/service/dap/stdio_pipe.go +++ b/service/dap/stdio_pipe.go @@ -6,6 +6,8 @@ package dap import ( "crypto/rand" "encoding/hex" + "fmt" + "io" "os" "path/filepath" "syscall" @@ -36,3 +38,15 @@ func generateStdioTempPipes() (res [2]string, err error) { res[1] = stderrPath return res, nil } + +func ReadRedirect(path string, f func(reader io.Reader)) error { + fmt.Println("qq") + stdioFile, err := os.OpenFile(path, os.O_RDONLY, os.ModeNamedPipe) + if err != nil { + return err + } + + f(stdioFile) + _ = os.Remove(path) + return nil +} diff --git a/service/dap/stdio_pipe_windows.go b/service/dap/stdio_pipe_windows.go index a71aa0467a..998aea1702 100644 --- a/service/dap/stdio_pipe_windows.go +++ b/service/dap/stdio_pipe_windows.go @@ -6,8 +6,11 @@ package dap import ( "crypto/rand" "encoding/hex" + "fmt" + "io" "os" - "path/filepath" + + "github.com/go-delve/delve/pkg/util" ) func generateStdioTempPipes() (res [2]string, err error) { @@ -17,12 +20,37 @@ func generateStdioTempPipes() (res [2]string, err error) { } var ( - prefix = filepath.Join(os.TempDir(), hex.EncodeToString(r)) + prefix = hex.EncodeToString(r) stdoutPath = prefix + "stdout" stderrPath = prefix + "stderr" ) + store := util.GetRedirectStrore() + stdoutReader, stdoutWriter, err := os.Pipe() + if err != nil { + return res, err + } + + store.Store(stdoutPath, &util.Pipe{Reader: stdoutReader, Writer: stdoutWriter}) + + stderrReader, stderrWriter, err := os.Pipe() + if err != nil { + return res, err + } + + store.Store(stderrPath, &util.Pipe{Reader: stderrReader, Writer: stderrWriter}) + res[0] = stdoutPath res[1] = stderrPath return res, nil } + +func ReadRedirect(path string, f func(reader io.Reader)) error { + pipe, exist := util.GetRedirectStrore().Load(path) + if !exist { + return fmt.Errorf("redirect key(%s) not found", path) + } + + f(pipe.Reader) + return nil +} From c4b8ce2b40d1731e4077233697e1f35175c8d029 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Thu, 2 Feb 2023 22:06:13 +0800 Subject: [PATCH 10/53] wip: fix windows remote debug not output --- pkg/proc/gdbserial/rr.go | 9 +-------- pkg/proc/native/proc.go | 7 ++++++- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/pkg/proc/gdbserial/rr.go b/pkg/proc/gdbserial/rr.go index a578b3722b..6292d98255 100644 --- a/pkg/proc/gdbserial/rr.go +++ b/pkg/proc/gdbserial/rr.go @@ -14,7 +14,6 @@ import ( "github.com/go-delve/delve/pkg/config" "github.com/go-delve/delve/pkg/proc" - "github.com/go-delve/delve/pkg/util" ) // RecordAsync configures rr to record the execution of the specified @@ -86,13 +85,7 @@ func openRedirects(redirects [3]string, quiet bool) (stdin, stdout, stderr *os.F } var f *os.File - - if pipe, ok := util.GetRedirectStrore().Load(path); ok { - f = pipe.Writer - } else { - f, err = os.Create(path) - } - + f, err = os.Create(path) if f != nil { toclose = append(toclose, f) } diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go index b1c906a48e..d31233c2f8 100644 --- a/pkg/proc/native/proc.go +++ b/pkg/proc/native/proc.go @@ -5,6 +5,7 @@ import ( "runtime" "github.com/go-delve/delve/pkg/proc" + "github.com/go-delve/delve/pkg/util" ) // Process represents all of the information the debugger @@ -324,7 +325,11 @@ func openRedirects(redirects [3]string, foreground bool) (stdin, stdout, stderr return dflt } var f *os.File - f, err = os.Create(path) + if pipe, ok := util.GetRedirectStrore().Load(path); ok { + f = pipe.Writer + } else { + f, err = os.Create(path) + } if f != nil { toclose = append(toclose, f) } From a605ff936920fd46d37b75f85b6dc1af6ef4e19b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Thu, 2 Feb 2023 22:58:21 +0800 Subject: [PATCH 11/53] wip: fix truncated output redirection --- pkg/util/redirect.go | 3 --- service/dap/server.go | 25 ++++++++++++++++++------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/pkg/util/redirect.go b/pkg/util/redirect.go index fdfc752412..5c93399e93 100644 --- a/pkg/util/redirect.go +++ b/pkg/util/redirect.go @@ -1,7 +1,6 @@ package util import ( - "fmt" "os" "sync" ) @@ -19,7 +18,6 @@ func GetRedirectStrore() *RedirectStroe { } func (r *RedirectStroe) Store(key string, val *Pipe) { - fmt.Println("store", key) r.rs.Store(key, val) } @@ -29,7 +27,6 @@ func (r *RedirectStroe) Load(key string) (*Pipe, bool) { return val.(*Pipe), true } - fmt.Println("Load not found", key) return nil, false } diff --git a/service/dap/server.go b/service/dap/server.go index ed6d624794..5e6f01b6f0 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -168,6 +168,9 @@ type Session struct { // stderrReader the program's stderr. stderrReader io.ReadCloser + + // wg the WaitGroup that needs to wait before sending a terminated event. + wg sync.WaitGroup } type outputMode int8 @@ -804,6 +807,13 @@ func (s *Session) handleRequest(request dap.Message) { } func (s *Session) send(message dap.Message) { + if event, ok := message.(*dap.OutputEvent); ok { + if event.GetEvent().Event == "terminated" { + // wait for all tasks(output redirects) to finish + s.wg.Wait() + } + } + jsonmsg, _ := json.Marshal(message) s.config.log.Debug("[-> to client]", string(jsonmsg)) // TODO(polina): consider using a channel for all the sends and to have a dedicated @@ -1106,18 +1116,17 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { // Start the program on a different goroutine, so we can listen for disconnect request. go func() { - wg := &sync.WaitGroup{} if redirected { - wg.Add(1) + s.wg.Add(1) go func() { + defer s.wg.Done() readerFunc(s.stdoutReader, "stdout") - wg.Done() }() - wg.Add(1) + s.wg.Add(1) go func() { + defer s.wg.Done() readerFunc(s.stderrReader, "stderr") - wg.Done() }() // Wait for the input and output to be read } @@ -1125,7 +1134,6 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { s.config.log.Debugf("program exited with error: %v", err) } - wg.Wait() close(s.noDebugProcess.exited) s.logToConsole(proc.ErrProcessExited{Pid: cmd.ProcessState.Pid(), Status: cmd.ProcessState.ExitCode()}.Error()) s.send(&dap.TerminatedEvent{Event: *newEvent("terminated")}) @@ -1143,7 +1151,9 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { s.config.Debugger.Redirects[1] = redirects[0] s.config.Debugger.Redirects[2] = redirects[1] + s.wg.Add(1) go func() { + defer s.wg.Done() if err = ReadRedirect(redirects[0], func(reader io.Reader) { readerFunc(reader, "stdout") }); err != nil { @@ -1153,8 +1163,9 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { } }() - + s.wg.Add(1) go func() { + defer s.wg.Done() if err = ReadRedirect(redirects[1], func(reader io.Reader) { readerFunc(reader, "stderr") }); err != nil { From 984e0129cdd48273cfec8e609ca656106958c6ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Sat, 4 Feb 2023 04:10:38 +0800 Subject: [PATCH 12/53] wip: delete printfln --- service/dap/stdio_pipe.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/service/dap/stdio_pipe.go b/service/dap/stdio_pipe.go index 0dbed2a282..cd9305ae71 100644 --- a/service/dap/stdio_pipe.go +++ b/service/dap/stdio_pipe.go @@ -6,7 +6,6 @@ package dap import ( "crypto/rand" "encoding/hex" - "fmt" "io" "os" "path/filepath" @@ -40,7 +39,6 @@ func generateStdioTempPipes() (res [2]string, err error) { } func ReadRedirect(path string, f func(reader io.Reader)) error { - fmt.Println("qq") stdioFile, err := os.OpenFile(path, os.O_RDONLY, os.ModeNamedPipe) if err != nil { return err From 63341e7efd379568a16c680495603c60b595bc94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Fri, 10 Feb 2023 01:01:10 +0800 Subject: [PATCH 13/53] wip: use debugger.Config to pass redirect(macOS) --- pkg/proc/gdbserial/gdbserver.go | 18 +++++++-- pkg/proc/native/nonative_darwin.go | 3 +- pkg/proc/redirect/redirect.go | 20 +++++++++ service/dap/server.go | 12 +++--- service/dap/stdio_pipe.go | 57 +++++++++++++++++++++----- service/dap/stdio_pipe_windows.go | 65 +++++++++++++++++++++--------- service/dap/types.go | 2 + service/debugger/debugger.go | 12 ++++-- 8 files changed, 146 insertions(+), 43 deletions(-) create mode 100644 pkg/proc/redirect/redirect.go diff --git a/pkg/proc/gdbserial/gdbserver.go b/pkg/proc/gdbserial/gdbserver.go index 8f99c999d0..14cdad92e6 100644 --- a/pkg/proc/gdbserial/gdbserver.go +++ b/pkg/proc/gdbserial/gdbserver.go @@ -84,6 +84,7 @@ import ( "github.com/go-delve/delve/pkg/proc/internal/ebpf" "github.com/go-delve/delve/pkg/proc/linutil" "github.com/go-delve/delve/pkg/proc/macutil" + "github.com/go-delve/delve/pkg/proc/redirect" isatty "github.com/mattn/go-isatty" ) @@ -450,7 +451,7 @@ func getLdEnvVars() []string { // LLDBLaunch starts an instance of lldb-server and connects to it, asking // it to launch the specified target program with the specified arguments // (cmd) on the specified directory wd. -func LLDBLaunch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []string, tty string, redirects [3]string) (*proc.Target, error) { +func LLDBLaunch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []string, tty string, redirects redirect.Redirect) (*proc.Target, error) { if runtime.GOOS == "windows" { return nil, ErrUnsupportedOS } @@ -483,11 +484,20 @@ func LLDBLaunch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs [ } else { found := [3]bool{} names := [3]string{"stdin", "stdout", "stderr"} - for i := range redirects { - if redirects[i] != "" { + rds := [3]string{} + if paths, err := redirects.RedirectPath(); err != nil { + if !errors.Is(redirect.ErrorNotImplemented, err) { + return nil, err + } + } else { + rds = paths + } + + for i := range rds { + if rds[i] != "" { found[i] = true hasRedirects = true - args = append(args, fmt.Sprintf("--%s-path", names[i]), redirects[i]) + args = append(args, fmt.Sprintf("--%s-path", names[i]), rds[i]) } } diff --git a/pkg/proc/native/nonative_darwin.go b/pkg/proc/native/nonative_darwin.go index 3aae7e4689..14ddd7e17c 100644 --- a/pkg/proc/native/nonative_darwin.go +++ b/pkg/proc/native/nonative_darwin.go @@ -11,12 +11,13 @@ import ( "github.com/go-delve/delve/pkg/proc" "github.com/go-delve/delve/pkg/proc/amd64util" "github.com/go-delve/delve/pkg/proc/internal/ebpf" + "github.com/go-delve/delve/pkg/proc/redirect" ) var ErrNativeBackendDisabled = errors.New("native backend disabled during compilation") // Launch returns ErrNativeBackendDisabled. -func Launch(_ []string, _ string, _ proc.LaunchFlags, _ []string, _ string, _ [3]string) (*proc.Target, error) { +func Launch(_ []string, _ string, _ proc.LaunchFlags, _ []string, _ string, _ redirect.Redirect) (*proc.Target, error) { return nil, ErrNativeBackendDisabled } diff --git a/pkg/proc/redirect/redirect.go b/pkg/proc/redirect/redirect.go new file mode 100644 index 0000000000..db90067007 --- /dev/null +++ b/pkg/proc/redirect/redirect.go @@ -0,0 +1,20 @@ +package redirect + +import ( + "errors" + "os" +) + +var ErrorNotImplemented = errors.New("not implemented") + +// Redirect +type Redirect interface { + // RedirectPath + RedirectPath() (redirects [3]string, err error) + // RedirectFile + RedirectReaderFile() (readerFile [3]*os.File, err error) + // RedirectWriterFile + RedirectWriterFile() (writerFile [3]*os.File, err error) + // ReStart + ReStart() error +} diff --git a/service/dap/server.go b/service/dap/server.go index eff48ea1f9..4a6fa6f001 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -1059,6 +1059,9 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { case "remote": s.outputMode |= outputToDAP redirected = true + case "only-remote": + s.outputMode = outputToDAP + redirected = true case "local", "": // noting default: @@ -1142,19 +1145,18 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { } if redirected { - redirects, err := generateStdioTempPipes() + redirector, err := NewRedirector() if err != nil { s.sendShowUserErrorResponse(request.Request, InternalError, "Internal Error", fmt.Sprintf("failed to generate stdio pipes - %v", err)) return } - s.config.Debugger.Redirects[1] = redirects[0] - s.config.Debugger.Redirects[2] = redirects[1] + s.config.Debugger.Redirect = redirector s.wg.Add(1) go func() { defer s.wg.Done() - if err = ReadRedirect(redirects[0], func(reader io.Reader) { + if err = redirector.ReadRedirect("stdout", func(reader io.Reader) { readerFunc(reader, "stdout") }); err != nil { s.sendShowUserErrorResponse(request.Request, InternalError, "Internal Error", @@ -1166,7 +1168,7 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { s.wg.Add(1) go func() { defer s.wg.Done() - if err = ReadRedirect(redirects[1], func(reader io.Reader) { + if err = redirector.ReadRedirect("stderr", func(reader io.Reader) { readerFunc(reader, "stderr") }); err != nil { s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", diff --git a/service/dap/stdio_pipe.go b/service/dap/stdio_pipe.go index cd9305ae71..122425fd8a 100644 --- a/service/dap/stdio_pipe.go +++ b/service/dap/stdio_pipe.go @@ -10,12 +10,19 @@ import ( "os" "path/filepath" "syscall" + + "github.com/go-delve/delve/pkg/proc/redirect" ) -func generateStdioTempPipes() (res [2]string, err error) { +type redirector struct { + stdoutPath string + stderrPath string +} + +func NewRedirector() (red *redirector, err error) { r := make([]byte, 4) if _, err := rand.Read(r); err != nil { - return res, err + return nil, err } var ( @@ -25,26 +32,54 @@ func generateStdioTempPipes() (res [2]string, err error) { ) if err := syscall.Mkfifo(stdoutPath, 0o600); err != nil { - return res, err + return nil, err } if err := syscall.Mkfifo(stderrPath, 0o600); err != nil { _ = os.Remove(stdoutPath) - return res, err + return nil, err } - res[0] = stdoutPath - res[1] = stderrPath - return res, nil + return &redirector{ + stdoutPath: stdoutPath, + stderrPath: stderrPath, + }, nil +} + +// RedirectFile +func (r *redirector) RedirectReaderFile() (readerFile [3]*os.File, err error) { + return readerFile, redirect.ErrorNotImplemented +} + +// RedirectWriterFile +func (r *redirector) RedirectWriterFile() (writerFile [3]*os.File, err error) { + return writerFile, redirect.ErrorNotImplemented } -func ReadRedirect(path string, f func(reader io.Reader)) error { - stdioFile, err := os.OpenFile(path, os.O_RDONLY, os.ModeNamedPipe) +// RedirectPath +func (r *redirector) RedirectPath() (redirects [3]string, err error) { + // TODO support stdin + return [3]string{"", r.stdoutPath, r.stderrPath}, nil +} + +// ReStart +func (r *redirector) ReStart() error { + panic("not implemented") // TODO: Implement +} + +func (r *redirector) ReadRedirect(stdType string, f func(reader io.Reader)) error { + path := r.stderrPath + if stdType == "stdout" { + path = r.stdoutPath + } + + defer os.Remove(path) + + stdoutFile, err := os.OpenFile(path, os.O_RDONLY, os.ModeNamedPipe) if err != nil { return err } - f(stdioFile) - _ = os.Remove(path) + f(stdoutFile) return nil } diff --git a/service/dap/stdio_pipe_windows.go b/service/dap/stdio_pipe_windows.go index 998aea1702..fec3a3a440 100644 --- a/service/dap/stdio_pipe_windows.go +++ b/service/dap/stdio_pipe_windows.go @@ -5,44 +5,64 @@ package dap import ( "crypto/rand" - "encoding/hex" "fmt" "io" "os" + "github.com/go-delve/delve/pkg/proc/redirect" "github.com/go-delve/delve/pkg/util" ) -func generateStdioTempPipes() (res [2]string, err error) { +type redirector struct { + stdoutWriterFile *os.File + stderrWriterFile *os.File + stdoutReaderFile *os.File + stderrReaderFile *os.File +} + +func NewRedirector() (red *redirector, err error) { r := make([]byte, 4) if _, err := rand.Read(r); err != nil { - return res, err + return nil, err } - var ( - prefix = hex.EncodeToString(r) - stdoutPath = prefix + "stdout" - stderrPath = prefix + "stderr" - ) - - store := util.GetRedirectStrore() stdoutReader, stdoutWriter, err := os.Pipe() if err != nil { - return res, err + return nil, err } - store.Store(stdoutPath, &util.Pipe{Reader: stdoutReader, Writer: stdoutWriter}) - stderrReader, stderrWriter, err := os.Pipe() if err != nil { - return res, err + return nil, err } - store.Store(stderrPath, &util.Pipe{Reader: stderrReader, Writer: stderrWriter}) + return &redirector{ + stdoutWriterFile: stdoutWriter, + stderrWriterFile: stderrWriter, + stdoutReaderFile: stdoutReader, + stderrReaderFile: stderrReader, + }, nil + +} + +// RedirectPath +func (r *redirector) RedirectPath() (redirects [3]string, err error) { + return redirects, redirect.ErrorNotImplemented +} + +// RedirectFile +func (r *redirector) RedirectReaderFile() (readerFile [3]*os.File, err error) { + return [3]*os.File{nil, r.stdoutReaderFile, r.stderrReaderFile}, nil +} + +// RedirectWriterFile +func (r *redirector) RedirectWriterFile() (writerFile [3]*os.File, err error) { + return [3]*os.File{nil, r.stdoutWriterFile, r.stderrWriterFile}, nil +} - res[0] = stdoutPath - res[1] = stderrPath - return res, nil +// ReStart +func (r *redirector) ReStart() error { + panic("not implemented") // TODO: Implement } func ReadRedirect(path string, f func(reader io.Reader)) error { @@ -54,3 +74,12 @@ func ReadRedirect(path string, f func(reader io.Reader)) error { f(pipe.Reader) return nil } + +func (r *redirector) ReadRedirect(stdType string, f func(reader io.Reader)) error { + if stdType == "stdout" { + f(r.stdoutReaderFile) + } else { + f(r.stderrReaderFile) + } + return nil +} diff --git a/service/dap/types.go b/service/dap/types.go index 09b3825348..73e579d048 100644 --- a/service/dap/types.go +++ b/service/dap/types.go @@ -148,7 +148,9 @@ type LaunchConfig struct { // reference to other environment variables is not supported. Env map[string]*string `json:"env,omitempty"` + // The output mode specifies how to handle the program's output. OutputMode string `json:"outputMode,omitempty"` + LaunchAttachCommonConfig } diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index 746d167472..fb118b5bdb 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -30,6 +30,7 @@ import ( "github.com/go-delve/delve/pkg/proc/core" "github.com/go-delve/delve/pkg/proc/gdbserial" "github.com/go-delve/delve/pkg/proc/native" + "github.com/go-delve/delve/pkg/proc/redirect" "github.com/go-delve/delve/service/api" "github.com/sirupsen/logrus" ) @@ -140,6 +141,9 @@ type Config struct { // Redirects specifies redirect rules for stdin, stdout and stderr Redirects [3]string + // Redirects specifies redirect rules for stdin, stdout and stderr + Redirect redirect.Redirect + // DisableASLR disables ASLR DisableASLR bool } @@ -263,9 +267,9 @@ func (d *Debugger) Launch(processArgs []string, wd string) (*proc.Target, error) switch d.config.Backend { case "native": - return native.Launch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, d.config.Redirects) + return native.Launch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, d.config.Redirect) case "lldb": - return betterGdbserialLaunchError(gdbserial.LLDBLaunch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, d.config.Redirects)) + return betterGdbserialLaunchError(gdbserial.LLDBLaunch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, d.config.Redirect)) case "rr": if d.target != nil { // restart should not call us if the backend is 'rr' @@ -307,9 +311,9 @@ func (d *Debugger) Launch(processArgs []string, wd string) (*proc.Target, error) case "default": if runtime.GOOS == "darwin" { - return betterGdbserialLaunchError(gdbserial.LLDBLaunch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, d.config.Redirects)) + return betterGdbserialLaunchError(gdbserial.LLDBLaunch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, d.config.Redirect)) } - return native.Launch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, d.config.Redirects) + return native.Launch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, d.config.Redirect) default: return nil, fmt.Errorf("unknown backend %q", d.config.Backend) } From d5f5fb35530dd03d98f0b5386c224505039a60c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Sun, 12 Feb 2023 01:19:06 +0800 Subject: [PATCH 14/53] wip: use debugger.Config to pass redirect(linux) --- pkg/proc/gdbserial/rr.go | 68 ++++++++++++++++++++++++----------- pkg/proc/gdbserial/rr_test.go | 3 +- pkg/proc/native/proc.go | 62 ++++++++++++++++++++++---------- pkg/proc/native/proc_linux.go | 5 +-- pkg/proc/redirect/redirect.go | 31 +++++++++++++++- service/debugger/debugger.go | 4 +-- 6 files changed, 128 insertions(+), 45 deletions(-) diff --git a/pkg/proc/gdbserial/rr.go b/pkg/proc/gdbserial/rr.go index 6292d98255..bb2ceb403c 100644 --- a/pkg/proc/gdbserial/rr.go +++ b/pkg/proc/gdbserial/rr.go @@ -2,6 +2,7 @@ package gdbserial import ( "bufio" + "errors" "fmt" "io" "io/ioutil" @@ -14,13 +15,14 @@ import ( "github.com/go-delve/delve/pkg/config" "github.com/go-delve/delve/pkg/proc" + "github.com/go-delve/delve/pkg/proc/redirect" ) // RecordAsync configures rr to record the execution of the specified // 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 redirect.Redirect) (run func() (string, error), stop func() error, err error) { if err := checkRRAvailable(); err != nil { return nil, nil, err } @@ -63,58 +65,82 @@ 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 redirect.Redirect, quiet bool) (stdin, stdout, stderr *os.File, closefn func(), err error) { + var ( + toclose = []*os.File{} + ) - if redirects[0] != "" { - stdin, err = os.Open(redirects[0]) + closefn = func() { + for _, f := range toclose { + _ = f.Close() + } + } + + writerFiles, err := redirects.RedirectWriterFile() + if err == nil { + stdin = writerFiles[0] + if stdin == nil && quiet { + stdin = os.Stdin + } + stdout = writerFiles[1] + stderr = writerFiles[2] + + return stdin, stdout, stderr, closefn, nil + } + + if !errors.Is(redirect.ErrorNotImplemented, err) { + return nil, nil, nil, nil, err + } + + // RedirectWriterFile is not implemented. + // use redirect path. + redirectPath, err := redirects.RedirectPath() + if err != nil { + if !errors.Is(redirect.ErrorNotImplemented, err) { + return nil, nil, nil, nil, err + } + + redirectPath = [3]string{} + } + + if redirectPath[0] != "" { + stdin, err = os.Open(redirectPath[0]) if err != nil { return nil, nil, nil, nil, err } toclose = append(toclose, stdin) - } else { + } else if quiet { stdin = os.Stdin } create := func(path string, dflt *os.File) *os.File { if path == "" { - if quiet { - return nil - } return dflt } - var f *os.File f, err = os.Create(path) if f != nil { toclose = append(toclose, f) } - return f } - stdout = create(redirects[1], os.Stdout) + stdout = create(redirectPath[1], os.Stdout) if err != nil { return nil, nil, nil, nil, err } - stderr = create(redirects[2], os.Stderr) + stderr = create(redirectPath[2], os.Stderr) if err != nil { return nil, nil, nil, nil, err } - closefn = func() { - for _, f := range toclose { - _ = f.Close() - } - } - return stdin, stdout, stderr, closefn, nil } // 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 redirect.Redirect) (tracedir string, err error) { run, _, err := RecordAsync(cmd, wd, quiet, redirects) if err != nil { return "", err @@ -281,7 +307,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.Target, string, error) { +func RecordAndReplay(cmd []string, wd string, quiet bool, debugInfoDirs []string, redirects redirect.Redirect) (*proc.Target, string, error) { tracedir, err := Record(cmd, wd, quiet, redirects) if tracedir == "" { return nil, "", err diff --git a/pkg/proc/gdbserial/rr_test.go b/pkg/proc/gdbserial/rr_test.go index a83e249f64..9e8d63154a 100644 --- a/pkg/proc/gdbserial/rr_test.go +++ b/pkg/proc/gdbserial/rr_test.go @@ -12,6 +12,7 @@ import ( "github.com/go-delve/delve/pkg/logflags" "github.com/go-delve/delve/pkg/proc" "github.com/go-delve/delve/pkg/proc/gdbserial" + "github.com/go-delve/delve/pkg/proc/redirect" protest "github.com/go-delve/delve/pkg/proc/test" ) @@ -30,7 +31,7 @@ func withTestRecording(name string, t testing.TB, fn func(grp *proc.TargetGroup, t.Skip("test skipped, rr not found") } t.Log("recording") - p, tracedir, err := gdbserial.RecordAndReplay([]string{fixture.Path}, ".", true, []string{}, [3]string{}) + p, tracedir, err := gdbserial.RecordAndReplay([]string{fixture.Path}, ".", true, []string{}, redirect.NewRedirectPath([3]string{})) if err != nil { t.Fatal("Launch():", err) } diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go index d31233c2f8..6f26068816 100644 --- a/pkg/proc/native/proc.go +++ b/pkg/proc/native/proc.go @@ -1,11 +1,12 @@ package native import ( + "errors" "os" "runtime" "github.com/go-delve/delve/pkg/proc" - "github.com/go-delve/delve/pkg/util" + "github.com/go-delve/delve/pkg/proc/redirect" ) // Process represents all of the information the debugger @@ -307,11 +308,46 @@ 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 redirect.Redirect, foreground bool) (stdin, stdout, stderr *os.File, closefn func(), err error) { + var ( + toclose = []*os.File{} + ) - if redirects[0] != "" { - stdin, err = os.Open(redirects[0]) + closefn = func() { + for _, f := range toclose { + _ = f.Close() + } + } + + writerFiles, err := redirects.RedirectWriterFile() + if err == nil { + stdin = writerFiles[0] + if stdin == nil && foreground { + stdin = os.Stdin + } + stdout = writerFiles[1] + stderr = writerFiles[2] + + return stdin, stdout, stderr, closefn, nil + } + + if !errors.Is(redirect.ErrorNotImplemented, err) { + return nil, nil, nil, nil, err + } + + // RedirectWriterFile is not implemented. + // use redirect path. + redirectPath, err := redirects.RedirectPath() + if err != nil { + if !errors.Is(redirect.ErrorNotImplemented, err) { + return nil, nil, nil, nil, err + } + + redirectPath = [3]string{} + } + + if redirectPath[0] != "" { + stdin, err = os.Open(redirectPath[0]) if err != nil { return nil, nil, nil, nil, err } @@ -325,32 +361,22 @@ func openRedirects(redirects [3]string, foreground bool) (stdin, stdout, stderr return dflt } var f *os.File - if pipe, ok := util.GetRedirectStrore().Load(path); ok { - f = pipe.Writer - } else { - f, err = os.Create(path) - } + f, err = os.Create(path) if f != nil { toclose = append(toclose, f) } return f } - stdout = create(redirects[1], os.Stdout) + stdout = create(redirectPath[1], os.Stdout) if err != nil { return nil, nil, nil, nil, err } - stderr = create(redirects[2], os.Stderr) + stderr = create(redirectPath[2], os.Stderr) if err != nil { return nil, nil, nil, nil, err } - closefn = func() { - for _, f := range toclose { - _ = f.Close() - } - } - return stdin, stdout, stderr, closefn, nil } diff --git a/pkg/proc/native/proc_linux.go b/pkg/proc/native/proc_linux.go index 29a08b506c..e3123a4b06 100644 --- a/pkg/proc/native/proc_linux.go +++ b/pkg/proc/native/proc_linux.go @@ -22,6 +22,7 @@ import ( "github.com/go-delve/delve/pkg/proc" "github.com/go-delve/delve/pkg/proc/internal/ebpf" "github.com/go-delve/delve/pkg/proc/linutil" + "github.com/go-delve/delve/pkg/proc/redirect" isatty "github.com/mattn/go-isatty" ) @@ -62,7 +63,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.Target, error) { +func Launch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []string, tty string, redirect redirect.Redirect) (*proc.Target, error) { var ( process *exec.Cmd err error @@ -70,7 +71,7 @@ func Launch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []str 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 } diff --git a/pkg/proc/redirect/redirect.go b/pkg/proc/redirect/redirect.go index db90067007..8644b3f310 100644 --- a/pkg/proc/redirect/redirect.go +++ b/pkg/proc/redirect/redirect.go @@ -7,7 +7,8 @@ import ( var ErrorNotImplemented = errors.New("not implemented") -// Redirect +// Redirect stpecify how the program is redirected. +// If the function is not supported, should return ErrorNotImplemented. type Redirect interface { // RedirectPath RedirectPath() (redirects [3]string, err error) @@ -18,3 +19,31 @@ type Redirect interface { // ReStart ReStart() error } + +type onlyRedirectPath struct { + redirects [3]string +} + +func NewRedirectPath(redirects [3]string) *onlyRedirectPath { + return &onlyRedirectPath{redirects: redirects} +} + +// RedirectPath +func (o *onlyRedirectPath) RedirectPath() (redirects [3]string, err error) { + return o.redirects, nil +} + +// RedirectFile +func (o *onlyRedirectPath) RedirectReaderFile() (readerFile [3]*os.File, err error) { + return readerFile, ErrorNotImplemented +} + +// RedirectWriterFile +func (o *onlyRedirectPath) RedirectWriterFile() (writerFile [3]*os.File, err error) { + return writerFile, ErrorNotImplemented +} + +// ReStart +func (o *onlyRedirectPath) ReStart() error { + return ErrorNotImplemented +} diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index fb118b5bdb..f393d4ce03 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -276,7 +276,7 @@ func (d *Debugger) Launch(processArgs []string, wd string) (*proc.Target, error) panic("internal error: call to Launch with rr backend and target already exists") } - run, stop, err := gdbserial.RecordAsync(processArgs, wd, false, d.config.Redirects) + run, stop, err := gdbserial.RecordAsync(processArgs, wd, false, d.config.Redirect) if err != nil { return nil, err } @@ -509,7 +509,7 @@ func (d *Debugger) Restart(rerecord bool, pos string, resetArgs bool, newArgs [] } if recorded { - run, stop, err2 := gdbserial.RecordAsync(d.processArgs, d.config.WorkingDir, false, d.config.Redirects) + run, stop, err2 := gdbserial.RecordAsync(d.processArgs, d.config.WorkingDir, false, d.config.Redirect) if err2 != nil { return nil, err2 } From 4621f4c7afdac11b18c9a155888ad3b0823fcc1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Sun, 12 Feb 2023 02:48:21 +0800 Subject: [PATCH 15/53] wip: Change redirect to a concrete type --- cmd/dlv/cmds/commands.go | 3 +- pkg/proc/gdbserial/gdbserver.go | 18 ++------- pkg/proc/gdbserial/rr.go | 44 ++++++--------------- pkg/proc/gdbserial/rr_test.go | 3 +- pkg/proc/native/nonative_darwin.go | 3 +- pkg/proc/native/proc.go | 36 +++++------------ pkg/proc/native/proc_linux.go | 3 +- pkg/proc/native/proc_windows.go | 2 +- pkg/proc/redirect.go | 29 ++++++++++++++ pkg/proc/redirect/redirect.go | 49 ----------------------- pkg/util/redirect.go | 36 ----------------- service/dap/server.go | 8 ++-- service/dap/stdio_pipe.go | 49 +++++------------------ service/dap/stdio_pipe_windows.go | 63 +++++------------------------- service/debugger/debugger.go | 8 +--- 15 files changed, 86 insertions(+), 268 deletions(-) create mode 100644 pkg/proc/redirect.go delete mode 100644 pkg/proc/redirect/redirect.go delete mode 100644 pkg/util/redirect.go diff --git a/cmd/dlv/cmds/commands.go b/cmd/dlv/cmds/commands.go index 39c0b3ea5a..753b225a1d 100644 --- a/cmd/dlv/cmds/commands.go +++ b/cmd/dlv/cmds/commands.go @@ -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" @@ -979,7 +980,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, }, }) diff --git a/pkg/proc/gdbserial/gdbserver.go b/pkg/proc/gdbserial/gdbserver.go index 14cdad92e6..409e01148f 100644 --- a/pkg/proc/gdbserial/gdbserver.go +++ b/pkg/proc/gdbserial/gdbserver.go @@ -84,7 +84,6 @@ import ( "github.com/go-delve/delve/pkg/proc/internal/ebpf" "github.com/go-delve/delve/pkg/proc/linutil" "github.com/go-delve/delve/pkg/proc/macutil" - "github.com/go-delve/delve/pkg/proc/redirect" isatty "github.com/mattn/go-isatty" ) @@ -451,7 +450,7 @@ func getLdEnvVars() []string { // LLDBLaunch starts an instance of lldb-server and connects to it, asking // it to launch the specified target program with the specified arguments // (cmd) on the specified directory wd. -func LLDBLaunch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []string, tty string, redirects redirect.Redirect) (*proc.Target, error) { +func LLDBLaunch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []string, tty string, redirects proc.Redirect) (*proc.Target, error) { if runtime.GOOS == "windows" { return nil, ErrUnsupportedOS } @@ -484,20 +483,11 @@ func LLDBLaunch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs [ } else { found := [3]bool{} names := [3]string{"stdin", "stdout", "stderr"} - rds := [3]string{} - if paths, err := redirects.RedirectPath(); err != nil { - if !errors.Is(redirect.ErrorNotImplemented, err) { - return nil, err - } - } else { - rds = paths - } - - for i := range rds { - if rds[i] != "" { + for i := range redirects.Paths { + if redirects.Paths[i] != "" { found[i] = true hasRedirects = true - args = append(args, fmt.Sprintf("--%s-path", names[i]), rds[i]) + args = append(args, fmt.Sprintf("--%s-path", names[i]), redirects.Paths[i]) } } diff --git a/pkg/proc/gdbserial/rr.go b/pkg/proc/gdbserial/rr.go index bb2ceb403c..081f589147 100644 --- a/pkg/proc/gdbserial/rr.go +++ b/pkg/proc/gdbserial/rr.go @@ -2,7 +2,6 @@ package gdbserial import ( "bufio" - "errors" "fmt" "io" "io/ioutil" @@ -15,14 +14,13 @@ import ( "github.com/go-delve/delve/pkg/config" "github.com/go-delve/delve/pkg/proc" - "github.com/go-delve/delve/pkg/proc/redirect" ) // RecordAsync configures rr to record the execution of the specified // 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 redirect.Redirect) (run func() (string, error), stop func() error, err error) { +func RecordAsync(cmd []string, wd string, quiet bool, redirects proc.Redirect) (run func() (string, error), stop func() error, err error) { if err := checkRRAvailable(); err != nil { return nil, nil, err } @@ -65,7 +63,7 @@ func RecordAsync(cmd []string, wd string, quiet bool, redirects redirect.Redirec return run, stop, nil } -func openRedirects(redirects redirect.Redirect, quiet bool) (stdin, stdout, stderr *os.File, closefn func(), err error) { +func openRedirects(redirects proc.Redirect, quiet bool) (stdin, stdout, stderr *os.File, closefn func(), err error) { var ( toclose = []*os.File{} ) @@ -76,35 +74,19 @@ func openRedirects(redirects redirect.Redirect, quiet bool) (stdin, stdout, stde } } - writerFiles, err := redirects.RedirectWriterFile() - if err == nil { - stdin = writerFiles[0] - if stdin == nil && quiet { + if redirects.Mode == proc.RedirectFileMode { + stdin = redirects.WriterFiles[0] + if stdin == nil { stdin = os.Stdin } - stdout = writerFiles[1] - stderr = writerFiles[2] + stdout = redirects.WriterFiles[1] + stderr = redirects.WriterFiles[2] return stdin, stdout, stderr, closefn, nil } - if !errors.Is(redirect.ErrorNotImplemented, err) { - return nil, nil, nil, nil, err - } - - // RedirectWriterFile is not implemented. - // use redirect path. - redirectPath, err := redirects.RedirectPath() - if err != nil { - if !errors.Is(redirect.ErrorNotImplemented, err) { - return nil, nil, nil, nil, err - } - - redirectPath = [3]string{} - } - - if redirectPath[0] != "" { - stdin, err = os.Open(redirectPath[0]) + if redirects.Paths[0] != "" { + stdin, err = os.Open(redirects.Paths[0]) if err != nil { return nil, nil, nil, nil, err } @@ -125,12 +107,12 @@ func openRedirects(redirects redirect.Redirect, quiet bool) (stdin, stdout, stde return f } - stdout = create(redirectPath[1], os.Stdout) + stdout = create(redirects.Paths[1], os.Stdout) if err != nil { return nil, nil, nil, nil, err } - stderr = create(redirectPath[2], os.Stderr) + stderr = create(redirects.Paths[2], os.Stderr) if err != nil { return nil, nil, nil, nil, err } @@ -140,7 +122,7 @@ func openRedirects(redirects redirect.Redirect, quiet bool) (stdin, stdout, stde // 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 redirect.Redirect) (tracedir string, err error) { +func Record(cmd []string, wd string, quiet bool, redirects proc.Redirect) (tracedir string, err error) { run, _, err := RecordAsync(cmd, wd, quiet, redirects) if err != nil { return "", err @@ -307,7 +289,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 redirect.Redirect) (*proc.Target, string, error) { +func RecordAndReplay(cmd []string, wd string, quiet bool, debugInfoDirs []string, redirects proc.Redirect) (*proc.Target, string, error) { tracedir, err := Record(cmd, wd, quiet, redirects) if tracedir == "" { return nil, "", err diff --git a/pkg/proc/gdbserial/rr_test.go b/pkg/proc/gdbserial/rr_test.go index 9e8d63154a..c156193da0 100644 --- a/pkg/proc/gdbserial/rr_test.go +++ b/pkg/proc/gdbserial/rr_test.go @@ -12,7 +12,6 @@ import ( "github.com/go-delve/delve/pkg/logflags" "github.com/go-delve/delve/pkg/proc" "github.com/go-delve/delve/pkg/proc/gdbserial" - "github.com/go-delve/delve/pkg/proc/redirect" protest "github.com/go-delve/delve/pkg/proc/test" ) @@ -31,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") - p, tracedir, err := gdbserial.RecordAndReplay([]string{fixture.Path}, ".", true, []string{}, redirect.NewRedirectPath([3]string{})) + p, tracedir, err := gdbserial.RecordAndReplay([]string{fixture.Path}, ".", true, []string{}, proc.NewEmptyRedirectByPath()) if err != nil { t.Fatal("Launch():", err) } diff --git a/pkg/proc/native/nonative_darwin.go b/pkg/proc/native/nonative_darwin.go index 14ddd7e17c..74ab308e61 100644 --- a/pkg/proc/native/nonative_darwin.go +++ b/pkg/proc/native/nonative_darwin.go @@ -11,13 +11,12 @@ import ( "github.com/go-delve/delve/pkg/proc" "github.com/go-delve/delve/pkg/proc/amd64util" "github.com/go-delve/delve/pkg/proc/internal/ebpf" - "github.com/go-delve/delve/pkg/proc/redirect" ) var ErrNativeBackendDisabled = errors.New("native backend disabled during compilation") // Launch returns ErrNativeBackendDisabled. -func Launch(_ []string, _ string, _ proc.LaunchFlags, _ []string, _ string, _ redirect.Redirect) (*proc.Target, error) { +func Launch(_ []string, _ string, _ proc.LaunchFlags, _ []string, _ string, _ proc.Redirect) (*proc.Target, error) { return nil, ErrNativeBackendDisabled } diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go index 6f26068816..49637cd217 100644 --- a/pkg/proc/native/proc.go +++ b/pkg/proc/native/proc.go @@ -1,12 +1,10 @@ package native import ( - "errors" "os" "runtime" "github.com/go-delve/delve/pkg/proc" - "github.com/go-delve/delve/pkg/proc/redirect" ) // Process represents all of the information the debugger @@ -308,7 +306,7 @@ func (dbp *nativeProcess) writeSoftwareBreakpoint(thread *nativeThread, addr uin return err } -func openRedirects(redirects redirect.Redirect, foreground bool) (stdin, stdout, stderr *os.File, closefn func(), err error) { +func openRedirects(redirects proc.Redirect, foreground bool) (stdin, stdout, stderr *os.File, closefn func(), err error) { var ( toclose = []*os.File{} ) @@ -319,35 +317,19 @@ func openRedirects(redirects redirect.Redirect, foreground bool) (stdin, stdout, } } - writerFiles, err := redirects.RedirectWriterFile() - if err == nil { - stdin = writerFiles[0] + if redirects.Mode == proc.RedirectFileMode { + stdin = redirects.WriterFiles[0] if stdin == nil && foreground { stdin = os.Stdin } - stdout = writerFiles[1] - stderr = writerFiles[2] + stdout = redirects.WriterFiles[1] + stderr = redirects.WriterFiles[2] return stdin, stdout, stderr, closefn, nil } - if !errors.Is(redirect.ErrorNotImplemented, err) { - return nil, nil, nil, nil, err - } - - // RedirectWriterFile is not implemented. - // use redirect path. - redirectPath, err := redirects.RedirectPath() - if err != nil { - if !errors.Is(redirect.ErrorNotImplemented, err) { - return nil, nil, nil, nil, err - } - - redirectPath = [3]string{} - } - - if redirectPath[0] != "" { - stdin, err = os.Open(redirectPath[0]) + if redirects.Paths[0] != "" { + stdin, err = os.Open(redirects.Paths[0]) if err != nil { return nil, nil, nil, nil, err } @@ -368,12 +350,12 @@ func openRedirects(redirects redirect.Redirect, foreground bool) (stdin, stdout, return f } - stdout = create(redirectPath[1], os.Stdout) + stdout = create(redirects.Paths[1], os.Stdout) if err != nil { return nil, nil, nil, nil, err } - stderr = create(redirectPath[2], os.Stderr) + stderr = create(redirects.Paths[2], os.Stderr) if err != nil { return nil, nil, nil, nil, err } diff --git a/pkg/proc/native/proc_linux.go b/pkg/proc/native/proc_linux.go index e3123a4b06..e04dbdea82 100644 --- a/pkg/proc/native/proc_linux.go +++ b/pkg/proc/native/proc_linux.go @@ -22,7 +22,6 @@ import ( "github.com/go-delve/delve/pkg/proc" "github.com/go-delve/delve/pkg/proc/internal/ebpf" "github.com/go-delve/delve/pkg/proc/linutil" - "github.com/go-delve/delve/pkg/proc/redirect" isatty "github.com/mattn/go-isatty" ) @@ -63,7 +62,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, redirect redirect.Redirect) (*proc.Target, error) { +func Launch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []string, tty string, redirect proc.Redirect) (*proc.Target, error) { var ( process *exec.Cmd err error diff --git a/pkg/proc/native/proc_windows.go b/pkg/proc/native/proc_windows.go index d37249ed0d..dba842c8ae 100644 --- a/pkg/proc/native/proc_windows.go +++ b/pkg/proc/native/proc_windows.go @@ -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.Target, error) { +func Launch(cmd []string, wd string, flags proc.LaunchFlags, _ []string, _ string, redirects proc.Redirect) (*proc.Target, error) { argv0Go := cmd[0] env := proc.DisableAsyncPreemptEnv() diff --git a/pkg/proc/redirect.go b/pkg/proc/redirect.go new file mode 100644 index 0000000000..fb7df62e0c --- /dev/null +++ b/pkg/proc/redirect.go @@ -0,0 +1,29 @@ +package proc + +import "os" + +type Mode string + +const ( + RedirectFileMode Mode = "file" + RedirectPathMode Mode = "path" +) + +type Redirect struct { + WriterFiles [3]*os.File + ReaderFiles [3]*os.File + Paths [3]string + Mode Mode +} + +func NewEmptyRedirectByPath() Redirect { + return Redirect{Mode: RedirectPathMode} +} + +func NewRedirectByPath(paths [3]string) Redirect { + return Redirect{Paths: paths, Mode: RedirectPathMode} +} + +func NewRedirectByFile(readerFiles [3]*os.File, writerFiles [3]*os.File) Redirect { + return Redirect{ReaderFiles: readerFiles, WriterFiles: writerFiles, Mode: RedirectFileMode} +} diff --git a/pkg/proc/redirect/redirect.go b/pkg/proc/redirect/redirect.go deleted file mode 100644 index 8644b3f310..0000000000 --- a/pkg/proc/redirect/redirect.go +++ /dev/null @@ -1,49 +0,0 @@ -package redirect - -import ( - "errors" - "os" -) - -var ErrorNotImplemented = errors.New("not implemented") - -// Redirect stpecify how the program is redirected. -// If the function is not supported, should return ErrorNotImplemented. -type Redirect interface { - // RedirectPath - RedirectPath() (redirects [3]string, err error) - // RedirectFile - RedirectReaderFile() (readerFile [3]*os.File, err error) - // RedirectWriterFile - RedirectWriterFile() (writerFile [3]*os.File, err error) - // ReStart - ReStart() error -} - -type onlyRedirectPath struct { - redirects [3]string -} - -func NewRedirectPath(redirects [3]string) *onlyRedirectPath { - return &onlyRedirectPath{redirects: redirects} -} - -// RedirectPath -func (o *onlyRedirectPath) RedirectPath() (redirects [3]string, err error) { - return o.redirects, nil -} - -// RedirectFile -func (o *onlyRedirectPath) RedirectReaderFile() (readerFile [3]*os.File, err error) { - return readerFile, ErrorNotImplemented -} - -// RedirectWriterFile -func (o *onlyRedirectPath) RedirectWriterFile() (writerFile [3]*os.File, err error) { - return writerFile, ErrorNotImplemented -} - -// ReStart -func (o *onlyRedirectPath) ReStart() error { - return ErrorNotImplemented -} diff --git a/pkg/util/redirect.go b/pkg/util/redirect.go deleted file mode 100644 index 5c93399e93..0000000000 --- a/pkg/util/redirect.go +++ /dev/null @@ -1,36 +0,0 @@ -package util - -import ( - "os" - "sync" -) - -var ( - redirectMap = &RedirectStroe{rs: sync.Map{}} -) - -type RedirectStroe struct { - rs sync.Map -} - -func GetRedirectStrore() *RedirectStroe { - return redirectMap -} - -func (r *RedirectStroe) Store(key string, val *Pipe) { - r.rs.Store(key, val) -} - -func (r *RedirectStroe) Load(key string) (*Pipe, bool) { - val, ok := r.rs.Load(key) - if ok { - return val.(*Pipe), true - } - - return nil, false -} - -type Pipe struct { - Reader *os.File - Writer *os.File -} diff --git a/service/dap/server.go b/service/dap/server.go index 4a6fa6f001..3f8ba59458 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -1145,18 +1145,18 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { } if redirected { - redirector, err := NewRedirector() + redirects, err := NewRedirector() if err != nil { s.sendShowUserErrorResponse(request.Request, InternalError, "Internal Error", fmt.Sprintf("failed to generate stdio pipes - %v", err)) return } - s.config.Debugger.Redirect = redirector + s.config.Debugger.Redirect = redirects s.wg.Add(1) go func() { defer s.wg.Done() - if err = redirector.ReadRedirect("stdout", func(reader io.Reader) { + if err = ReadRedirect("stdout", redirects, func(reader io.Reader) { readerFunc(reader, "stdout") }); err != nil { s.sendShowUserErrorResponse(request.Request, InternalError, "Internal Error", @@ -1168,7 +1168,7 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { s.wg.Add(1) go func() { defer s.wg.Done() - if err = redirector.ReadRedirect("stderr", func(reader io.Reader) { + if err = ReadRedirect("stderr", redirects, func(reader io.Reader) { readerFunc(reader, "stderr") }); err != nil { s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", diff --git a/service/dap/stdio_pipe.go b/service/dap/stdio_pipe.go index 122425fd8a..8adb48dbde 100644 --- a/service/dap/stdio_pipe.go +++ b/service/dap/stdio_pipe.go @@ -11,18 +11,13 @@ import ( "path/filepath" "syscall" - "github.com/go-delve/delve/pkg/proc/redirect" + "github.com/go-delve/delve/pkg/proc" ) -type redirector struct { - stdoutPath string - stderrPath string -} - -func NewRedirector() (red *redirector, err error) { +func NewRedirector() (redirect proc.Redirect, err error) { r := make([]byte, 4) if _, err := rand.Read(r); err != nil { - return nil, err + return redirect, err } var ( @@ -32,45 +27,21 @@ func NewRedirector() (red *redirector, err error) { ) if err := syscall.Mkfifo(stdoutPath, 0o600); err != nil { - return nil, err + return redirect, err } if err := syscall.Mkfifo(stderrPath, 0o600); err != nil { _ = os.Remove(stdoutPath) - return nil, err + return redirect, err } - return &redirector{ - stdoutPath: stdoutPath, - stderrPath: stderrPath, - }, nil -} - -// RedirectFile -func (r *redirector) RedirectReaderFile() (readerFile [3]*os.File, err error) { - return readerFile, redirect.ErrorNotImplemented -} - -// RedirectWriterFile -func (r *redirector) RedirectWriterFile() (writerFile [3]*os.File, err error) { - return writerFile, redirect.ErrorNotImplemented -} - -// RedirectPath -func (r *redirector) RedirectPath() (redirects [3]string, err error) { - // TODO support stdin - return [3]string{"", r.stdoutPath, r.stderrPath}, nil -} - -// ReStart -func (r *redirector) ReStart() error { - panic("not implemented") // TODO: Implement + return proc.NewRedirectByPath([3]string{"", stdoutPath, stderrPath}), nil } -func (r *redirector) ReadRedirect(stdType string, f func(reader io.Reader)) error { - path := r.stderrPath - if stdType == "stdout" { - path = r.stdoutPath +func ReadRedirect(stdType string, redirect proc.Redirect, f func(reader io.Reader)) error { + path := redirect.Paths[1] + if stdType == "stderr" { + path = redirect.Paths[2] } defer os.Remove(path) diff --git a/service/dap/stdio_pipe_windows.go b/service/dap/stdio_pipe_windows.go index fec3a3a440..caa843b4e3 100644 --- a/service/dap/stdio_pipe_windows.go +++ b/service/dap/stdio_pipe_windows.go @@ -5,81 +5,36 @@ package dap import ( "crypto/rand" - "fmt" "io" "os" - "github.com/go-delve/delve/pkg/proc/redirect" - "github.com/go-delve/delve/pkg/util" + "github.com/go-delve/delve/pkg/proc" ) -type redirector struct { - stdoutWriterFile *os.File - stderrWriterFile *os.File - stdoutReaderFile *os.File - stderrReaderFile *os.File -} - -func NewRedirector() (red *redirector, err error) { +func NewRedirector() (redirect proc.Redirect, err error) { r := make([]byte, 4) if _, err := rand.Read(r); err != nil { - return nil, err + return redirect, err } stdoutReader, stdoutWriter, err := os.Pipe() if err != nil { - return nil, err + return redirect, err } stderrReader, stderrWriter, err := os.Pipe() if err != nil { - return nil, err - } - - return &redirector{ - stdoutWriterFile: stdoutWriter, - stderrWriterFile: stderrWriter, - stdoutReaderFile: stdoutReader, - stderrReaderFile: stderrReader, - }, nil - -} - -// RedirectPath -func (r *redirector) RedirectPath() (redirects [3]string, err error) { - return redirects, redirect.ErrorNotImplemented -} - -// RedirectFile -func (r *redirector) RedirectReaderFile() (readerFile [3]*os.File, err error) { - return [3]*os.File{nil, r.stdoutReaderFile, r.stderrReaderFile}, nil -} - -// RedirectWriterFile -func (r *redirector) RedirectWriterFile() (writerFile [3]*os.File, err error) { - return [3]*os.File{nil, r.stdoutWriterFile, r.stderrWriterFile}, nil -} - -// ReStart -func (r *redirector) ReStart() error { - panic("not implemented") // TODO: Implement -} - -func ReadRedirect(path string, f func(reader io.Reader)) error { - pipe, exist := util.GetRedirectStrore().Load(path) - if !exist { - return fmt.Errorf("redirect key(%s) not found", path) + return redirect, err } - f(pipe.Reader) - return nil + return proc.NewRedirectByFile([3]*os.File{nil, stdoutReader, stderrReader}, [3]*os.File{nil, stdoutWriter, stderrWriter}), nil } -func (r *redirector) ReadRedirect(stdType string, f func(reader io.Reader)) error { +func ReadRedirect(stdType string, redirect proc.Redirect, f func(reader io.Reader)) error { if stdType == "stdout" { - f(r.stdoutReaderFile) + f(redirect.ReaderFiles[1]) } else { - f(r.stderrReaderFile) + f(redirect.ReaderFiles[2]) } return nil } diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index f393d4ce03..572db0a856 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -30,7 +30,6 @@ import ( "github.com/go-delve/delve/pkg/proc/core" "github.com/go-delve/delve/pkg/proc/gdbserial" "github.com/go-delve/delve/pkg/proc/native" - "github.com/go-delve/delve/pkg/proc/redirect" "github.com/go-delve/delve/service/api" "github.com/sirupsen/logrus" ) @@ -139,10 +138,7 @@ type Config struct { ExecuteKind ExecuteKind // Redirects specifies redirect rules for stdin, stdout and stderr - Redirects [3]string - - // Redirects specifies redirect rules for stdin, stdout and stderr - Redirect redirect.Redirect + Redirect proc.Redirect // DisableASLR disables ASLR DisableASLR bool @@ -485,7 +481,7 @@ func (d *Debugger) Restart(rerecord bool, pos string, resetArgs bool, newArgs [] } if resetArgs { d.processArgs = append([]string{d.processArgs[0]}, newArgs...) - d.config.Redirects = newRedirects + d.config.Redirect = proc.NewRedirectByPath(newRedirects) } var p *proc.Target var err error From f433b232c8232a90358bdc9e57d4f9a80d3d7753 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Sun, 12 Feb 2023 03:03:37 +0800 Subject: [PATCH 16/53] wip: s.wg.wait before sending "terminated" --- service/dap/server.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/service/dap/server.go b/service/dap/server.go index 3f8ba59458..6d36c155cb 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -807,13 +807,6 @@ func (s *Session) handleRequest(request dap.Message) { } func (s *Session) send(message dap.Message) { - if event, ok := message.(*dap.OutputEvent); ok { - if event.GetEvent().Event == "terminated" { - // wait for all tasks(output redirects) to finish - s.wg.Wait() - } - } - jsonmsg, _ := json.Marshal(message) s.config.log.Debug("[-> to client]", string(jsonmsg)) // TODO(polina): consider using a channel for all the sends and to have a dedicated @@ -1275,15 +1268,17 @@ func (s *Session) onDisconnectRequest(request *dap.DisconnectRequest) { defer s.mu.Unlock() if s.debugger != nil && s.config.AcceptMulti && !request.Arguments.TerminateDebuggee { - // This is a multi-use server/debugger, so a disconnect request that doesn't + // this is a multi-use server/debugger, so a disconnect request that doesn't // terminate the debuggee should clean up only the client connection and pointer to debugger, // but not the entire server. status := "halted" if s.isRunningCmd() { status = "running" - } else if s, err := s.debugger.State(false); processExited(s, err) { + } else if state, err := s.debugger.State(false); processExited(state, err) { status = "exited" + s.wg.Wait() } + s.logToConsole(fmt.Sprintf("Closing client session, but leaving multi-client DAP server at %s with debuggee %s", s.config.Listener.Addr().String(), status)) s.send(&dap.DisconnectResponse{Response: *newResponse(request.Request)}) s.send(&dap.TerminatedEvent{Event: *newEvent("terminated")}) @@ -1317,6 +1312,7 @@ func (s *Session) onDisconnectRequest(request *dap.DisconnectRequest) { } else { s.send(&dap.DisconnectResponse{Response: *newResponse(request.Request)}) } + s.wg.Wait() // The debugging session has ended, so we send a terminated event. s.send(&dap.TerminatedEvent{Event: *newEvent("terminated")}) } @@ -2876,6 +2872,7 @@ func (s *Session) doCall(goid, frame int, expr string) (*api.DebuggerState, []*p GoroutineID: int64(goid), }, nil) if processExited(state, err) { + s.wg.Wait() e := &dap.TerminatedEvent{Event: *newEvent("terminated")} s.send(e) return nil, nil, errors.New("terminated") From ffad8a4421ddc8e9259b3ba93a0bef5bd02fe8db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Sat, 18 Feb 2023 04:26:06 +0800 Subject: [PATCH 17/53] wip: add proc/redirect test(darwin and linux) --- _fixtures/out_redirect-stderr.txt | 2 + _fixtures/out_redirect-stdout.txt | 2 + _fixtures/out_redirect.go | 13 +++ pkg/proc/proc_test.go | 149 +++++++++++++++++++++++++++++- service/dap/server.go | 1 + service/test/integration2_test.go | 3 +- 6 files changed, 164 insertions(+), 6 deletions(-) create mode 100644 _fixtures/out_redirect-stderr.txt create mode 100644 _fixtures/out_redirect-stdout.txt create mode 100644 _fixtures/out_redirect.go diff --git a/_fixtures/out_redirect-stderr.txt b/_fixtures/out_redirect-stderr.txt new file mode 100644 index 0000000000..9b79af9385 --- /dev/null +++ b/_fixtures/out_redirect-stderr.txt @@ -0,0 +1,2 @@ +hello world! +hello world! error! diff --git a/_fixtures/out_redirect-stdout.txt b/_fixtures/out_redirect-stdout.txt new file mode 100644 index 0000000000..9f0dc0f034 --- /dev/null +++ b/_fixtures/out_redirect-stdout.txt @@ -0,0 +1,2 @@ +hello world! +hello world! diff --git a/_fixtures/out_redirect.go b/_fixtures/out_redirect.go new file mode 100644 index 0000000000..faa48e926f --- /dev/null +++ b/_fixtures/out_redirect.go @@ -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!") +} diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 5ee67eee30..15caf4fdef 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -8,6 +8,7 @@ import ( "go/ast" "go/constant" "go/token" + "io" "io/ioutil" "math/rand" "net" @@ -20,6 +21,7 @@ import ( "sort" "strconv" "strings" + "syscall" "testing" "text/tabwriter" "time" @@ -45,6 +47,82 @@ func init() { os.Setenv("GOMAXPROCS", "4") } +func TestRedirect(t *testing.T) { + fixture := protest.BuildFixture("out_redirect", 0) + var ( + p *proc.Target + tracedir string + err error + redirect proc.Redirect = proc.NewEmptyRedirectByPath() + errChan = make(chan error, 2) + canceFunc func() + needCheck = false + stdoutExpectFile = "out_redirect-stdout.txt" + stderrExpectFile = "out_redirect-stderr.txt" + ) + switch testBackend { + case "native": + if runtime.GOOS == "linux" { + redirect, canceFunc, err = testGenRedireByPath(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + if err != nil { + break + } + needCheck = true + } else if runtime.GOOS == "windows" { + // TODO toad + } + + p, err = native.Launch([]string{fixture.Path}, ".", 0, []string{}, "", redirect) + case "lldb": + if runtime.GOOS == "darwin" { + redirect, canceFunc, err = testGenRedireByPath(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + if err != nil { + break + } + needCheck = true + } + p, err = gdbserial.LLDBLaunch([]string{fixture.Path}, ".", 0, []string{}, "", redirect) + case "rr": + protest.MustHaveRecordingAllowed(t) + t.Log("recording") + if runtime.GOOS != "windows" { + redirect, canceFunc, err = testGenRedireByPath(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + if err != nil { + break + } + needCheck = true + } + p, tracedir, err = gdbserial.RecordAndReplay([]string{fixture.Path}, ".", true, []string{}, redirect) + t.Logf("replaying %q", tracedir) + default: + t.Fatal("unknown backend") + } + if err != nil { + // clear reader goroutine + if canceFunc != nil { + canceFunc() + _, _ = <-errChan, <-errChan + } + t.Fatal("Launch():", err) + } + _ = p.Continue() + + if needCheck { + err1 := <-errChan + err2 := <-errChan + if err1 != nil { + t.Fatal("checkOut():", err1) + } + if err2 != nil { + t.Fatal("checkOut():", err2) + } + } + + defer func() { + p.Detach(true) + }() +} + func TestMain(m *testing.M) { flag.StringVar(&testBackend, "backend", "", "selects backend") flag.StringVar(&buildMode, "test-buildmode", "", "selects build mode") @@ -103,13 +181,13 @@ func withTestProcessArgs(name string, t testing.TB, wd string, args []string, bu switch testBackend { case "native": - p, err = native.Launch(append([]string{fixture.Path}, args...), wd, 0, []string{}, "", [3]string{}) + p, err = native.Launch(append([]string{fixture.Path}, args...), wd, 0, []string{}, "", proc.NewEmptyRedirectByPath()) case "lldb": - p, err = gdbserial.LLDBLaunch(append([]string{fixture.Path}, args...), wd, 0, []string{}, "", [3]string{}) + p, err = gdbserial.LLDBLaunch(append([]string{fixture.Path}, args...), wd, 0, []string{}, "", proc.NewEmptyRedirectByPath()) case "rr": protest.MustHaveRecordingAllowed(t) t.Log("recording") - p, tracedir, err = gdbserial.RecordAndReplay(append([]string{fixture.Path}, args...), wd, true, []string{}, [3]string{}) + p, tracedir, err = gdbserial.RecordAndReplay(append([]string{fixture.Path}, args...), wd, true, []string{}, proc.NewEmptyRedirectByPath()) t.Logf("replaying %q", tracedir) default: t.Fatal("unknown backend") @@ -2219,9 +2297,9 @@ func TestUnsupportedArch(t *testing.T) { switch testBackend { case "native": - p, err = native.Launch([]string{outfile}, ".", 0, []string{}, "", [3]string{}) + p, err = native.Launch([]string{outfile}, ".", 0, []string{}, "", proc.NewEmptyRedirectByPath()) case "lldb": - p, err = gdbserial.LLDBLaunch([]string{outfile}, ".", 0, []string{}, "", [3]string{}) + p, err = gdbserial.LLDBLaunch([]string{outfile}, ".", 0, []string{}, "", proc.NewEmptyRedirectByPath()) default: t.Skip("test not valid for this backend") } @@ -6024,3 +6102,64 @@ func TestStacktraceExtlinkMac(t *testing.T) { } }) } + +func testGenRedireByPath(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { + var ( + stdoutPath = "./stdout" + stderrPath = "./stderr" + ) + + if err = syscall.Mkfifo(stdoutPath, 0o600); err != nil { + return redirect, nil, err + } + + if err = syscall.Mkfifo(stderrPath, 0o600); err != nil { + os.Remove(stdoutPath) + return redirect, nil, err + } + + redirect = proc.NewRedirectByPath([3]string{"", stdoutPath, stderrPath}) + reader := func(mode string, path string, expectFile string) { + defer os.Remove(path) + fmt.Println(path) + outFile, err := os.OpenFile(path, os.O_RDONLY, os.ModeNamedPipe) + if err != nil { + errChan <- err + return + } + + expect, err := os.ReadFile(filepath.Join(fixture.BuildDir, expectFile)) + if err != nil { + errChan <- err + return + } + + out, err := io.ReadAll(outFile) + if err != nil { + errChan <- err + return + } + + if string(expect) == string(out) { + errChan <- fmt.Errorf("%s,Not as expected!\nexpect:%s\nout:%s", mode, expect, out) + return + } + errChan <- nil + } + + canceFunc = func() { + stdout, err := os.OpenFile(stdoutPath, os.O_WRONLY, os.ModeNamedPipe) + if err == nil { + stdout.Close() + } + stderr, err := os.OpenFile(stderrPath, os.O_WRONLY, os.ModeNamedPipe) + if err == nil { + stderr.Close() + } + } + + go reader("stdout", stdoutPath, stdoutExpectFile) + go reader("stderr", stderrPath, stderrExpectFile) + + return redirect, canceFunc, nil +} diff --git a/service/dap/server.go b/service/dap/server.go index 6d36c155cb..44b2572249 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -3617,6 +3617,7 @@ func (s *Session) runUntilStopAndNotify(command string, allowNextStateChange cha } if processExited(state, err) { + s.wg.Wait() s.send(&dap.TerminatedEvent{Event: *newEvent("terminated")}) return } diff --git a/service/test/integration2_test.go b/service/test/integration2_test.go index 5550f7a3c0..181b4ca0da 100644 --- a/service/test/integration2_test.go +++ b/service/test/integration2_test.go @@ -18,6 +18,7 @@ import ( "testing" "time" + "github.com/go-delve/delve/pkg/proc" protest "github.com/go-delve/delve/pkg/proc/test" "github.com/go-delve/delve/service/debugger" @@ -84,7 +85,7 @@ func startServer(name string, buildFlags protest.BuildFlags, t *testing.T, redir Packages: []string{fixture.Source}, BuildFlags: "", // build flags can be an empty string here because the only test that uses it, does not set special flags. ExecuteKind: debugger.ExecutingGeneratedFile, - Redirects: redirects, + Redirect: proc.NewRedirectByPath(redirects), }, }) if err := server.Run(); err != nil { From 70216a530c20a63cf9a4ca5f38d79ffc7c683935 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Mon, 20 Feb 2023 02:53:19 +0800 Subject: [PATCH 18/53] Merge branch 'master' of github.com:tttoad/delve into feat-console --- pkg/proc/native/proc_freebsd.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/proc/native/proc_freebsd.go b/pkg/proc/native/proc_freebsd.go index 44fbc6b6eb..1e1aff414a 100644 --- a/pkg/proc/native/proc_freebsd.go +++ b/pkg/proc/native/proc_freebsd.go @@ -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.Target, error) { +func Launch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []string, tty string, redirects proc.Redirects) (*proc.Target, error) { var ( process *exec.Cmd err error From 902235dce5c03ec0db80decf777ece8e289ea21e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Tue, 21 Feb 2023 00:21:38 +0800 Subject: [PATCH 19/53] wip: Fix test failure on windows --- pkg/proc/proc_test.go | 189 ++++++++++-------------------- pkg/proc/proc_unix_test.go | 66 +++++++++++ pkg/proc/proc_windows_test.go | 62 ++++++++++ pkg/proc/redirect.go | 4 + service/dap/stdio_pipe_windows.go | 5 - 5 files changed, 197 insertions(+), 129 deletions(-) create mode 100644 pkg/proc/proc_windows_test.go diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 39e48d871e..95d2737932 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -8,7 +8,6 @@ import ( "go/ast" "go/constant" "go/token" - "io" "io/ioutil" "math/rand" "net" @@ -21,7 +20,6 @@ import ( "sort" "strconv" "strings" - "syscall" "testing" "text/tabwriter" "time" @@ -47,82 +45,6 @@ func init() { os.Setenv("GOMAXPROCS", "4") } -func TestRedirect(t *testing.T) { - fixture := protest.BuildFixture("out_redirect", 0) - var ( - p *proc.Target - tracedir string - err error - redirect proc.Redirect = proc.NewEmptyRedirectByPath() - errChan = make(chan error, 2) - canceFunc func() - needCheck = false - stdoutExpectFile = "out_redirect-stdout.txt" - stderrExpectFile = "out_redirect-stderr.txt" - ) - switch testBackend { - case "native": - if runtime.GOOS == "linux" { - redirect, canceFunc, err = testGenRedireByPath(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) - if err != nil { - break - } - needCheck = true - } else if runtime.GOOS == "windows" { - // TODO toad - } - - p, err = native.Launch([]string{fixture.Path}, ".", 0, []string{}, "", redirect) - case "lldb": - if runtime.GOOS == "darwin" { - redirect, canceFunc, err = testGenRedireByPath(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) - if err != nil { - break - } - needCheck = true - } - p, err = gdbserial.LLDBLaunch([]string{fixture.Path}, ".", 0, []string{}, "", redirect) - case "rr": - protest.MustHaveRecordingAllowed(t) - t.Log("recording") - if runtime.GOOS != "windows" { - redirect, canceFunc, err = testGenRedireByPath(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) - if err != nil { - break - } - needCheck = true - } - p, tracedir, err = gdbserial.RecordAndReplay([]string{fixture.Path}, ".", true, []string{}, redirect) - t.Logf("replaying %q", tracedir) - default: - t.Fatal("unknown backend") - } - if err != nil { - // clear reader goroutine - if canceFunc != nil { - canceFunc() - _, _ = <-errChan, <-errChan - } - t.Fatal("Launch():", err) - } - _ = p.Continue() - - if needCheck { - err1 := <-errChan - err2 := <-errChan - if err1 != nil { - t.Fatal("checkOut():", err1) - } - if err2 != nil { - t.Fatal("checkOut():", err2) - } - } - - defer func() { - p.Detach(true) - }() -} - func TestMain(m *testing.M) { flag.StringVar(&testBackend, "backend", "", "selects backend") flag.StringVar(&buildMode, "test-buildmode", "", "selects build mode") @@ -6025,63 +5947,82 @@ func TestStacktraceExtlinkMac(t *testing.T) { }) } -func testGenRedireByPath(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { +func TestRedirect(t *testing.T) { + fixture := protest.BuildFixture("out_redirect", 0) var ( - stdoutPath = "./stdout" - stderrPath = "./stderr" + p *proc.Target + tracedir string + err error + redirect proc.Redirect = proc.NewEmptyRedirectByPath() + errChan = make(chan error, 2) + canceFunc func() + needCheck = false + stdoutExpectFile = "out_redirect-stdout.txt" + stderrExpectFile = "out_redirect-stderr.txt" ) - - if err = syscall.Mkfifo(stdoutPath, 0o600); err != nil { - return redirect, nil, err - } - - if err = syscall.Mkfifo(stderrPath, 0o600); err != nil { - os.Remove(stdoutPath) - return redirect, nil, err - } - - redirect = proc.NewRedirectByPath([3]string{"", stdoutPath, stderrPath}) - reader := func(mode string, path string, expectFile string) { - defer os.Remove(path) - fmt.Println(path) - outFile, err := os.OpenFile(path, os.O_RDONLY, os.ModeNamedPipe) - if err != nil { - errChan <- err - return + switch testBackend { + case "native": + if runtime.GOOS == "linux" { + redirect, canceFunc, err = testGenRedireByPath(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + if err != nil { + break + } + needCheck = true + } else if runtime.GOOS == "windows" { + redirect, canceFunc, err = testGenRedireByFile(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + if err != nil { + break + } + needCheck = true } - expect, err := os.ReadFile(filepath.Join(fixture.BuildDir, expectFile)) - if err != nil { - errChan <- err - return + p, err = native.Launch([]string{fixture.Path}, ".", 0, []string{}, "", redirect) + case "lldb": + if runtime.GOOS == "darwin" { + redirect, canceFunc, err = testGenRedireByPath(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + if err != nil { + break + } + needCheck = true } - - out, err := io.ReadAll(outFile) - if err != nil { - errChan <- err - return + p, err = gdbserial.LLDBLaunch([]string{fixture.Path}, ".", 0, []string{}, "", redirect) + case "rr": + protest.MustHaveRecordingAllowed(t) + t.Log("recording") + if runtime.GOOS != "windows" { + redirect, canceFunc, err = testGenRedireByPath(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + if err != nil { + break + } + needCheck = true } - - if string(expect) == string(out) { - errChan <- fmt.Errorf("%s,Not as expected!\nexpect:%s\nout:%s", mode, expect, out) - return + p, tracedir, err = gdbserial.RecordAndReplay([]string{fixture.Path}, ".", true, []string{}, redirect) + t.Logf("replaying %q", tracedir) + default: + t.Fatal("unknown backend") + } + if err != nil { + // clear reader goroutine + if canceFunc != nil { + canceFunc() + _, _ = <-errChan, <-errChan } - errChan <- nil + t.Fatal("Launch():", err) } + _ = p.Continue() - canceFunc = func() { - stdout, err := os.OpenFile(stdoutPath, os.O_WRONLY, os.ModeNamedPipe) - if err == nil { - stdout.Close() + if needCheck { + err1 := <-errChan + err2 := <-errChan + if err1 != nil { + t.Fatal("checkOut():", err1) } - stderr, err := os.OpenFile(stderrPath, os.O_WRONLY, os.ModeNamedPipe) - if err == nil { - stderr.Close() + if err2 != nil { + t.Fatal("checkOut():", err2) } } - go reader("stdout", stdoutPath, stdoutExpectFile) - go reader("stderr", stderrPath, stderrExpectFile) - - return redirect, canceFunc, nil + defer func() { + p.Detach(true) + }() } diff --git a/pkg/proc/proc_unix_test.go b/pkg/proc/proc_unix_test.go index fa9ac0ae00..f5ae9b17e8 100644 --- a/pkg/proc/proc_unix_test.go +++ b/pkg/proc/proc_unix_test.go @@ -5,8 +5,10 @@ package proc_test import ( "fmt" + "io" "os" "os/exec" + "path/filepath" "runtime" "syscall" "testing" @@ -101,3 +103,67 @@ func TestSignalDeath(t *testing.T) { t.Fatalf("expected SIGPIPE got %d\n", exitErr.Status) } } + +func testGenRedireByPath(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { + var ( + stdoutPath = "./stdout" + stderrPath = "./stderr" + ) + + if err = syscall.Mkfifo(stdoutPath, 0o600); err != nil { + return redirect, nil, err + } + + if err = syscall.Mkfifo(stderrPath, 0o600); err != nil { + os.Remove(stdoutPath) + return redirect, nil, err + } + + redirect = proc.NewRedirectByPath([3]string{"", stdoutPath, stderrPath}) + reader := func(mode string, path string, expectFile string) { + defer os.Remove(path) + outFile, err := os.OpenFile(path, os.O_RDONLY, os.ModeNamedPipe) + if err != nil { + errChan <- err + return + } + + expect, err := os.ReadFile(filepath.Join(fixture.BuildDir, expectFile)) + if err != nil { + errChan <- err + return + } + + out, err := io.ReadAll(outFile) + if err != nil { + errChan <- err + return + } + + if string(expect) == string(out) { + errChan <- fmt.Errorf("%s,Not as expected!\nexpect:%s\nout:%s", mode, expect, out) + return + } + errChan <- nil + } + + canceFunc = func() { + stdout, err := os.OpenFile(stdoutPath, os.O_WRONLY, os.ModeNamedPipe) + if err == nil { + stdout.Close() + } + stderr, err := os.OpenFile(stderrPath, os.O_WRONLY, os.ModeNamedPipe) + if err == nil { + stderr.Close() + } + } + + go reader("stdout", stdoutPath, stdoutExpectFile) + go reader("stderr", stderrPath, stderrExpectFile) + + return redirect, canceFunc, nil +} + +func testGenRedireByFile(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { + return proc.NewEmptyRedirectByFile(), nil, nil +} diff --git a/pkg/proc/proc_windows_test.go b/pkg/proc/proc_windows_test.go new file mode 100644 index 0000000000..22ba46483c --- /dev/null +++ b/pkg/proc/proc_windows_test.go @@ -0,0 +1,62 @@ +//go:build windows +// +build windows + +package proc_test + +import ( + "fmt" + "io" + "os" + "path/filepath" + "testing" + + "github.com/go-delve/delve/pkg/proc" + protest "github.com/go-delve/delve/pkg/proc/test" +) + +func testGenRedireByFile(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { + stdoutReader, stdoutWriter, err := os.Pipe() + if err != nil { + return redirect, nil, err + } + + stderrReader, stderrWriter, err := os.Pipe() + if err != nil { + return redirect, nil, err + } + + redirect = proc.NewRedirectByFile([3]*os.File{nil, stderrReader, stdoutReader}, [3]*os.File{nil, stdoutWriter, stderrWriter}) + reader := func(mode string, f *os.File, expectFile string) { + expect, err := os.ReadFile(filepath.Join(fixture.BuildDir, expectFile)) + if err != nil { + errChan <- err + return + } + + out, err := io.ReadAll(f) + if err != nil { + errChan <- err + return + } + + if string(expect) == string(out) { + errChan <- fmt.Errorf("%s,Not as expected!\nexpect:%s\nout:%s", mode, expect, out) + return + } + errChan <- nil + } + + canceFunc = func() { + _ = stdoutWriter.Close() + _ = stderrWriter.Close() + } + + go reader("stdout", stdoutReader, stdoutExpectFile) + go reader("stderr", stderrReader, stderrExpectFile) + + return redirect, canceFunc, nil +} + +func testGenRedireByPath(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { + return proc.NewEmptyRedirectByPath(), nil, nil +} diff --git a/pkg/proc/redirect.go b/pkg/proc/redirect.go index fb7df62e0c..fccd27e217 100644 --- a/pkg/proc/redirect.go +++ b/pkg/proc/redirect.go @@ -20,6 +20,10 @@ func NewEmptyRedirectByPath() Redirect { return Redirect{Mode: RedirectPathMode} } +func NewEmptyRedirectByFile() Redirect { + return Redirect{Mode: RedirectFileMode} +} + func NewRedirectByPath(paths [3]string) Redirect { return Redirect{Paths: paths, Mode: RedirectPathMode} } diff --git a/service/dap/stdio_pipe_windows.go b/service/dap/stdio_pipe_windows.go index caa843b4e3..7f23e5d899 100644 --- a/service/dap/stdio_pipe_windows.go +++ b/service/dap/stdio_pipe_windows.go @@ -12,11 +12,6 @@ import ( ) func NewRedirector() (redirect proc.Redirect, err error) { - r := make([]byte, 4) - if _, err := rand.Read(r); err != nil { - return redirect, err - } - stdoutReader, stdoutWriter, err := os.Pipe() if err != nil { return redirect, err From 9e08369d6a6753fd0309950e409e8d38e0bf413a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Tue, 21 Feb 2023 02:18:52 +0800 Subject: [PATCH 20/53] fix: undefined: proc.Redirects --- pkg/proc/native/proc_darwin.go | 2 +- pkg/proc/native/proc_freebsd.go | 2 +- pkg/proc/proc_linux_test.go | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/proc/native/proc_darwin.go b/pkg/proc/native/proc_darwin.go index fa0f755895..ed69debd25 100644 --- a/pkg/proc/native/proc_darwin.go +++ b/pkg/proc/native/proc_darwin.go @@ -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.Target, error) { +func Launch(cmd []string, wd string, flags proc.LaunchFlags, _ []string, _ string, _ proc.Redirect) (*proc.Target, error) { argv0Go, err := filepath.Abs(cmd[0]) if err != nil { return nil, err diff --git a/pkg/proc/native/proc_freebsd.go b/pkg/proc/native/proc_freebsd.go index 1e1aff414a..38a16070c8 100644 --- a/pkg/proc/native/proc_freebsd.go +++ b/pkg/proc/native/proc_freebsd.go @@ -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 proc.Redirects) (*proc.Target, error) { +func Launch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []string, tty string, redirects proc.Redirect) (*proc.Target, error) { var ( process *exec.Cmd err error diff --git a/pkg/proc/proc_linux_test.go b/pkg/proc/proc_linux_test.go index d10e17ff4c..db82b7f972 100644 --- a/pkg/proc/proc_linux_test.go +++ b/pkg/proc/proc_linux_test.go @@ -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" ) @@ -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.NewEmptyRedirectByPath()) if err != nil { t.Fatal(err) } From 0b3c0663055fc7d00adc294287565657f5861802 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Tue, 21 Feb 2023 02:38:21 +0800 Subject: [PATCH 21/53] fix: compile failure --- pkg/proc/proc_not_windows_test.go | 68 +++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 pkg/proc/proc_not_windows_test.go diff --git a/pkg/proc/proc_not_windows_test.go b/pkg/proc/proc_not_windows_test.go new file mode 100644 index 0000000000..d1964f1a4f --- /dev/null +++ b/pkg/proc/proc_not_windows_test.go @@ -0,0 +1,68 @@ +//go:build !windows +// +build !windows + +package proc_test + +func testGenRedireByPath(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { + var ( + stdoutPath = "./stdout" + stderrPath = "./stderr" + ) + + if err = syscall.Mkfifo(stdoutPath, 0o600); err != nil { + return redirect, nil, err + } + + if err = syscall.Mkfifo(stderrPath, 0o600); err != nil { + os.Remove(stdoutPath) + return redirect, nil, err + } + + redirect = proc.NewRedirectByPath([3]string{"", stdoutPath, stderrPath}) + reader := func(mode string, path string, expectFile string) { + defer os.Remove(path) + outFile, err := os.OpenFile(path, os.O_RDONLY, os.ModeNamedPipe) + if err != nil { + errChan <- err + return + } + + expect, err := os.ReadFile(filepath.Join(fixture.BuildDir, expectFile)) + if err != nil { + errChan <- err + return + } + + out, err := io.ReadAll(outFile) + if err != nil { + errChan <- err + return + } + + if string(expect) == string(out) { + errChan <- fmt.Errorf("%s,Not as expected!\nexpect:%s\nout:%s", mode, expect, out) + return + } + errChan <- nil + } + + canceFunc = func() { + stdout, err := os.OpenFile(stdoutPath, os.O_WRONLY, os.ModeNamedPipe) + if err == nil { + stdout.Close() + } + stderr, err := os.OpenFile(stderrPath, os.O_WRONLY, os.ModeNamedPipe) + if err == nil { + stderr.Close() + } + } + + go reader("stdout", stdoutPath, stdoutExpectFile) + go reader("stderr", stderrPath, stderrExpectFile) + + return redirect, canceFunc, nil +} + +func testGenRedireByFile(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { + return proc.NewEmptyRedirectByFile(), nil, nil +} From 38958bc035d51357228c0b56d5f9e3ac41a5e703 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Tue, 21 Feb 2023 02:48:16 +0800 Subject: [PATCH 22/53] wip: Remove useless code --- pkg/proc/proc_unix_test.go | 66 -------------------------------------- 1 file changed, 66 deletions(-) diff --git a/pkg/proc/proc_unix_test.go b/pkg/proc/proc_unix_test.go index f5ae9b17e8..fa9ac0ae00 100644 --- a/pkg/proc/proc_unix_test.go +++ b/pkg/proc/proc_unix_test.go @@ -5,10 +5,8 @@ package proc_test import ( "fmt" - "io" "os" "os/exec" - "path/filepath" "runtime" "syscall" "testing" @@ -103,67 +101,3 @@ func TestSignalDeath(t *testing.T) { t.Fatalf("expected SIGPIPE got %d\n", exitErr.Status) } } - -func testGenRedireByPath(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { - var ( - stdoutPath = "./stdout" - stderrPath = "./stderr" - ) - - if err = syscall.Mkfifo(stdoutPath, 0o600); err != nil { - return redirect, nil, err - } - - if err = syscall.Mkfifo(stderrPath, 0o600); err != nil { - os.Remove(stdoutPath) - return redirect, nil, err - } - - redirect = proc.NewRedirectByPath([3]string{"", stdoutPath, stderrPath}) - reader := func(mode string, path string, expectFile string) { - defer os.Remove(path) - outFile, err := os.OpenFile(path, os.O_RDONLY, os.ModeNamedPipe) - if err != nil { - errChan <- err - return - } - - expect, err := os.ReadFile(filepath.Join(fixture.BuildDir, expectFile)) - if err != nil { - errChan <- err - return - } - - out, err := io.ReadAll(outFile) - if err != nil { - errChan <- err - return - } - - if string(expect) == string(out) { - errChan <- fmt.Errorf("%s,Not as expected!\nexpect:%s\nout:%s", mode, expect, out) - return - } - errChan <- nil - } - - canceFunc = func() { - stdout, err := os.OpenFile(stdoutPath, os.O_WRONLY, os.ModeNamedPipe) - if err == nil { - stdout.Close() - } - stderr, err := os.OpenFile(stderrPath, os.O_WRONLY, os.ModeNamedPipe) - if err == nil { - stderr.Close() - } - } - - go reader("stdout", stdoutPath, stdoutExpectFile) - go reader("stderr", stderrPath, stderrExpectFile) - - return redirect, canceFunc, nil -} - -func testGenRedireByFile(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { - return proc.NewEmptyRedirectByFile(), nil, nil -} From fc898820f54a9bac68b79ec1102551d1bf685a10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Thu, 23 Feb 2023 14:42:03 +0800 Subject: [PATCH 23/53] fix: filename error --- ...c_not_windows_test.go => proc_notwindows_test.go} | 12 ++++++++++++ 1 file changed, 12 insertions(+) rename pkg/proc/{proc_not_windows_test.go => proc_notwindows_test.go} (91%) diff --git a/pkg/proc/proc_not_windows_test.go b/pkg/proc/proc_notwindows_test.go similarity index 91% rename from pkg/proc/proc_not_windows_test.go rename to pkg/proc/proc_notwindows_test.go index d1964f1a4f..c63e24d008 100644 --- a/pkg/proc/proc_not_windows_test.go +++ b/pkg/proc/proc_notwindows_test.go @@ -3,6 +3,18 @@ package proc_test +import ( + "fmt" + "io" + "os" + "path/filepath" + "syscall" + "testing" + + "github.com/go-delve/delve/pkg/proc" + protest "github.com/go-delve/delve/pkg/proc/test" +) + func testGenRedireByPath(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { var ( stdoutPath = "./stdout" From 359ee2c20c1e0f2946c749523105c8d6e47008cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Sun, 26 Feb 2023 23:13:08 +0800 Subject: [PATCH 24/53] fix: os.file not close --- pkg/proc/native/proc.go | 1 + service/dap/stdio_pipe_windows.go | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go index 812056a1c1..f91e434f2b 100644 --- a/pkg/proc/native/proc.go +++ b/pkg/proc/native/proc.go @@ -388,6 +388,7 @@ func openRedirects(redirects proc.Redirect, foreground bool) (stdin, stdout, std } stdout = redirects.WriterFiles[1] stderr = redirects.WriterFiles[2] + toclose = append(toclose, stdin, stderr) return stdin, stdout, stderr, closefn, nil } diff --git a/service/dap/stdio_pipe_windows.go b/service/dap/stdio_pipe_windows.go index 7f23e5d899..66e51aa779 100644 --- a/service/dap/stdio_pipe_windows.go +++ b/service/dap/stdio_pipe_windows.go @@ -4,7 +4,6 @@ package dap import ( - "crypto/rand" "io" "os" From ad0dec1f268691e302f101057417ae45734816d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Thu, 2 Mar 2023 00:32:40 +0800 Subject: [PATCH 25/53] test: add server_test.redirect --- pkg/proc/proc_windows_test.go | 2 +- service/dap/server_test.go | 106 ++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 1 deletion(-) diff --git a/pkg/proc/proc_windows_test.go b/pkg/proc/proc_windows_test.go index 22ba46483c..fb74da5f0d 100644 --- a/pkg/proc/proc_windows_test.go +++ b/pkg/proc/proc_windows_test.go @@ -39,7 +39,7 @@ func testGenRedireByFile(t *testing.T, fixture protest.Fixture, stdoutExpectFile return } - if string(expect) == string(out) { + if string(expect) != string(out) { errChan <- fmt.Errorf("%s,Not as expected!\nexpect:%s\nout:%s", mode, expect, out) return } diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 8b775487c4..e04ec365bc 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -2,6 +2,7 @@ package dap import ( "bufio" + "bytes" "flag" "fmt" "io" @@ -7367,3 +7368,108 @@ func TestDisassembleCgo(t *testing.T) { }, protest.AllNonOptimized, true) } + +func TestRedirect(t *testing.T) { + runTest(t, "out_redirect", func(client *daptest.Client, fixture protest.Fixture) { + // 1 >> initialize, << initialize + client.InitializeRequest() + initResp := client.ExpectInitializeResponseAndCapabilities(t) + if initResp.Seq != 0 || initResp.RequestSeq != 1 { + t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=1", initResp) + } + + // 2 >> launch, << initialized, << launch + client.LaunchRequestWithArgs(map[string]interface{}{ + "request": "launch", + "mode": "debug", + "program": fixture.Path, + "stopOnEntry": stopOnEntry, + "outputMode": "only-remote", + }) + initEvent := client.ExpectInitializedEvent(t) + if initEvent.Seq != 0 { + t.Errorf("\ngot %#v\nwant Seq=0", initEvent) + } + launchResp := client.ExpectLaunchResponse(t) + if launchResp.Seq != 0 || launchResp.RequestSeq != 2 { + t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=2", launchResp) + } + + // 5 >> configurationDone, << stopped, << configurationDone + client.ConfigurationDoneRequest() + + stopEvent := client.ExpectStoppedEvent(t) + if stopEvent.Seq != 0 || + stopEvent.Body.Reason != "entry" || + stopEvent.Body.ThreadId != 1 || + !stopEvent.Body.AllThreadsStopped { + t.Errorf("\ngot %#v\nwant Seq=0, Body={Reason=\"entry\", ThreadId=1, AllThreadsStopped=true}", stopEvent) + } + cdResp := client.ExpectConfigurationDoneResponse(t) + if cdResp.Seq != 0 || cdResp.RequestSeq != 5 { + t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=5", cdResp) + } + + // wait for output and terminated + var ( + stdout = bytes.NewBufferString("") + stderr = bytes.NewBufferString("") + ) + for hasNext := true; hasNext; { + message := client.ExpectMessage(t) + switch m := message.(type) { + case *dap.OutputEvent: + switch m.Body.Category { + case "stdout": + stdout.WriteString(m.Body.Output) + case "stderr": + stderr.WriteString(m.Body.Output) + default: + t.Errorf("\ngot %#v\nwant Category='stdout' or 'stderr'", m) + } + case *dap.TerminatedEvent: + hasNext = false + default: + t.Errorf("\n got %#v, want *dap.OutputEvent or *dap.TerminateResponse", m) + } + } + var ( + stdoutFilePath = filepath.Join(fixture.Path, "out_redirect-stdout.txt") + stderrFilePath = filepath.Join(fixture.Path, "out_redirect-stderr.txt") + ) + // check output + expectStdout, err := os.ReadFile(stdoutFilePath) + if err != nil { + t.Errorf("\n failed to read file: %s", stdoutFilePath) + } + + if string(expectStdout) != stdout.String() { + t.Errorf("\n got stdout:\n%s\nwant:%s", stdout.String(), string(expectStdout)) + } + + expectStderr, err := os.ReadFile(stderrFilePath) + if err != nil { + t.Errorf("\n failed to read file: %s", stderrFilePath) + } + + if string(expectStderr) != stderr.String() { + t.Errorf("\n got stderr:\n%s\nwant:%s", stderr.String(), string(expectStderr)) + } + + // 7 >> disconnect, << disconnect + client.DisconnectRequest() + oep := client.ExpectOutputEventProcessExited(t, 0) + if oep.Seq != 0 || oep.Body.Category != "console" { + t.Errorf("\ngot %#v\nwant Seq=0 Category='console'", oep) + } + oed := client.ExpectOutputEventDetaching(t) + if oed.Seq != 0 || oed.Body.Category != "console" { + t.Errorf("\ngot %#v\nwant Seq=0 Category='console'", oed) + } + dResp := client.ExpectDisconnectResponse(t) + if dResp.Seq != 0 || dResp.RequestSeq != 13 { + t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=13", dResp) + } + client.ExpectTerminatedEvent(t) + }) +} From 73c70762530ec30186fcca25a6d149ad71fac2dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Fri, 3 Mar 2023 00:57:06 +0800 Subject: [PATCH 26/53] fix: Remove 'eol' from end of file --- _fixtures/out_redirect-stderr.txt | 2 +- _fixtures/out_redirect-stdout.txt | 2 +- pkg/proc/proc_notwindows_test.go | 2 +- service/dap/server_test.go | 33 ++++++++++++------------------- 4 files changed, 16 insertions(+), 23 deletions(-) diff --git a/_fixtures/out_redirect-stderr.txt b/_fixtures/out_redirect-stderr.txt index 9b79af9385..bd9b413af3 100644 --- a/_fixtures/out_redirect-stderr.txt +++ b/_fixtures/out_redirect-stderr.txt @@ -1,2 +1,2 @@ hello world! -hello world! error! +hello world! error! \ No newline at end of file diff --git a/_fixtures/out_redirect-stdout.txt b/_fixtures/out_redirect-stdout.txt index 9f0dc0f034..a2348d6e3f 100644 --- a/_fixtures/out_redirect-stdout.txt +++ b/_fixtures/out_redirect-stdout.txt @@ -1,2 +1,2 @@ hello world! -hello world! +hello world! \ No newline at end of file diff --git a/pkg/proc/proc_notwindows_test.go b/pkg/proc/proc_notwindows_test.go index c63e24d008..0be767ea2d 100644 --- a/pkg/proc/proc_notwindows_test.go +++ b/pkg/proc/proc_notwindows_test.go @@ -51,7 +51,7 @@ func testGenRedireByPath(t *testing.T, fixture protest.Fixture, stdoutExpectFile return } - if string(expect) == string(out) { + if string(expect) != string(out) { errChan <- fmt.Errorf("%s,Not as expected!\nexpect:%s\nout:%s", mode, expect, out) return } diff --git a/service/dap/server_test.go b/service/dap/server_test.go index e04ec365bc..385bc1552e 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -7380,11 +7380,10 @@ func TestRedirect(t *testing.T) { // 2 >> launch, << initialized, << launch client.LaunchRequestWithArgs(map[string]interface{}{ - "request": "launch", - "mode": "debug", - "program": fixture.Path, - "stopOnEntry": stopOnEntry, - "outputMode": "only-remote", + "request": "launch", + "mode": "debug", + "program": fixture.Source, + "outputMode": "only-remote", }) initEvent := client.ExpectInitializedEvent(t) if initEvent.Seq != 0 { @@ -7398,19 +7397,12 @@ func TestRedirect(t *testing.T) { // 5 >> configurationDone, << stopped, << configurationDone client.ConfigurationDoneRequest() - stopEvent := client.ExpectStoppedEvent(t) - if stopEvent.Seq != 0 || - stopEvent.Body.Reason != "entry" || - stopEvent.Body.ThreadId != 1 || - !stopEvent.Body.AllThreadsStopped { - t.Errorf("\ngot %#v\nwant Seq=0, Body={Reason=\"entry\", ThreadId=1, AllThreadsStopped=true}", stopEvent) - } cdResp := client.ExpectConfigurationDoneResponse(t) - if cdResp.Seq != 0 || cdResp.RequestSeq != 5 { + if cdResp.Seq != 0 || cdResp.RequestSeq != 3 { t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=5", cdResp) } - // wait for output and terminated + // 6 << output, << terminated var ( stdout = bytes.NewBufferString("") stderr = bytes.NewBufferString("") @@ -7434,9 +7426,10 @@ func TestRedirect(t *testing.T) { } } var ( - stdoutFilePath = filepath.Join(fixture.Path, "out_redirect-stdout.txt") - stderrFilePath = filepath.Join(fixture.Path, "out_redirect-stderr.txt") + stdoutFilePath = filepath.Join(fixture.BuildDir, "out_redirect-stdout.txt") + stderrFilePath = filepath.Join(fixture.BuildDir, "out_redirect-stderr.txt") ) + // check output expectStdout, err := os.ReadFile(stdoutFilePath) if err != nil { @@ -7444,7 +7437,7 @@ func TestRedirect(t *testing.T) { } if string(expectStdout) != stdout.String() { - t.Errorf("\n got stdout:\n%s\nwant:%s", stdout.String(), string(expectStdout)) + t.Errorf("\n got stdout: len:%d\n%s\nwant: len:%d\n%s", stdout.Len(), stdout.String(), len(expectStdout), string(expectStdout)) } expectStderr, err := os.ReadFile(stderrFilePath) @@ -7453,7 +7446,7 @@ func TestRedirect(t *testing.T) { } if string(expectStderr) != stderr.String() { - t.Errorf("\n got stderr:\n%s\nwant:%s", stderr.String(), string(expectStderr)) + t.Errorf("\n got stderr: len:%d \n%s\nwant: len:%d\n%s", stderr.Len(), stderr.String(), len(expectStderr), string(expectStderr)) } // 7 >> disconnect, << disconnect @@ -7467,8 +7460,8 @@ func TestRedirect(t *testing.T) { t.Errorf("\ngot %#v\nwant Seq=0 Category='console'", oed) } dResp := client.ExpectDisconnectResponse(t) - if dResp.Seq != 0 || dResp.RequestSeq != 13 { - t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=13", dResp) + if dResp.Seq != 0 || dResp.RequestSeq != 4 { + t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=43", dResp) } client.ExpectTerminatedEvent(t) }) From 61092e25fb9cce14eca576dd35add50d7ba2d0fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Fri, 3 Mar 2023 01:16:23 +0800 Subject: [PATCH 27/53] fix: gdbserial: File not closed in file mode. (in reality, gdbserial will never use file mode) --- pkg/proc/gdbserial/rr.go | 1 + service/dap/server.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/proc/gdbserial/rr.go b/pkg/proc/gdbserial/rr.go index a1c3625b06..68536ef968 100644 --- a/pkg/proc/gdbserial/rr.go +++ b/pkg/proc/gdbserial/rr.go @@ -82,6 +82,7 @@ func openRedirects(redirects proc.Redirect, quiet bool) (stdin, stdout, stderr * stdout = redirects.WriterFiles[1] stderr = redirects.WriterFiles[2] + toclose = append(toclose, stdout, stderr) return stdin, stdout, stderr, closefn, nil } diff --git a/service/dap/server.go b/service/dap/server.go index dfcea82d23..efabf94eff 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -1266,7 +1266,7 @@ func (s *Session) onDisconnectRequest(request *dap.DisconnectRequest) { defer s.mu.Unlock() if s.debugger != nil && s.config.AcceptMulti && !request.Arguments.TerminateDebuggee { - // this is a multi-use server/debugger, so a disconnect request that doesn't + // This is a multi-use server/debugger, so a disconnect request that doesn't // terminate the debuggee should clean up only the client connection and pointer to debugger, // but not the entire server. status := "halted" From bdd13c9baeeb9639b78243a9abcc7e8dd13c2cbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Fri, 3 Mar 2023 21:30:07 +0800 Subject: [PATCH 28/53] feat: Remove "only-remote". Fix spelling mistakes. --- service/dap/server.go | 3 --- service/dap/server_test.go | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/service/dap/server.go b/service/dap/server.go index efabf94eff..f8dd97f532 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -1048,9 +1048,6 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { s.outputMode |= outputToStd switch args.OutputMode { case "remote": - s.outputMode |= outputToDAP - redirected = true - case "only-remote": s.outputMode = outputToDAP redirected = true case "local", "": diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 385bc1552e..23e2626739 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -7383,7 +7383,7 @@ func TestRedirect(t *testing.T) { "request": "launch", "mode": "debug", "program": fixture.Source, - "outputMode": "only-remote", + "outputMode": "remote", }) initEvent := client.ExpectInitializedEvent(t) if initEvent.Seq != 0 { From f43b3d3e8c4b8ea9c2326f378e34f3cfb08ba800 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Fri, 3 Mar 2023 21:39:24 +0800 Subject: [PATCH 29/53] fix: spelling mistakes --- pkg/proc/proc_notwindows_test.go | 4 ++-- pkg/proc/proc_test.go | 14 +++++++------- pkg/proc/proc_windows_test.go | 4 ++-- service/dap/server.go | 28 ++++++++++++++-------------- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/pkg/proc/proc_notwindows_test.go b/pkg/proc/proc_notwindows_test.go index 0be767ea2d..2a874af395 100644 --- a/pkg/proc/proc_notwindows_test.go +++ b/pkg/proc/proc_notwindows_test.go @@ -15,7 +15,7 @@ import ( protest "github.com/go-delve/delve/pkg/proc/test" ) -func testGenRedireByPath(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { +func testGenRediretByPath(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { var ( stdoutPath = "./stdout" stderrPath = "./stderr" @@ -75,6 +75,6 @@ func testGenRedireByPath(t *testing.T, fixture protest.Fixture, stdoutExpectFile return redirect, canceFunc, nil } -func testGenRedireByFile(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { +func testGenRediretByFile(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { return proc.NewEmptyRedirectByFile(), nil, nil } diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 541596beaf..9f9c318bd8 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -5943,7 +5943,7 @@ func TestRedirect(t *testing.T) { err error redirect proc.Redirect = proc.NewEmptyRedirectByPath() errChan = make(chan error, 2) - canceFunc func() + cancelFunc func() needCheck = false stdoutExpectFile = "out_redirect-stdout.txt" stderrExpectFile = "out_redirect-stderr.txt" @@ -5951,13 +5951,13 @@ func TestRedirect(t *testing.T) { switch testBackend { case "native": if runtime.GOOS == "linux" { - redirect, canceFunc, err = testGenRedireByPath(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + redirect, cancelFunc, err = testGenRediretByPath(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) if err != nil { break } needCheck = true } else if runtime.GOOS == "windows" { - redirect, canceFunc, err = testGenRedireByFile(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + redirect, cancelFunc, err = testGenRediretByFile(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) if err != nil { break } @@ -5967,7 +5967,7 @@ func TestRedirect(t *testing.T) { grp, err = native.Launch([]string{fixture.Path}, ".", 0, []string{}, "", redirect) case "lldb": if runtime.GOOS == "darwin" { - redirect, canceFunc, err = testGenRedireByPath(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + redirect, cancelFunc, err = testGenRediretByPath(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) if err != nil { break } @@ -5978,7 +5978,7 @@ func TestRedirect(t *testing.T) { protest.MustHaveRecordingAllowed(t) t.Log("recording") if runtime.GOOS != "windows" { - redirect, canceFunc, err = testGenRedireByPath(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + redirect, cancelFunc, err = testGenRediretByPath(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) if err != nil { break } @@ -5991,8 +5991,8 @@ func TestRedirect(t *testing.T) { } if err != nil { // clear reader goroutine - if canceFunc != nil { - canceFunc() + if cancelFunc != nil { + cancelFunc() _, _ = <-errChan, <-errChan } t.Fatal("Launch():", err) diff --git a/pkg/proc/proc_windows_test.go b/pkg/proc/proc_windows_test.go index fb74da5f0d..d7ff35e942 100644 --- a/pkg/proc/proc_windows_test.go +++ b/pkg/proc/proc_windows_test.go @@ -14,7 +14,7 @@ import ( protest "github.com/go-delve/delve/pkg/proc/test" ) -func testGenRedireByFile(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { +func testGenRediretByFile(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { stdoutReader, stdoutWriter, err := os.Pipe() if err != nil { return redirect, nil, err @@ -57,6 +57,6 @@ func testGenRedireByFile(t *testing.T, fixture protest.Fixture, stdoutExpectFile return redirect, canceFunc, nil } -func testGenRedireByPath(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { +func testGenRediretByPath(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { return proc.NewEmptyRedirectByPath(), nil, nil } diff --git a/service/dap/server.go b/service/dap/server.go index f8dd97f532..dc12270dad 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -167,8 +167,8 @@ type Session struct { // stderrReader the program's stderr. stderrReader io.ReadCloser - // wg the WaitGroup that needs to wait before sending a terminated event. - wg sync.WaitGroup + // preTerminatedWG the WaitGroup that needs to wait before sending a terminated event. + preTerminatedWG sync.WaitGroup } type outputMode int8 @@ -1108,15 +1108,15 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { // Start the program on a different goroutine, so we can listen for disconnect request. go func() { if redirected { - s.wg.Add(1) + s.preTerminatedWG.Add(1) go func() { - defer s.wg.Done() + defer s.preTerminatedWG.Done() readerFunc(s.stdoutReader, "stdout") }() - s.wg.Add(1) + s.preTerminatedWG.Add(1) go func() { - defer s.wg.Done() + defer s.preTerminatedWG.Done() readerFunc(s.stderrReader, "stderr") }() // Wait for the input and output to be read @@ -1141,9 +1141,9 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { } s.config.Debugger.Redirect = redirects - s.wg.Add(1) + s.preTerminatedWG.Add(1) go func() { - defer s.wg.Done() + defer s.preTerminatedWG.Done() if err = ReadRedirect("stdout", redirects, func(reader io.Reader) { readerFunc(reader, "stdout") }); err != nil { @@ -1153,9 +1153,9 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { } }() - s.wg.Add(1) + s.preTerminatedWG.Add(1) go func() { - defer s.wg.Done() + defer s.preTerminatedWG.Done() if err = ReadRedirect("stderr", redirects, func(reader io.Reader) { readerFunc(reader, "stderr") }); err != nil { @@ -1271,7 +1271,7 @@ func (s *Session) onDisconnectRequest(request *dap.DisconnectRequest) { status = "running" } else if state, err := s.debugger.State(false); processExited(state, err) { status = "exited" - s.wg.Wait() + s.preTerminatedWG.Wait() } s.logToConsole(fmt.Sprintf("Closing client session, but leaving multi-client DAP server at %s with debuggee %s", s.config.Listener.Addr().String(), status)) @@ -1307,7 +1307,7 @@ func (s *Session) onDisconnectRequest(request *dap.DisconnectRequest) { } else { s.send(&dap.DisconnectResponse{Response: *newResponse(request.Request)}) } - s.wg.Wait() + s.preTerminatedWG.Wait() // The debugging session has ended, so we send a terminated event. s.send(&dap.TerminatedEvent{Event: *newEvent("terminated")}) } @@ -2867,7 +2867,7 @@ func (s *Session) doCall(goid, frame int, expr string) (*api.DebuggerState, []*p GoroutineID: int64(goid), }, nil) if processExited(state, err) { - s.wg.Wait() + s.preTerminatedWG.Wait() e := &dap.TerminatedEvent{Event: *newEvent("terminated")} s.send(e) return nil, nil, errors.New("terminated") @@ -3612,7 +3612,7 @@ func (s *Session) runUntilStopAndNotify(command string, allowNextStateChange cha } if processExited(state, err) { - s.wg.Wait() + s.preTerminatedWG.Wait() s.send(&dap.TerminatedEvent{Event: *newEvent("terminated")}) return } From 28cea4752003d072bc4d8be841acb1557ba66561 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Thu, 9 Mar 2023 01:48:53 +0800 Subject: [PATCH 30/53] refactor: redirect --- pkg/proc/gdbserial/gdbserver.go | 8 +-- pkg/proc/gdbserial/rr.go | 69 +++++++++++---------- pkg/proc/gdbserial/rr_test.go | 2 +- pkg/proc/native/nonative_darwin.go | 2 +- pkg/proc/native/proc.go | 63 ++++++++++---------- pkg/proc/native/proc_darwin.go | 2 +- pkg/proc/native/proc_freebsd.go | 2 +- pkg/proc/native/proc_linux.go | 2 +- pkg/proc/native/proc_windows.go | 2 +- pkg/proc/proc_notwindows_test.go | 80 ------------------------- pkg/proc/proc_test.go | 79 ++++++++++++++++++++---- pkg/proc/proc_windows_test.go | 62 ------------------- pkg/proc/redirect.go | 28 ++++----- pkg/proc/stdio_pipe.go | 81 +++++++++++++++++++++++++ pkg/proc/stdio_pipe_windows.go | 43 +++++++++++++ service/dap/server.go | 96 +++++++++--------------------- service/dap/server_test.go | 7 ++- service/dap/stdio_pipe.go | 56 ----------------- service/dap/stdio_pipe_windows.go | 34 ----------- service/debugger/debugger.go | 2 +- 20 files changed, 311 insertions(+), 409 deletions(-) delete mode 100644 pkg/proc/proc_notwindows_test.go delete mode 100644 pkg/proc/proc_windows_test.go create mode 100644 pkg/proc/stdio_pipe.go create mode 100644 pkg/proc/stdio_pipe_windows.go delete mode 100644 service/dap/stdio_pipe.go delete mode 100644 service/dap/stdio_pipe_windows.go diff --git a/pkg/proc/gdbserial/gdbserver.go b/pkg/proc/gdbserial/gdbserver.go index 5dda5a043e..26cc337364 100644 --- a/pkg/proc/gdbserial/gdbserver.go +++ b/pkg/proc/gdbserial/gdbserver.go @@ -450,7 +450,7 @@ func getLdEnvVars() []string { // LLDBLaunch starts an instance of lldb-server and connects to it, asking // it to launch the specified target program with the specified arguments // (cmd) on the specified directory wd. -func LLDBLaunch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []string, tty string, redirects proc.Redirect) (*proc.TargetGroup, error) { +func LLDBLaunch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []string, tty string, redirects [3]proc.OutputRedirect) (*proc.TargetGroup, error) { if runtime.GOOS == "windows" { return nil, ErrUnsupportedOS } @@ -483,11 +483,11 @@ func LLDBLaunch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs [ } else { found := [3]bool{} names := [3]string{"stdin", "stdout", "stderr"} - for i := range redirects.Paths { - if redirects.Paths[i] != "" { + for i := range redirects { + if redirects[i].Path != "" { found[i] = true hasRedirects = true - args = append(args, fmt.Sprintf("--%s-path", names[i]), redirects.Paths[i]) + args = append(args, fmt.Sprintf("--%s-path", names[i]), redirects[i].Path) } } diff --git a/pkg/proc/gdbserial/rr.go b/pkg/proc/gdbserial/rr.go index 68536ef968..388dd789b8 100644 --- a/pkg/proc/gdbserial/rr.go +++ b/pkg/proc/gdbserial/rr.go @@ -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 proc.Redirect) (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) { if err := checkRRAvailable(); err != nil { return nil, nil, err } @@ -63,67 +63,66 @@ func RecordAsync(cmd []string, wd string, quiet bool, redirects proc.Redirect) ( return run, stop, nil } -func openRedirects(redirects proc.Redirect, quiet bool) (stdin, stdout, stderr *os.File, closefn func(), err error) { +func openRedirects(redirects [3]proc.OutputRedirect, quiet bool) (stdin, stdout, stderr *os.File, closefn func(), err error) { var ( toclose = []*os.File{} ) - closefn = func() { - for _, f := range toclose { - _ = f.Close() - } - } - - if redirects.Mode == proc.RedirectFileMode { - stdin = redirects.WriterFiles[0] - if stdin == nil { - stdin = os.Stdin - } - stdout = redirects.WriterFiles[1] - stderr = redirects.WriterFiles[2] - - toclose = append(toclose, stdout, stderr) - return stdin, stdout, stderr, closefn, nil - } - - if redirects.Paths[0] != "" { - stdin, err = os.Open(redirects.Paths[0]) + if redirects[0].Path != "" { + stdin, err = os.Open(redirects[0].Path) if err != nil { return nil, nil, nil, nil, err } toclose = append(toclose, stdin) - } else if quiet { + } else { stdin = os.Stdin } - create := func(path string, dflt *os.File) *os.File { - if path == "" { - return dflt + 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 } - var f *os.File - f, err = os.Create(path) - if f != nil { - toclose = append(toclose, f) + + if redirect.File != nil { + toclose = append(toclose, redirect.File) + + return redirect.File + } + + if quiet { + return nil } - return f + + return dflt } - stdout = create(redirects.Paths[1], os.Stdout) + stdout = create(redirects[1], os.Stdout) if err != nil { return nil, nil, nil, nil, err } - stderr = create(redirects.Paths[2], os.Stderr) + stderr = create(redirects[2], os.Stderr) if err != nil { return nil, nil, nil, nil, err } + closefn = func() { + for _, f := range toclose { + _ = f.Close() + } + } + return stdin, stdout, stderr, closefn, nil } // 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 proc.Redirect) (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 @@ -290,7 +289,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 proc.Redirect) (*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 diff --git a/pkg/proc/gdbserial/rr_test.go b/pkg/proc/gdbserial/rr_test.go index 569882f7f7..7c03d41d64 100644 --- a/pkg/proc/gdbserial/rr_test.go +++ b/pkg/proc/gdbserial/rr_test.go @@ -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{}, proc.NewEmptyRedirectByPath()) + grp, tracedir, err := gdbserial.RecordAndReplay([]string{fixture.Path}, ".", true, []string{}, proc.NewEmptyRedirect()) if err != nil { t.Fatal("Launch():", err) } diff --git a/pkg/proc/native/nonative_darwin.go b/pkg/proc/native/nonative_darwin.go index 4bd3c7154e..3c56159a4c 100644 --- a/pkg/proc/native/nonative_darwin.go +++ b/pkg/proc/native/nonative_darwin.go @@ -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, _ proc.Redirect) (*proc.TargetGroup, error) { +func Launch(_ []string, _ string, _ proc.LaunchFlags, _ []string, _ string, _ [3]proc.OutputRedirect) (*proc.TargetGroup, error) { return nil, ErrNativeBackendDisabled } diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go index f91e434f2b..8088baa9db 100644 --- a/pkg/proc/native/proc.go +++ b/pkg/proc/native/proc.go @@ -370,61 +370,60 @@ func (dbp *nativeProcess) writeSoftwareBreakpoint(thread *nativeThread, addr uin return err } -func openRedirects(redirects proc.Redirect, foreground bool) (stdin, stdout, stderr *os.File, closefn func(), err error) { +func openRedirects(redirects [3]proc.OutputRedirect, foreground bool) (stdin, stdout, stderr *os.File, closefn func(), err error) { var ( toclose = []*os.File{} ) - closefn = func() { - for _, f := range toclose { - _ = f.Close() - } - } - - if redirects.Mode == proc.RedirectFileMode { - stdin = redirects.WriterFiles[0] - if stdin == nil && foreground { - stdin = os.Stdin - } - stdout = redirects.WriterFiles[1] - stderr = redirects.WriterFiles[2] - toclose = append(toclose, stdin, stderr) - - return stdin, stdout, stderr, closefn, nil - } - - if redirects.Paths[0] != "" { - stdin, err = os.Open(redirects.Paths[0]) + if redirects[0].Path != "" { + stdin, err = os.Open(redirects[0].Path) if err != nil { return nil, nil, nil, nil, err } toclose = append(toclose, stdin) - } else if foreground { + } else { stdin = os.Stdin } - create := func(path string, dflt *os.File) *os.File { - if path == "" { - return dflt + 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 } - var f *os.File - f, err = os.Create(path) - if f != nil { - toclose = append(toclose, f) + + if redirect.File != nil { + toclose = append(toclose, redirect.File) + + return redirect.File + } + + if foreground { + return nil } - return f + + return dflt } - stdout = create(redirects.Paths[1], os.Stdout) + stdout = create(redirects[1], os.Stdout) if err != nil { return nil, nil, nil, nil, err } - stderr = create(redirects.Paths[2], os.Stderr) + stderr = create(redirects[2], os.Stderr) if err != nil { return nil, nil, nil, nil, err } + closefn = func() { + for _, f := range toclose { + _ = f.Close() + } + } + return stdin, stdout, stderr, closefn, nil } diff --git a/pkg/proc/native/proc_darwin.go b/pkg/proc/native/proc_darwin.go index cf085d590f..194ef10120 100644 --- a/pkg/proc/native/proc_darwin.go +++ b/pkg/proc/native/proc_darwin.go @@ -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, _ proc.Redirect) (*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 diff --git a/pkg/proc/native/proc_freebsd.go b/pkg/proc/native/proc_freebsd.go index 101e6e5617..c236871526 100644 --- a/pkg/proc/native/proc_freebsd.go +++ b/pkg/proc/native/proc_freebsd.go @@ -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 proc.Redirect) (*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 diff --git a/pkg/proc/native/proc_linux.go b/pkg/proc/native/proc_linux.go index 3d5186629a..3d29ea20ba 100644 --- a/pkg/proc/native/proc_linux.go +++ b/pkg/proc/native/proc_linux.go @@ -62,7 +62,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, redirect proc.Redirect) (*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 diff --git a/pkg/proc/native/proc_windows.go b/pkg/proc/native/proc_windows.go index c332f7208d..ca0bb1279f 100644 --- a/pkg/proc/native/proc_windows.go +++ b/pkg/proc/native/proc_windows.go @@ -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 proc.Redirect) (*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() diff --git a/pkg/proc/proc_notwindows_test.go b/pkg/proc/proc_notwindows_test.go deleted file mode 100644 index 2a874af395..0000000000 --- a/pkg/proc/proc_notwindows_test.go +++ /dev/null @@ -1,80 +0,0 @@ -//go:build !windows -// +build !windows - -package proc_test - -import ( - "fmt" - "io" - "os" - "path/filepath" - "syscall" - "testing" - - "github.com/go-delve/delve/pkg/proc" - protest "github.com/go-delve/delve/pkg/proc/test" -) - -func testGenRediretByPath(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { - var ( - stdoutPath = "./stdout" - stderrPath = "./stderr" - ) - - if err = syscall.Mkfifo(stdoutPath, 0o600); err != nil { - return redirect, nil, err - } - - if err = syscall.Mkfifo(stderrPath, 0o600); err != nil { - os.Remove(stdoutPath) - return redirect, nil, err - } - - redirect = proc.NewRedirectByPath([3]string{"", stdoutPath, stderrPath}) - reader := func(mode string, path string, expectFile string) { - defer os.Remove(path) - outFile, err := os.OpenFile(path, os.O_RDONLY, os.ModeNamedPipe) - if err != nil { - errChan <- err - return - } - - expect, err := os.ReadFile(filepath.Join(fixture.BuildDir, expectFile)) - if err != nil { - errChan <- err - return - } - - out, err := io.ReadAll(outFile) - if err != nil { - errChan <- err - return - } - - if string(expect) != string(out) { - errChan <- fmt.Errorf("%s,Not as expected!\nexpect:%s\nout:%s", mode, expect, out) - return - } - errChan <- nil - } - - canceFunc = func() { - stdout, err := os.OpenFile(stdoutPath, os.O_WRONLY, os.ModeNamedPipe) - if err == nil { - stdout.Close() - } - stderr, err := os.OpenFile(stderrPath, os.O_WRONLY, os.ModeNamedPipe) - if err == nil { - stderr.Close() - } - } - - go reader("stdout", stdoutPath, stdoutExpectFile) - go reader("stderr", stderrPath, stderrExpectFile) - - return redirect, canceFunc, nil -} - -func testGenRediretByFile(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { - return proc.NewEmptyRedirectByFile(), nil, nil -} diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 9f9c318bd8..333f21b961 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -8,6 +8,7 @@ import ( "go/ast" "go/constant" "go/token" + "io" "io/ioutil" "math/rand" "net" @@ -103,13 +104,13 @@ func withTestProcessArgs(name string, t testing.TB, wd string, args []string, bu switch testBackend { case "native": - grp, err = native.Launch(append([]string{fixture.Path}, args...), wd, 0, []string{}, "", proc.NewEmptyRedirectByPath()) + grp, err = native.Launch(append([]string{fixture.Path}, args...), wd, 0, []string{}, "", proc.NewEmptyRedirect()) case "lldb": - grp, err = gdbserial.LLDBLaunch(append([]string{fixture.Path}, args...), wd, 0, []string{}, "", proc.NewEmptyRedirectByPath()) + grp, err = gdbserial.LLDBLaunch(append([]string{fixture.Path}, args...), wd, 0, []string{}, "", proc.NewEmptyRedirect()) case "rr": protest.MustHaveRecordingAllowed(t) t.Log("recording") - grp, tracedir, err = gdbserial.RecordAndReplay(append([]string{fixture.Path}, args...), wd, true, []string{}, proc.NewEmptyRedirectByPath()) + grp, tracedir, err = gdbserial.RecordAndReplay(append([]string{fixture.Path}, args...), wd, true, []string{}, proc.NewEmptyRedirect()) t.Logf("replaying %q", tracedir) default: t.Fatal("unknown backend") @@ -2217,9 +2218,9 @@ func TestUnsupportedArch(t *testing.T) { switch testBackend { case "native": - p, err = native.Launch([]string{outfile}, ".", 0, []string{}, "", proc.NewEmptyRedirectByPath()) + p, err = native.Launch([]string{outfile}, ".", 0, []string{}, "", proc.NewEmptyRedirect()) case "lldb": - p, err = gdbserial.LLDBLaunch([]string{outfile}, ".", 0, []string{}, "", proc.NewEmptyRedirectByPath()) + p, err = gdbserial.LLDBLaunch([]string{outfile}, ".", 0, []string{}, "", proc.NewEmptyRedirect()) default: t.Skip("test not valid for this backend") } @@ -5935,14 +5936,70 @@ func TestStacktraceExtlinkMac(t *testing.T) { }) } +func testGenRedirect(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect [3]proc.OutputRedirect, cancelFunc func(), err error) { + redirector, err := proc.NewRedirector() + if err != nil { + return redirect, nil, err + } + + redirect = redirector.Writer() + cancelFunc = func() { + for _, outputRedirect := range redirect { + if outputRedirect.File != nil { + _ = outputRedirect.File.Close() + } + + if outputRedirect.Path != "" { + stdio, err := os.OpenFile(outputRedirect.Path, os.O_WRONLY, os.ModeNamedPipe) + if err == nil { + stdio.Close() + } + } + } + } + + // redirector.Reader() will be blocked. + go func() { + reader := func(mode string, f io.ReadCloser, expectFile string) { + expect, err := os.ReadFile(filepath.Join(fixture.BuildDir, expectFile)) + if err != nil { + errChan <- err + return + } + + out, err := io.ReadAll(f) + if err != nil { + errChan <- err + return + } + + if string(expect) != string(out) { + errChan <- fmt.Errorf("%s,Not as expected!\nexpect:%s\nout:%s", mode, expect, out) + return + } + errChan <- nil + } + + readers, err := redirector.Reader() + if err != nil { + errChan <- err + return + } + go reader("stdout", readers[0], stdoutExpectFile) + go reader("stderr", readers[1], stderrExpectFile) + }() + + return redirect, cancelFunc, nil +} + func TestRedirect(t *testing.T) { fixture := protest.BuildFixture("out_redirect", 0) var ( grp *proc.TargetGroup tracedir string err error - redirect proc.Redirect = proc.NewEmptyRedirectByPath() - errChan = make(chan error, 2) + redirect [3]proc.OutputRedirect = proc.NewEmptyRedirect() + errChan = make(chan error, 2) cancelFunc func() needCheck = false stdoutExpectFile = "out_redirect-stdout.txt" @@ -5951,13 +6008,13 @@ func TestRedirect(t *testing.T) { switch testBackend { case "native": if runtime.GOOS == "linux" { - redirect, cancelFunc, err = testGenRediretByPath(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + redirect, cancelFunc, err = testGenRedirect(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) if err != nil { break } needCheck = true } else if runtime.GOOS == "windows" { - redirect, cancelFunc, err = testGenRediretByFile(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + redirect, cancelFunc, err = testGenRedirect(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) if err != nil { break } @@ -5967,7 +6024,7 @@ func TestRedirect(t *testing.T) { grp, err = native.Launch([]string{fixture.Path}, ".", 0, []string{}, "", redirect) case "lldb": if runtime.GOOS == "darwin" { - redirect, cancelFunc, err = testGenRediretByPath(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + redirect, cancelFunc, err = testGenRedirect(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) if err != nil { break } @@ -5978,7 +6035,7 @@ func TestRedirect(t *testing.T) { protest.MustHaveRecordingAllowed(t) t.Log("recording") if runtime.GOOS != "windows" { - redirect, cancelFunc, err = testGenRediretByPath(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + redirect, cancelFunc, err = testGenRedirect(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) if err != nil { break } diff --git a/pkg/proc/proc_windows_test.go b/pkg/proc/proc_windows_test.go deleted file mode 100644 index d7ff35e942..0000000000 --- a/pkg/proc/proc_windows_test.go +++ /dev/null @@ -1,62 +0,0 @@ -//go:build windows -// +build windows - -package proc_test - -import ( - "fmt" - "io" - "os" - "path/filepath" - "testing" - - "github.com/go-delve/delve/pkg/proc" - protest "github.com/go-delve/delve/pkg/proc/test" -) - -func testGenRediretByFile(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { - stdoutReader, stdoutWriter, err := os.Pipe() - if err != nil { - return redirect, nil, err - } - - stderrReader, stderrWriter, err := os.Pipe() - if err != nil { - return redirect, nil, err - } - - redirect = proc.NewRedirectByFile([3]*os.File{nil, stderrReader, stdoutReader}, [3]*os.File{nil, stdoutWriter, stderrWriter}) - reader := func(mode string, f *os.File, expectFile string) { - expect, err := os.ReadFile(filepath.Join(fixture.BuildDir, expectFile)) - if err != nil { - errChan <- err - return - } - - out, err := io.ReadAll(f) - if err != nil { - errChan <- err - return - } - - if string(expect) != string(out) { - errChan <- fmt.Errorf("%s,Not as expected!\nexpect:%s\nout:%s", mode, expect, out) - return - } - errChan <- nil - } - - canceFunc = func() { - _ = stdoutWriter.Close() - _ = stderrWriter.Close() - } - - go reader("stdout", stdoutReader, stdoutExpectFile) - go reader("stderr", stderrReader, stderrExpectFile) - - return redirect, canceFunc, nil -} - -func testGenRediretByPath(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect proc.Redirect, canceFunc func(), err error) { - return proc.NewEmptyRedirectByPath(), nil, nil -} diff --git a/pkg/proc/redirect.go b/pkg/proc/redirect.go index fccd27e217..fedcefb050 100644 --- a/pkg/proc/redirect.go +++ b/pkg/proc/redirect.go @@ -4,30 +4,24 @@ import "os" type Mode string +type OutputRedirect struct { + Path string + File *os.File +} + const ( RedirectFileMode Mode = "file" RedirectPathMode Mode = "path" ) -type Redirect struct { - WriterFiles [3]*os.File - ReaderFiles [3]*os.File - Paths [3]string - Mode Mode -} - -func NewEmptyRedirectByPath() Redirect { - return Redirect{Mode: RedirectPathMode} -} - -func NewEmptyRedirectByFile() Redirect { - return Redirect{Mode: RedirectFileMode} +func NewEmptyRedirect() [3]OutputRedirect { + return [3]OutputRedirect{} } -func NewRedirectByPath(paths [3]string) Redirect { - return Redirect{Paths: paths, Mode: RedirectPathMode} +func NewRedirectByPath(paths [3]string) [3]OutputRedirect { + return [3]OutputRedirect{{Path: paths[0]}, {Path: paths[1]}, {Path: paths[2]}} } -func NewRedirectByFile(readerFiles [3]*os.File, writerFiles [3]*os.File) Redirect { - return Redirect{ReaderFiles: readerFiles, WriterFiles: writerFiles, Mode: RedirectFileMode} +func NewRedirectByFile(writerFiles [3]*os.File) [3]OutputRedirect { + return [3]OutputRedirect{{File: writerFiles[0]}, {File: writerFiles[1]}, {File: writerFiles[2]}} } diff --git a/pkg/proc/stdio_pipe.go b/pkg/proc/stdio_pipe.go new file mode 100644 index 0000000000..542f9d7d62 --- /dev/null +++ b/pkg/proc/stdio_pipe.go @@ -0,0 +1,81 @@ +//go:build !windows +// +build !windows + +package proc + +import ( + "crypto/rand" + "encoding/hex" + "io" + "os" + "path/filepath" + "syscall" +) + +type stdioRedirector struct { + Paths [3]string +} + +func NewRedirector() (redirect *stdioRedirector, err error) { + r := make([]byte, 4) + if _, err := rand.Read(r); err != nil { + return redirect, 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 redirect, err + } + + if err := syscall.Mkfifo(stderrPath, 0o600); err != nil { + _ = os.Remove(stdoutPath) + return redirect, err + } + + return &stdioRedirector{Paths: [3]string{"", stdoutPath, stderrPath}}, nil +} + +// Writer return [3]{stdin,stdout,stderr} +func (s *stdioRedirector) Writer() [3]OutputRedirect { + return NewRedirectByPath(s.Paths) +} + +type warpClose struct { + *os.File + path string +} + +func (s *warpClose) Close() error { + defer os.Remove(s.path) + return s.File.Close() +} + +func newWarpClose(file *os.File, path string) io.ReadCloser { + return &warpClose{File: file, path: path} +} + +// Reader return []{stdout,stderr}. +// Delete the file when the Close interface is called. +// OpenFile(path,os.O_RDONLY,os.ModeNamedPipe) will be blocked. +// The Reader should be called asynchronously. +func (s *stdioRedirector) Reader() (reader [2]io.ReadCloser, err error) { + stdoutFile, err := os.OpenFile(s.Paths[1], os.O_RDONLY, os.ModeNamedPipe) + if err != nil { + return reader, err + } + + stderrFile, err := os.OpenFile(s.Paths[2], os.O_RDONLY, os.ModeNamedPipe) + if err != nil { + return reader, err + } + + reader[0] = newWarpClose(stdoutFile, s.Paths[1]) + reader[1] = newWarpClose(stderrFile, s.Paths[2]) + + return reader, nil +} diff --git a/pkg/proc/stdio_pipe_windows.go b/pkg/proc/stdio_pipe_windows.go new file mode 100644 index 0000000000..9c06324e08 --- /dev/null +++ b/pkg/proc/stdio_pipe_windows.go @@ -0,0 +1,43 @@ +//go:build windows +// +build windows + +package proc + +import ( + "io" + "os" +) + +type stdioRedirector struct { + writers [3]*os.File + readers [3]*os.File +} + +func NewRedirector() (redirect *stdioRedirector, err error) { + stdoutReader, stdoutWriter, err := os.Pipe() + if err != nil { + return redirect, err + } + + stderrReader, stderrWriter, err := os.Pipe() + if err != nil { + return redirect, err + } + + return &stdioRedirector{ + writers: [3]*os.File{nil, stdoutWriter, stderrWriter}, + readers: [3]*os.File{nil, stdoutReader, stderrReader}, + }, err +} + +// Writer return [3]{stdin,stdout,stderr} +func (s *stdioRedirector) Writer() [3]OutputRedirect { + return NewRedirectByFile(s.writers) +} + +// Reader return [2]{stdout,stderr} +func (s *stdioRedirector) Reader() (reader [2]io.ReadCloser, err error) { + reader[0] = s.readers[1] + reader[1] = s.readers[2] + return reader, nil +} diff --git a/service/dap/server.go b/service/dap/server.go index dc12270dad..b3a4a7f5a3 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -158,9 +158,6 @@ type Session struct { // changing the state of the running process at the same time. changeStateMu sync.Mutex - // outputMode specifies how to print the program's output. - outputMode outputMode - // stdoutReader the programs's stdout. stdoutReader io.ReadCloser @@ -171,15 +168,6 @@ type Session struct { preTerminatedWG sync.WaitGroup } -type outputMode int8 - -const ( - // outputToStd os.Stdin and os.Stdout - outputToStd outputMode = 1 << iota - // outputToDAP Sending program output to the client. - outputToDAP -) - // Config is all the information needed to start the debugger, handle // DAP connection traffic and signal to the server when it is time to stop. type Config struct { @@ -1045,10 +1033,8 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { s.config.log.Debugf("launching binary '%s' with config: %s", debugbinary, prettyPrint(argsToLog)) var redirected = false - s.outputMode |= outputToStd switch args.OutputMode { case "remote": - s.outputMode = outputToDAP redirected = true case "local", "": // noting @@ -1058,31 +1044,22 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { return } - readerFunc := func(reader io.Reader, category string) { - var stdWriter io.Writer - if category == "stdout" { - stdWriter = os.Stdout - } else { - stdWriter = os.Stderr - } - - var out [1024]byte - for { - n, err := reader.Read(out[:]) - if err != nil { - if errors.Is(io.EOF, err) { + redirectedFunc := func(stdoutReader io.ReadCloser, stderrReader io.ReadCloser) { + runReadFunc := func(reader io.ReadCloser, category string) { + defer s.preTerminatedWG.Done() + defer reader.Close() + // Read output from `reader` and send to client + var out [1024]byte + for { + n, err := reader.Read(out[:]) + if err != nil { + if errors.Is(io.EOF, err) { + return + } + s.config.log.Errorf("failed read by %s - %v ", category, err) return } - s.config.log.Errorf("failed read by %s - %v ", category, err) - return - } - outs := string(out[:n]) - - if s.outputMode&outputToStd != 0 { - fmt.Fprintf(stdWriter, outs) - } - - if s.outputMode&outputToDAP != 0 { + outs := string(out[:n]) s.send(&dap.OutputEvent{ Event: *newEvent("output"), Body: dap.OutputEventBody{ @@ -1091,6 +1068,10 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { }}) } } + + s.preTerminatedWG.Add(2) + go runReadFunc(stdoutReader, "stdout") + go runReadFunc(stderrReader, "stderr") } if args.NoDebug { @@ -1108,19 +1089,9 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { // Start the program on a different goroutine, so we can listen for disconnect request. go func() { if redirected { - s.preTerminatedWG.Add(1) - go func() { - defer s.preTerminatedWG.Done() - readerFunc(s.stdoutReader, "stdout") - }() - - s.preTerminatedWG.Add(1) - go func() { - defer s.preTerminatedWG.Done() - readerFunc(s.stderrReader, "stderr") - }() - // Wait for the input and output to be read + redirectedFunc(s.stdoutReader, s.stderrReader) } + if err := cmd.Wait(); err != nil { s.config.log.Debugf("program exited with error: %v", err) } @@ -1133,36 +1104,23 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { } if redirected { - redirects, err := NewRedirector() + redirects, err := proc.NewRedirector() if err != nil { s.sendShowUserErrorResponse(request.Request, InternalError, "Internal Error", fmt.Sprintf("failed to generate stdio pipes - %v", err)) return } - s.config.Debugger.Redirect = redirects - s.preTerminatedWG.Add(1) - go func() { - defer s.preTerminatedWG.Done() - if err = ReadRedirect("stdout", redirects, func(reader io.Reader) { - readerFunc(reader, "stdout") - }); err != nil { - s.sendShowUserErrorResponse(request.Request, InternalError, "Internal Error", - fmt.Sprintf("failed to open stdout pipe - %v", err)) - return - } + s.config.Debugger.Redirect = redirects.Writer() - }() - s.preTerminatedWG.Add(1) go func() { - defer s.preTerminatedWG.Done() - if err = ReadRedirect("stderr", redirects, func(reader io.Reader) { - readerFunc(reader, "stderr") - }); err != nil { - s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", - fmt.Sprintf("failed to open stderr pipe - %v", err)) + readers, err := redirects.Reader() + if err != nil { + s.sendShowUserErrorResponse(request.Request, InternalError, "Internal Error", + fmt.Sprintf("failed to open pipe - %v", err)) return } + redirectedFunc(readers[0], readers[1]) }() } diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 23e2626739..71e61803af 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -7407,7 +7407,9 @@ func TestRedirect(t *testing.T) { stdout = bytes.NewBufferString("") stderr = bytes.NewBufferString("") ) - for hasNext := true; hasNext; { + + TerminnatedPoint: + for { message := client.ExpectMessage(t) switch m := message.(type) { case *dap.OutputEvent: @@ -7420,11 +7422,12 @@ func TestRedirect(t *testing.T) { t.Errorf("\ngot %#v\nwant Category='stdout' or 'stderr'", m) } case *dap.TerminatedEvent: - hasNext = false + break TerminnatedPoint default: t.Errorf("\n got %#v, want *dap.OutputEvent or *dap.TerminateResponse", m) } } + var ( stdoutFilePath = filepath.Join(fixture.BuildDir, "out_redirect-stdout.txt") stderrFilePath = filepath.Join(fixture.BuildDir, "out_redirect-stderr.txt") diff --git a/service/dap/stdio_pipe.go b/service/dap/stdio_pipe.go deleted file mode 100644 index 8adb48dbde..0000000000 --- a/service/dap/stdio_pipe.go +++ /dev/null @@ -1,56 +0,0 @@ -//go:build !windows -// +build !windows - -package dap - -import ( - "crypto/rand" - "encoding/hex" - "io" - "os" - "path/filepath" - "syscall" - - "github.com/go-delve/delve/pkg/proc" -) - -func NewRedirector() (redirect proc.Redirect, err error) { - r := make([]byte, 4) - if _, err := rand.Read(r); err != nil { - return redirect, 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 redirect, err - } - - if err := syscall.Mkfifo(stderrPath, 0o600); err != nil { - _ = os.Remove(stdoutPath) - return redirect, err - } - - return proc.NewRedirectByPath([3]string{"", stdoutPath, stderrPath}), nil -} - -func ReadRedirect(stdType string, redirect proc.Redirect, f func(reader io.Reader)) error { - path := redirect.Paths[1] - if stdType == "stderr" { - path = redirect.Paths[2] - } - - defer os.Remove(path) - - stdoutFile, err := os.OpenFile(path, os.O_RDONLY, os.ModeNamedPipe) - if err != nil { - return err - } - - f(stdoutFile) - return nil -} diff --git a/service/dap/stdio_pipe_windows.go b/service/dap/stdio_pipe_windows.go deleted file mode 100644 index 66e51aa779..0000000000 --- a/service/dap/stdio_pipe_windows.go +++ /dev/null @@ -1,34 +0,0 @@ -//go:build windows -// +build windows - -package dap - -import ( - "io" - "os" - - "github.com/go-delve/delve/pkg/proc" -) - -func NewRedirector() (redirect proc.Redirect, err error) { - stdoutReader, stdoutWriter, err := os.Pipe() - if err != nil { - return redirect, err - } - - stderrReader, stderrWriter, err := os.Pipe() - if err != nil { - return redirect, err - } - - return proc.NewRedirectByFile([3]*os.File{nil, stdoutReader, stderrReader}, [3]*os.File{nil, stdoutWriter, stderrWriter}), nil -} - -func ReadRedirect(stdType string, redirect proc.Redirect, f func(reader io.Reader)) error { - if stdType == "stdout" { - f(redirect.ReaderFiles[1]) - } else { - f(redirect.ReaderFiles[2]) - } - return nil -} diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index a10c3e3f48..1ab7e16ed0 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -137,7 +137,7 @@ type Config struct { ExecuteKind ExecuteKind // Redirects specifies redirect rules for stdin, stdout and stderr - Redirect proc.Redirect + Redirect [3]proc.OutputRedirect // DisableASLR disables ASLR DisableASLR bool From c0b0d0779860f592812102f4c7dbf9c9364f4910 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Thu, 9 Mar 2023 23:23:32 +0800 Subject: [PATCH 31/53] fix: stdout and stderr are not set to default values --- pkg/proc/gdbserial/rr.go | 6 +----- pkg/proc/native/proc.go | 4 ---- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/pkg/proc/gdbserial/rr.go b/pkg/proc/gdbserial/rr.go index 388dd789b8..500c8032d5 100644 --- a/pkg/proc/gdbserial/rr.go +++ b/pkg/proc/gdbserial/rr.go @@ -74,7 +74,7 @@ func openRedirects(redirects [3]proc.OutputRedirect, quiet bool) (stdin, stdout, return nil, nil, nil, nil, err } toclose = append(toclose, stdin) - } else { + } else if quiet { stdin = os.Stdin } @@ -94,10 +94,6 @@ func openRedirects(redirects [3]proc.OutputRedirect, quiet bool) (stdin, stdout, return redirect.File } - if quiet { - return nil - } - return dflt } diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go index 8088baa9db..c045b77e76 100644 --- a/pkg/proc/native/proc.go +++ b/pkg/proc/native/proc.go @@ -401,10 +401,6 @@ func openRedirects(redirects [3]proc.OutputRedirect, foreground bool) (stdin, st return redirect.File } - if foreground { - return nil - } - return dflt } From 4617c1f321d2cb58f956c1a128d252a5284c9e41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Thu, 9 Mar 2023 23:28:09 +0800 Subject: [PATCH 32/53] fix: Restore code logic for rr.openRedirects() --- pkg/proc/gdbserial/rr.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/proc/gdbserial/rr.go b/pkg/proc/gdbserial/rr.go index 500c8032d5..84bfd6ea67 100644 --- a/pkg/proc/gdbserial/rr.go +++ b/pkg/proc/gdbserial/rr.go @@ -94,6 +94,10 @@ func openRedirects(redirects [3]proc.OutputRedirect, quiet bool) (stdin, stdout, return redirect.File } + if quiet { + return nil + } + return dflt } From 123de138d2f3a57247a518b57b9367e3070fe275 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Wed, 15 Mar 2023 11:39:55 +0800 Subject: [PATCH 33/53] fix: Optimization Code --- pkg/proc/gdbserial/gdbserver.go | 6 ++-- pkg/proc/gdbserial/rr.go | 4 +-- pkg/proc/native/proc.go | 6 ++-- pkg/proc/proc_test.go | 50 +++++++++++++++------------------ pkg/proc/redirect.go | 5 ---- service/dap/server_test.go | 16 ++--------- service/debugger/debugger.go | 6 ++-- 7 files changed, 35 insertions(+), 58 deletions(-) diff --git a/pkg/proc/gdbserial/gdbserver.go b/pkg/proc/gdbserial/gdbserver.go index 26cc337364..2bb871991e 100644 --- a/pkg/proc/gdbserial/gdbserver.go +++ b/pkg/proc/gdbserial/gdbserver.go @@ -450,7 +450,7 @@ func getLdEnvVars() []string { // LLDBLaunch starts an instance of lldb-server and connects to it, asking // it to launch the specified target program with the specified arguments // (cmd) on the specified directory wd. -func LLDBLaunch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []string, tty string, redirects [3]proc.OutputRedirect) (*proc.TargetGroup, error) { +func LLDBLaunch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []string, tty string, redirects [3]string) (*proc.TargetGroup, error) { if runtime.GOOS == "windows" { return nil, ErrUnsupportedOS } @@ -484,10 +484,10 @@ func LLDBLaunch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs [ found := [3]bool{} names := [3]string{"stdin", "stdout", "stderr"} for i := range redirects { - if redirects[i].Path != "" { + if redirects[i] != "" { found[i] = true hasRedirects = true - args = append(args, fmt.Sprintf("--%s-path", names[i]), redirects[i].Path) + args = append(args, fmt.Sprintf("--%s-path", names[i]), redirects[i]) } } diff --git a/pkg/proc/gdbserial/rr.go b/pkg/proc/gdbserial/rr.go index 84bfd6ea67..c75a433190 100644 --- a/pkg/proc/gdbserial/rr.go +++ b/pkg/proc/gdbserial/rr.go @@ -86,9 +86,7 @@ func openRedirects(redirects [3]proc.OutputRedirect, quiet bool) (stdin, stdout, } return f - } - - if redirect.File != nil { + } else if redirect.File != nil { toclose = append(toclose, redirect.File) return redirect.File diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go index c045b77e76..46832024a9 100644 --- a/pkg/proc/native/proc.go +++ b/pkg/proc/native/proc.go @@ -381,7 +381,7 @@ func openRedirects(redirects [3]proc.OutputRedirect, foreground bool) (stdin, st return nil, nil, nil, nil, err } toclose = append(toclose, stdin) - } else { + } else if foreground { stdin = os.Stdin } @@ -393,9 +393,7 @@ func openRedirects(redirects [3]proc.OutputRedirect, foreground bool) (stdin, st } return f - } - - if redirect.File != nil { + } else if redirect.File != nil { toclose = append(toclose, redirect.File) return redirect.File diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 333f21b961..09abaf3a13 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -106,7 +106,7 @@ func withTestProcessArgs(name string, t testing.TB, wd string, args []string, bu case "native": grp, err = native.Launch(append([]string{fixture.Path}, args...), wd, 0, []string{}, "", proc.NewEmptyRedirect()) case "lldb": - grp, err = gdbserial.LLDBLaunch(append([]string{fixture.Path}, args...), wd, 0, []string{}, "", proc.NewEmptyRedirect()) + grp, err = gdbserial.LLDBLaunch(append([]string{fixture.Path}, args...), wd, 0, []string{}, "", [3]string{}) case "rr": protest.MustHaveRecordingAllowed(t) t.Log("recording") @@ -2220,7 +2220,7 @@ func TestUnsupportedArch(t *testing.T) { case "native": p, err = native.Launch([]string{outfile}, ".", 0, []string{}, "", proc.NewEmptyRedirect()) case "lldb": - p, err = gdbserial.LLDBLaunch([]string{outfile}, ".", 0, []string{}, "", proc.NewEmptyRedirect()) + p, err = gdbserial.LLDBLaunch([]string{outfile}, ".", 0, []string{}, "", [3]string{}) default: t.Skip("test not valid for this backend") } @@ -5936,7 +5936,7 @@ func TestStacktraceExtlinkMac(t *testing.T) { }) } -func testGenRedirect(t *testing.T, fixture protest.Fixture, stdoutExpectFile string, stderrExpectFile string, errChan chan error) (redirect [3]proc.OutputRedirect, cancelFunc func(), err error) { +func testGenRedirect(t *testing.T, fixture protest.Fixture, expectStdout string, expectStderr string, errChan chan error) (redirect [3]proc.OutputRedirect, cancelFunc func(), err error) { redirector, err := proc.NewRedirector() if err != nil { return redirect, nil, err @@ -5960,21 +5960,15 @@ func testGenRedirect(t *testing.T, fixture protest.Fixture, stdoutExpectFile str // redirector.Reader() will be blocked. go func() { - reader := func(mode string, f io.ReadCloser, expectFile string) { - expect, err := os.ReadFile(filepath.Join(fixture.BuildDir, expectFile)) - if err != nil { - errChan <- err - return - } - + reader := func(mode string, f io.ReadCloser, expectOut string) { out, err := io.ReadAll(f) if err != nil { errChan <- err return } - if string(expect) != string(out) { - errChan <- fmt.Errorf("%s,Not as expected!\nexpect:%s\nout:%s", mode, expect, out) + if expectOut != string(out) { + errChan <- fmt.Errorf("%s,Not as expected!\nexpect:%s\nout:%s", mode, expectOut, out) return } errChan <- nil @@ -5985,8 +5979,8 @@ func testGenRedirect(t *testing.T, fixture protest.Fixture, stdoutExpectFile str errChan <- err return } - go reader("stdout", readers[0], stdoutExpectFile) - go reader("stderr", readers[1], stderrExpectFile) + go reader("stdout", readers[0], expectStdout) + go reader("stderr", readers[1], expectStderr) }() return redirect, cancelFunc, nil @@ -5995,26 +5989,26 @@ func testGenRedirect(t *testing.T, fixture protest.Fixture, stdoutExpectFile str func TestRedirect(t *testing.T) { fixture := protest.BuildFixture("out_redirect", 0) var ( - grp *proc.TargetGroup - tracedir string - err error - redirect [3]proc.OutputRedirect = proc.NewEmptyRedirect() - errChan = make(chan error, 2) - cancelFunc func() - needCheck = false - stdoutExpectFile = "out_redirect-stdout.txt" - stderrExpectFile = "out_redirect-stderr.txt" + grp *proc.TargetGroup + tracedir string + err error + redirect [3]proc.OutputRedirect = proc.NewEmptyRedirect() + errChan = make(chan error, 2) + cancelFunc func() + needCheck = false + expectStdout = "hello world!\nhello world! error!" + expectStderr = "hello world!\nhello world!" ) switch testBackend { case "native": if runtime.GOOS == "linux" { - redirect, cancelFunc, err = testGenRedirect(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + redirect, cancelFunc, err = testGenRedirect(t, fixture, expectStdout, expectStderr, errChan) if err != nil { break } needCheck = true } else if runtime.GOOS == "windows" { - redirect, cancelFunc, err = testGenRedirect(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + redirect, cancelFunc, err = testGenRedirect(t, fixture, expectStdout, expectStderr, errChan) if err != nil { break } @@ -6024,18 +6018,18 @@ func TestRedirect(t *testing.T) { grp, err = native.Launch([]string{fixture.Path}, ".", 0, []string{}, "", redirect) case "lldb": if runtime.GOOS == "darwin" { - redirect, cancelFunc, err = testGenRedirect(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + redirect, cancelFunc, err = testGenRedirect(t, fixture, expectStdout, expectStderr, errChan) if err != nil { break } needCheck = true } - grp, err = gdbserial.LLDBLaunch([]string{fixture.Path}, ".", 0, []string{}, "", redirect) + grp, err = gdbserial.LLDBLaunch([]string{fixture.Path}, ".", 0, []string{}, "", [3]string{redirect[0].Path, redirect[1].Path, redirect[2].Path}) case "rr": protest.MustHaveRecordingAllowed(t) t.Log("recording") if runtime.GOOS != "windows" { - redirect, cancelFunc, err = testGenRedirect(t, fixture, stdoutExpectFile, stderrExpectFile, errChan) + redirect, cancelFunc, err = testGenRedirect(t, fixture, expectStdout, expectStderr, errChan) if err != nil { break } diff --git a/pkg/proc/redirect.go b/pkg/proc/redirect.go index fedcefb050..9e03a921b9 100644 --- a/pkg/proc/redirect.go +++ b/pkg/proc/redirect.go @@ -9,11 +9,6 @@ type OutputRedirect struct { File *os.File } -const ( - RedirectFileMode Mode = "file" - RedirectPathMode Mode = "path" -) - func NewEmptyRedirect() [3]OutputRedirect { return [3]OutputRedirect{} } diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 71e61803af..d3be2dcb6a 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -7429,26 +7429,16 @@ func TestRedirect(t *testing.T) { } var ( - stdoutFilePath = filepath.Join(fixture.BuildDir, "out_redirect-stdout.txt") - stderrFilePath = filepath.Join(fixture.BuildDir, "out_redirect-stderr.txt") + expectStdout = "hello world!\nhello world! error!" + expectStderr = "hello world!\nhello world!" ) // check output - expectStdout, err := os.ReadFile(stdoutFilePath) - if err != nil { - t.Errorf("\n failed to read file: %s", stdoutFilePath) - } - if string(expectStdout) != stdout.String() { t.Errorf("\n got stdout: len:%d\n%s\nwant: len:%d\n%s", stdout.Len(), stdout.String(), len(expectStdout), string(expectStdout)) } - expectStderr, err := os.ReadFile(stderrFilePath) - if err != nil { - t.Errorf("\n failed to read file: %s", stderrFilePath) - } - - if string(expectStderr) != stderr.String() { + if expectStderr != stderr.String() { t.Errorf("\n got stderr: len:%d \n%s\nwant: len:%d\n%s", stderr.Len(), stderr.String(), len(expectStderr), string(expectStderr)) } diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index 1ab7e16ed0..eca486470d 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -259,7 +259,8 @@ func (d *Debugger) Launch(processArgs []string, wd string) (*proc.TargetGroup, e case "native": return native.Launch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, d.config.Redirect) case "lldb": - return betterGdbserialLaunchError(gdbserial.LLDBLaunch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, d.config.Redirect)) + redirect := d.config.Redirect + return betterGdbserialLaunchError(gdbserial.LLDBLaunch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, [3]string{redirect[0].Path, redirect[1].Path, redirect[2].Path})) case "rr": if d.target != nil { // restart should not call us if the backend is 'rr' @@ -301,7 +302,8 @@ func (d *Debugger) Launch(processArgs []string, wd string) (*proc.TargetGroup, e case "default": if runtime.GOOS == "darwin" { - return betterGdbserialLaunchError(gdbserial.LLDBLaunch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, d.config.Redirect)) + redirect := d.config.Redirect + return betterGdbserialLaunchError(gdbserial.LLDBLaunch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, [3]string{redirect[0].Path, redirect[1].Path, redirect[2].Path})) } return native.Launch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, d.config.Redirect) default: From 980d43014a3077671fb76019c7f5b56d4b505423 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Wed, 15 Mar 2023 16:03:08 +0800 Subject: [PATCH 34/53] fix: utiltest --- pkg/proc/gdbserial/rr.go | 2 +- service/dap/server_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/proc/gdbserial/rr.go b/pkg/proc/gdbserial/rr.go index c75a433190..02d100d047 100644 --- a/pkg/proc/gdbserial/rr.go +++ b/pkg/proc/gdbserial/rr.go @@ -86,7 +86,7 @@ func openRedirects(redirects [3]proc.OutputRedirect, quiet bool) (stdin, stdout, } return f - } else if redirect.File != nil { + }else if redirect.File != nil { toclose = append(toclose, redirect.File) return redirect.File diff --git a/service/dap/server_test.go b/service/dap/server_test.go index d3be2dcb6a..695061ef14 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -7434,7 +7434,7 @@ func TestRedirect(t *testing.T) { ) // check output - if string(expectStdout) != stdout.String() { + if expectStdout != stdout.String() { t.Errorf("\n got stdout: len:%d\n%s\nwant: len:%d\n%s", stdout.Len(), stdout.String(), len(expectStdout), string(expectStdout)) } From 72c6718fb9a84deb5df41c1b897e50b158962677 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Wed, 15 Mar 2023 23:56:48 +0800 Subject: [PATCH 35/53] fix: execpt out --- pkg/proc/proc_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 09abaf3a13..1ce3e7788b 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -5996,8 +5996,8 @@ func TestRedirect(t *testing.T) { errChan = make(chan error, 2) cancelFunc func() needCheck = false - expectStdout = "hello world!\nhello world! error!" - expectStderr = "hello world!\nhello world!" + expectStdout = "hello world!\nhello world!" + expectStderr = "hello world!\nhello world! error!" ) switch testBackend { case "native": From f83ec151b1b55400060509138c99bfe87f4038a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Sat, 18 Mar 2023 17:00:08 +0800 Subject: [PATCH 36/53] fix: Resource release for redirects --- _fixtures/out_redirect-stderr.txt | 2 - _fixtures/out_redirect-stdout.txt | 2 - .../{stdio_pipe.go => namedpipe_other.go} | 41 +++++++++++++++---- ...o_pipe_windows.go => namedpipe_windows.go} | 15 ++++--- service/dap/server.go | 13 +++++- service/dap/server_test.go | 4 +- 6 files changed, 57 insertions(+), 20 deletions(-) delete mode 100644 _fixtures/out_redirect-stderr.txt delete mode 100644 _fixtures/out_redirect-stdout.txt rename pkg/proc/{stdio_pipe.go => namedpipe_other.go} (57%) rename pkg/proc/{stdio_pipe_windows.go => namedpipe_windows.go} (67%) diff --git a/_fixtures/out_redirect-stderr.txt b/_fixtures/out_redirect-stderr.txt deleted file mode 100644 index bd9b413af3..0000000000 --- a/_fixtures/out_redirect-stderr.txt +++ /dev/null @@ -1,2 +0,0 @@ -hello world! -hello world! error! \ No newline at end of file diff --git a/_fixtures/out_redirect-stdout.txt b/_fixtures/out_redirect-stdout.txt deleted file mode 100644 index a2348d6e3f..0000000000 --- a/_fixtures/out_redirect-stdout.txt +++ /dev/null @@ -1,2 +0,0 @@ -hello world! -hello world! \ No newline at end of file diff --git a/pkg/proc/stdio_pipe.go b/pkg/proc/namedpipe_other.go similarity index 57% rename from pkg/proc/stdio_pipe.go rename to pkg/proc/namedpipe_other.go index 542f9d7d62..d74704365f 100644 --- a/pkg/proc/stdio_pipe.go +++ b/pkg/proc/namedpipe_other.go @@ -9,14 +9,16 @@ import ( "io" "os" "path/filepath" + "sync/atomic" "syscall" ) -type stdioRedirector struct { - Paths [3]string +type StdioRedirector struct { + Paths [3]string + needClear int32 } -func NewRedirector() (redirect *stdioRedirector, err error) { +func NewRedirector() (redirect *StdioRedirector, err error) { r := make([]byte, 4) if _, err := rand.Read(r); err != nil { return redirect, err @@ -37,11 +39,11 @@ func NewRedirector() (redirect *stdioRedirector, err error) { return redirect, err } - return &stdioRedirector{Paths: [3]string{"", stdoutPath, stderrPath}}, nil + return &StdioRedirector{Paths: [3]string{"", stdoutPath, stderrPath}}, nil } // Writer return [3]{stdin,stdout,stderr} -func (s *stdioRedirector) Writer() [3]OutputRedirect { +func (s *StdioRedirector) Writer() [3]OutputRedirect { return NewRedirectByPath(s.Paths) } @@ -61,9 +63,12 @@ func newWarpClose(file *os.File, path string) io.ReadCloser { // Reader return []{stdout,stderr}. // Delete the file when the Close interface is called. -// OpenFile(path,os.O_RDONLY,os.ModeNamedPipe) will be blocked. +// OpenFile(path,os.O_RDONLY,os.ModeNamedPipe) will be blocked. // The Reader should be called asynchronously. -func (s *stdioRedirector) Reader() (reader [2]io.ReadCloser, err error) { +func (s *StdioRedirector) Reader() (reader [2]io.ReadCloser, err error) { + atomic.StoreInt32(&s.needClear, 1) + defer atomic.StoreInt32(&s.needClear, 0) + stdoutFile, err := os.OpenFile(s.Paths[1], os.O_RDONLY, os.ModeNamedPipe) if err != nil { return reader, err @@ -79,3 +84,25 @@ func (s *stdioRedirector) Reader() (reader [2]io.ReadCloser, err error) { return reader, nil } + +// Clean up resources created by redirects. +// When the s.Reader() method is blocked by os.OpenFile(only-read), +// it opens the file in write mode and call file.Close(). +func (s *StdioRedirector) Clean() error { + if atomic.LoadInt32(&s.needClear) == 1 { + stdoutFile, err := os.OpenFile(s.Paths[1], os.O_WRONLY, os.ModeNamedPipe) + if err != nil { + return err + } + + stderrFile, err := os.OpenFile(s.Paths[2], os.O_WRONLY, os.ModeNamedPipe) + if err != nil { + return err + } + + stderrFile.Close() + stdoutFile.Close() + } + + return nil +} diff --git a/pkg/proc/stdio_pipe_windows.go b/pkg/proc/namedpipe_windows.go similarity index 67% rename from pkg/proc/stdio_pipe_windows.go rename to pkg/proc/namedpipe_windows.go index 9c06324e08..161eacfd96 100644 --- a/pkg/proc/stdio_pipe_windows.go +++ b/pkg/proc/namedpipe_windows.go @@ -8,12 +8,12 @@ import ( "os" ) -type stdioRedirector struct { +type StdioRedirector struct { writers [3]*os.File readers [3]*os.File } -func NewRedirector() (redirect *stdioRedirector, err error) { +func NewRedirector() (redirect *StdioRedirector, err error) { stdoutReader, stdoutWriter, err := os.Pipe() if err != nil { return redirect, err @@ -24,20 +24,25 @@ func NewRedirector() (redirect *stdioRedirector, err error) { return redirect, err } - return &stdioRedirector{ + return &StdioRedirector{ writers: [3]*os.File{nil, stdoutWriter, stderrWriter}, readers: [3]*os.File{nil, stdoutReader, stderrReader}, }, err } // Writer return [3]{stdin,stdout,stderr} -func (s *stdioRedirector) Writer() [3]OutputRedirect { +func (s *StdioRedirector) Writer() [3]OutputRedirect { return NewRedirectByFile(s.writers) } // Reader return [2]{stdout,stderr} -func (s *stdioRedirector) Reader() (reader [2]io.ReadCloser, err error) { +func (s *StdioRedirector) Reader() (reader [2]io.ReadCloser, err error) { reader[0] = s.readers[1] reader[1] = s.readers[2] return reader, nil } + +// Clean +func (s *StdioRedirector) Clean() error { + return nil +} diff --git a/service/dap/server.go b/service/dap/server.go index b3a4a7f5a3..0d1c0477ce 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -1103,8 +1103,9 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { return } + var redirects *proc.StdioRedirector if redirected { - redirects, err := proc.NewRedirector() + redirects, err = proc.NewRedirector() if err != nil { s.sendShowUserErrorResponse(request.Request, InternalError, "Internal Error", fmt.Sprintf("failed to generate stdio pipes - %v", err)) @@ -1112,8 +1113,9 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { } s.config.Debugger.Redirect = redirects.Writer() - + s.preTerminatedWG.Add(1) go func() { + defer s.preTerminatedWG.Done() readers, err := redirects.Reader() if err != nil { s.sendShowUserErrorResponse(request.Request, InternalError, "Internal Error", @@ -1131,6 +1133,12 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { }() if err != nil { s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", err.Error()) + if redirected && redirects != nil { + // On startup failure, clean up the resources created for the redirect. + if clearErr := redirects.Clean(); clearErr != nil { + s.config.log.Warnf("failed to clear redirects - %v", clearErr) + } + } return } // Enable StepBack controls on supported backends @@ -1160,6 +1168,7 @@ func (s *Session) getPackageDir(pkg string) string { func (s *Session) newNoDebugProcess(program string, targetArgs []string, wd string, redirected bool) (cmd *exec.Cmd, err error) { if s.noDebugProcess != nil { return nil, fmt.Errorf("another launch request is in progress") + } cmd = exec.Command(program, targetArgs...) diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 695061ef14..af7a2f9fee 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -7429,8 +7429,8 @@ func TestRedirect(t *testing.T) { } var ( - expectStdout = "hello world!\nhello world! error!" - expectStderr = "hello world!\nhello world!" + expectStdout = "hello world!\nhello world!" + expectStderr = "hello world!\nhello world! error!" ) // check output From 31dba4953c741dc63202d1f1bb8b4642a1bbab0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Sun, 19 Mar 2023 02:55:38 +0800 Subject: [PATCH 37/53] fix: build failure --- pkg/proc/proc_linux_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/proc/proc_linux_test.go b/pkg/proc/proc_linux_test.go index db82b7f972..381c014ba0 100644 --- a/pkg/proc/proc_linux_test.go +++ b/pkg/proc/proc_linux_test.go @@ -15,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)}, "", proc.NewEmptyRedirectByPath()) + p, err := native.Launch(append([]string{fixture.Path}, ""), "", 0, []string{filepath.Dir(fixture.Path)}, "", proc.NewEmptyRedirect()) if err != nil { t.Fatal(err) } From 8ebdb9d9d8891041258b1dbaecb46aa5f955c8c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Sun, 19 Mar 2023 03:43:19 +0800 Subject: [PATCH 38/53] fix: clean->clear --- pkg/proc/namedpipe_windows.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/proc/namedpipe_windows.go b/pkg/proc/namedpipe_windows.go index 161eacfd96..ce8bb2b45b 100644 --- a/pkg/proc/namedpipe_windows.go +++ b/pkg/proc/namedpipe_windows.go @@ -42,7 +42,7 @@ func (s *StdioRedirector) Reader() (reader [2]io.ReadCloser, err error) { return reader, nil } -// Clean -func (s *StdioRedirector) Clean() error { +// Clear +func (s *StdioRedirector) Clear() error { return nil } From 7d441c28ba41e6ae18dce691dd885650b6b5d06b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Sun, 19 Mar 2023 15:03:10 +0800 Subject: [PATCH 39/53] fix: build failure --- pkg/proc/namedpipe_other.go | 4 ++-- service/dap/server.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/proc/namedpipe_other.go b/pkg/proc/namedpipe_other.go index d74704365f..98ca38723b 100644 --- a/pkg/proc/namedpipe_other.go +++ b/pkg/proc/namedpipe_other.go @@ -85,10 +85,10 @@ func (s *StdioRedirector) Reader() (reader [2]io.ReadCloser, err error) { return reader, nil } -// Clean up resources created by redirects. +// Clear Clean up resources created by redirects. // When the s.Reader() method is blocked by os.OpenFile(only-read), // it opens the file in write mode and call file.Close(). -func (s *StdioRedirector) Clean() error { +func (s *StdioRedirector) Clear() error { if atomic.LoadInt32(&s.needClear) == 1 { stdoutFile, err := os.OpenFile(s.Paths[1], os.O_WRONLY, os.ModeNamedPipe) if err != nil { diff --git a/service/dap/server.go b/service/dap/server.go index 0d1c0477ce..b9a2d2b7cd 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -1135,7 +1135,7 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", err.Error()) if redirected && redirects != nil { // On startup failure, clean up the resources created for the redirect. - if clearErr := redirects.Clean(); clearErr != nil { + if clearErr := redirects.Clear(); clearErr != nil { s.config.log.Warnf("failed to clear redirects - %v", clearErr) } } From ca76f9d1fd33a4cd256001c7a485513d3bfac220 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Sun, 19 Mar 2023 18:07:09 +0800 Subject: [PATCH 40/53] fix: test failure --- service/test/integration2_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/service/test/integration2_test.go b/service/test/integration2_test.go index dd66f87cb8..c4e4b94fcd 100644 --- a/service/test/integration2_test.go +++ b/service/test/integration2_test.go @@ -18,7 +18,6 @@ import ( "testing" "time" - "github.com/go-delve/delve/pkg/proc" protest "github.com/go-delve/delve/pkg/proc/test" "github.com/go-delve/delve/service/debugger" From 36d0e924ceea0388093ce881c38f8ed60700eb15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Tue, 21 Mar 2023 02:59:55 +0800 Subject: [PATCH 41/53] fix: Optimization Code --- pkg/proc/gdbserial/rr.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/proc/gdbserial/rr.go b/pkg/proc/gdbserial/rr.go index 684cf91a45..b20137d179 100644 --- a/pkg/proc/gdbserial/rr.go +++ b/pkg/proc/gdbserial/rr.go @@ -74,7 +74,7 @@ func openRedirects(redirects [3]proc.OutputRedirect, quiet bool) (stdin, stdout, return nil, nil, nil, nil, err } toclose = append(toclose, stdin) - } else if quiet { + } else { stdin = os.Stdin } @@ -86,7 +86,7 @@ func openRedirects(redirects [3]proc.OutputRedirect, quiet bool) (stdin, stdout, } return f - }else if redirect.File != nil { + } else if redirect.File != nil { toclose = append(toclose, redirect.File) return redirect.File From 6e33d4a343c3aaf81f770ff6e3f237ad2a2fcd2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Mon, 27 Mar 2023 15:34:52 +0800 Subject: [PATCH 42/53] style: remove useless code --- pkg/proc/redirect.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/proc/redirect.go b/pkg/proc/redirect.go index 9e03a921b9..d28f58a9e8 100644 --- a/pkg/proc/redirect.go +++ b/pkg/proc/redirect.go @@ -2,8 +2,6 @@ package proc import "os" -type Mode string - type OutputRedirect struct { Path string File *os.File From 702341da52553e396ead17aaf3a4a9ffad45445f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Sun, 2 Apr 2023 03:44:38 +0800 Subject: [PATCH 43/53] refactor: namedpipe --- pkg/proc/namedpipe_other.go | 120 ++++++++++++++-------------------- pkg/proc/namedpipe_windows.go | 37 ++--------- pkg/proc/proc_test.go | 22 +------ service/dap/server.go | 30 ++++----- 4 files changed, 69 insertions(+), 140 deletions(-) diff --git a/pkg/proc/namedpipe_other.go b/pkg/proc/namedpipe_other.go index 98ca38723b..99c5d1571e 100644 --- a/pkg/proc/namedpipe_other.go +++ b/pkg/proc/namedpipe_other.go @@ -13,96 +13,72 @@ import ( "syscall" ) -type StdioRedirector struct { - Paths [3]string - needClear int32 +type openOnRead struct { + p string + rd io.ReadCloser + status int32 // 0: initialization 1: file is opened 2: file is closed } -func NewRedirector() (redirect *StdioRedirector, err error) { - r := make([]byte, 4) - if _, err := rand.Read(r); err != nil { - return redirect, err +func (oor *openOnRead) Read(p []byte) (n int, err error) { + if oor.rd != nil { + return oor.rd.Read(p) } - var ( - prefix = filepath.Join(os.TempDir(), hex.EncodeToString(r)) - stdoutPath = prefix + "stdout" - stderrPath = prefix + "stderr" - ) - - if err := syscall.Mkfifo(stdoutPath, 0o600); err != nil { - return redirect, err + // try from "into" to "open" + if !atomic.CompareAndSwapInt32(&oor.status, 0, 1) { + return 0, io.EOF } - if err := syscall.Mkfifo(stderrPath, 0o600); err != nil { - _ = os.Remove(stdoutPath) - return redirect, err + fh, err := os.OpenFile(oor.p, os.O_RDONLY, os.ModeNamedPipe) + if err != nil { + return 0, err } - return &StdioRedirector{Paths: [3]string{"", stdoutPath, stderrPath}}, nil -} - -// Writer return [3]{stdin,stdout,stderr} -func (s *StdioRedirector) Writer() [3]OutputRedirect { - return NewRedirectByPath(s.Paths) + oor.rd = fh + return oor.rd.Read(p) } -type warpClose struct { - *os.File - path string -} +func (oor *openOnRead) Close() error { + defer os.Remove(oor.p) + + // try from "init" to "close". + if !atomic.CompareAndSwapInt32(&oor.status, 0, 2) { + // 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 + } + } + } -func (s *warpClose) Close() error { - defer os.Remove(s.path) - return s.File.Close() + return oor.rd.Close() } -func newWarpClose(file *os.File, path string) io.ReadCloser { - return &warpClose{File: file, path: path} -} +func NamedPipe() (reader [2]io.ReadCloser, output [3]OutputRedirect, err error) { + r := make([]byte, 4) + if _, err = rand.Read(r); err != nil { + return reader, output, err + } -// Reader return []{stdout,stderr}. -// Delete the file when the Close interface is called. -// OpenFile(path,os.O_RDONLY,os.ModeNamedPipe) will be blocked. -// The Reader should be called asynchronously. -func (s *StdioRedirector) Reader() (reader [2]io.ReadCloser, err error) { - atomic.StoreInt32(&s.needClear, 1) - defer atomic.StoreInt32(&s.needClear, 0) + var ( + prefix = filepath.Join(os.TempDir(), hex.EncodeToString(r)) + stdoutPath = prefix + "stdout" + stderrPath = prefix + "stderr" + ) - stdoutFile, err := os.OpenFile(s.Paths[1], os.O_RDONLY, os.ModeNamedPipe) - if err != nil { - return reader, err + if err = syscall.Mkfifo(stdoutPath, 0o600); err != nil { + return reader, output, err } - stderrFile, err := os.OpenFile(s.Paths[2], os.O_RDONLY, os.ModeNamedPipe) - if err != nil { - return reader, err + if err := syscall.Mkfifo(stderrPath, 0o600); err != nil { + _ = os.Remove(stdoutPath) + return reader, output, err } - reader[0] = newWarpClose(stdoutFile, s.Paths[1]) - reader[1] = newWarpClose(stderrFile, s.Paths[2]) - - return reader, nil -} - -// Clear Clean up resources created by redirects. -// When the s.Reader() method is blocked by os.OpenFile(only-read), -// it opens the file in write mode and call file.Close(). -func (s *StdioRedirector) Clear() error { - if atomic.LoadInt32(&s.needClear) == 1 { - stdoutFile, err := os.OpenFile(s.Paths[1], os.O_WRONLY, os.ModeNamedPipe) - if err != nil { - return err - } - - stderrFile, err := os.OpenFile(s.Paths[2], os.O_WRONLY, os.ModeNamedPipe) - if err != nil { - return err - } - - stderrFile.Close() - stdoutFile.Close() - } + reader[0] = &openOnRead{p: stdoutPath} + reader[1] = &openOnRead{p: stderrPath} - return nil + return reader, NewRedirectByPath([3]string{"", stdoutPath, stderrPath}), nil } diff --git a/pkg/proc/namedpipe_windows.go b/pkg/proc/namedpipe_windows.go index ce8bb2b45b..3f70d7bf90 100644 --- a/pkg/proc/namedpipe_windows.go +++ b/pkg/proc/namedpipe_windows.go @@ -8,41 +8,16 @@ import ( "os" ) -type StdioRedirector struct { - writers [3]*os.File - readers [3]*os.File -} - -func NewRedirector() (redirect *StdioRedirector, err error) { - stdoutReader, stdoutWriter, err := os.Pipe() +func NamedPipe() (reader [2]io.ReadCloser, output [3]OutputRedirect, err error) { + reader[0], output[1].File, err = os.Pipe() if err != nil { - return redirect, err + return reader, output, err } - stderrReader, stderrWriter, err := os.Pipe() + reader[1], output[2].File, err = os.Pipe() if err != nil { - return redirect, err + return reader, output, err } - return &StdioRedirector{ - writers: [3]*os.File{nil, stdoutWriter, stderrWriter}, - readers: [3]*os.File{nil, stdoutReader, stderrReader}, - }, err -} - -// Writer return [3]{stdin,stdout,stderr} -func (s *StdioRedirector) Writer() [3]OutputRedirect { - return NewRedirectByFile(s.writers) -} - -// Reader return [2]{stdout,stderr} -func (s *StdioRedirector) Reader() (reader [2]io.ReadCloser, err error) { - reader[0] = s.readers[1] - reader[1] = s.readers[2] - return reader, nil -} - -// Clear -func (s *StdioRedirector) Clear() error { - return nil + return reader, output, nil } diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 56b19fd5d3..7c69c089a5 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -5928,28 +5928,17 @@ func TestStacktraceExtlinkMac(t *testing.T) { } func testGenRedirect(t *testing.T, fixture protest.Fixture, expectStdout string, expectStderr string, errChan chan error) (redirect [3]proc.OutputRedirect, cancelFunc func(), err error) { - redirector, err := proc.NewRedirector() + readers, redirect, err := proc.NamedPipe() if err != nil { return redirect, nil, err } - redirect = redirector.Writer() cancelFunc = func() { - for _, outputRedirect := range redirect { - if outputRedirect.File != nil { - _ = outputRedirect.File.Close() - } - - if outputRedirect.Path != "" { - stdio, err := os.OpenFile(outputRedirect.Path, os.O_WRONLY, os.ModeNamedPipe) - if err == nil { - stdio.Close() - } - } + for _, reader := range readers { + _ = reader.Close() } } - // redirector.Reader() will be blocked. go func() { reader := func(mode string, f io.ReadCloser, expectOut string) { out, err := io.ReadAll(f) @@ -5965,11 +5954,6 @@ func testGenRedirect(t *testing.T, fixture protest.Fixture, expectStdout string, errChan <- nil } - readers, err := redirector.Reader() - if err != nil { - errChan <- err - return - } go reader("stdout", readers[0], expectStdout) go reader("stderr", readers[1], expectStderr) }() diff --git a/service/dap/server.go b/service/dap/server.go index b9a2d2b7cd..3e7ce7b7b8 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -1103,27 +1103,24 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { return } - var redirects *proc.StdioRedirector + var clear func() if redirected { - redirects, err = proc.NewRedirector() + var readers [2]io.ReadCloser + readers, s.config.Debugger.Redirect, err = proc.NamedPipe() if err != nil { s.sendShowUserErrorResponse(request.Request, InternalError, "Internal Error", fmt.Sprintf("failed to generate stdio pipes - %v", err)) return } - s.config.Debugger.Redirect = redirects.Writer() - s.preTerminatedWG.Add(1) - go func() { - defer s.preTerminatedWG.Done() - readers, err := redirects.Reader() - if err != nil { - s.sendShowUserErrorResponse(request.Request, InternalError, "Internal Error", - fmt.Sprintf("failed to open pipe - %v", err)) - return + redirectedFunc(readers[0], readers[1]) + clear = func() { + for index := range readers { + if closeErr := readers[index].Close(); closeErr != nil { + s.config.log.Warnf("failed to clear redirects - %v", closeErr) + } } - redirectedFunc(readers[0], readers[1]) - }() + } } func() { @@ -1133,11 +1130,8 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { }() if err != nil { s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", err.Error()) - if redirected && redirects != nil { - // On startup failure, clean up the resources created for the redirect. - if clearErr := redirects.Clear(); clearErr != nil { - s.config.log.Warnf("failed to clear redirects - %v", clearErr) - } + if redirected { + clear() } return } From 5aeb3f8773be1efd40f10ea4b1b6d42ed0f41e3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Wed, 5 Apr 2023 02:57:32 +0800 Subject: [PATCH 44/53] refactor: namedpipe, launch ... --- cmd/dlv/cmds/commands.go | 4 +- pkg/proc/gdbserial/rr.go | 22 +++--- pkg/proc/gdbserial/rr_test.go | 2 +- pkg/proc/namedpipe_other.go | 49 +++--------- pkg/proc/namedpipe_windows.go | 14 +--- pkg/proc/native/nonative_darwin.go | 2 +- pkg/proc/native/proc.go | 10 +-- pkg/proc/native/proc_linux.go | 4 +- pkg/proc/native/proc_windows.go | 4 +- pkg/proc/proc_linux_test.go | 2 +- pkg/proc/proc_test.go | 121 +---------------------------- pkg/proc/redirect.go | 4 +- service/dap/server.go | 21 +++-- service/dap/server_test.go | 4 +- service/debugger/debugger.go | 28 ++++--- service/test/integration2_test.go | 4 +- 16 files changed, 83 insertions(+), 212 deletions(-) diff --git a/cmd/dlv/cmds/commands.go b/cmd/dlv/cmds/commands.go index 3de12f34fb..bdc6a2ece9 100644 --- a/cmd/dlv/cmds/commands.go +++ b/cmd/dlv/cmds/commands.go @@ -986,7 +986,9 @@ func execute(attachPid int, processArgs []string, conf *config.Config, coreFile DebugInfoDirectories: conf.DebugInfoDirectories, CheckGoVersion: checkGoVersion, TTY: tty, - Redirect: proc.NewRedirectByPath(redirects), + Stdin: redirects[0], + Stdout: proc.OutputRedirect{Path: redirects[1]}, + Stderr: proc.OutputRedirect{Path: redirects[2]}, DisableASLR: disableASLR, RrOnProcessPid: rrOnProcessPid, }, diff --git a/pkg/proc/gdbserial/rr.go b/pkg/proc/gdbserial/rr.go index b20137d179..0492968a3e 100644 --- a/pkg/proc/gdbserial/rr.go +++ b/pkg/proc/gdbserial/rr.go @@ -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]proc.OutputRedirect) (run func() (string, error), stop func() error, err error) { +func RecordAsync(cmd []string, wd string, quiet bool, stdin string, stdout proc.OutputRedirect, stderr proc.OutputRedirect) (run func() (string, error), stop func() error, err error) { if err := checkRRAvailable(); err != nil { return nil, nil, err } @@ -35,7 +35,7 @@ func RecordAsync(cmd []string, wd string, quiet bool, redirects [3]proc.OutputRe args = append(args, cmd...) rrcmd := exec.Command("rr", args...) var closefn func() - rrcmd.Stdin, rrcmd.Stdout, rrcmd.Stderr, closefn, err = openRedirects(redirects, quiet) + rrcmd.Stdin, rrcmd.Stdout, rrcmd.Stderr, closefn, err = openRedirects(stdin, stdout, stderr, quiet) if err != nil { return nil, nil, err } @@ -63,13 +63,13 @@ func RecordAsync(cmd []string, wd string, quiet bool, redirects [3]proc.OutputRe return run, stop, nil } -func openRedirects(redirects [3]proc.OutputRedirect, quiet bool) (stdin, stdout, stderr *os.File, closefn func(), err error) { +func openRedirects(stdinPath string, stdoutOR proc.OutputRedirect, stderrOR proc.OutputRedirect, quiet bool) (stdin, stdout, stderr *os.File, closefn func(), err error) { var ( toclose = []*os.File{} ) - if redirects[0].Path != "" { - stdin, err = os.Open(redirects[0].Path) + if stdinPath != "" { + stdin, err = os.Open(stdinPath) if err != nil { return nil, nil, nil, nil, err } @@ -99,12 +99,12 @@ func openRedirects(redirects [3]proc.OutputRedirect, quiet bool) (stdin, stdout, return dflt } - stdout = create(redirects[1], os.Stdout) + stdout = create(stdoutOR, os.Stdout) if err != nil { return nil, nil, nil, nil, err } - stderr = create(redirects[2], os.Stderr) + stderr = create(stderrOR, os.Stderr) if err != nil { return nil, nil, nil, nil, err } @@ -120,8 +120,8 @@ func openRedirects(redirects [3]proc.OutputRedirect, quiet bool) (stdin, stdout, // 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]proc.OutputRedirect) (tracedir string, err error) { - run, _, err := RecordAsync(cmd, wd, quiet, redirects) +func Record(cmd []string, wd string, quiet bool, stdin string, stdout proc.OutputRedirect, stderr proc.OutputRedirect) (tracedir string, err error) { + run, _, err := RecordAsync(cmd, wd, quiet, stdin, stdout, stderr) if err != nil { return "", err } @@ -296,8 +296,8 @@ 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]proc.OutputRedirect) (*proc.TargetGroup, string, error) { - tracedir, err := Record(cmd, wd, quiet, redirects) +func RecordAndReplay(cmd []string, wd string, quiet bool, debugInfoDirs []string, stdin string, stdout proc.OutputRedirect, stderr proc.OutputRedirect) (*proc.TargetGroup, string, error) { + tracedir, err := Record(cmd, wd, quiet, stdin, stdout, stderr) if tracedir == "" { return nil, "", err } diff --git a/pkg/proc/gdbserial/rr_test.go b/pkg/proc/gdbserial/rr_test.go index 7c03d41d64..3eb97119c6 100644 --- a/pkg/proc/gdbserial/rr_test.go +++ b/pkg/proc/gdbserial/rr_test.go @@ -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{}, proc.NewEmptyRedirect()) + grp, tracedir, err := gdbserial.RecordAndReplay([]string{fixture.Path}, ".", true, []string{}, "", proc.OutputRedirect{}, proc.OutputRedirect{}) if err != nil { t.Fatal("Launch():", err) } diff --git a/pkg/proc/namedpipe_other.go b/pkg/proc/namedpipe_other.go index 99c5d1571e..ba76c95087 100644 --- a/pkg/proc/namedpipe_other.go +++ b/pkg/proc/namedpipe_other.go @@ -9,14 +9,12 @@ import ( "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 + path string + rd io.ReadCloser } func (oor *openOnRead) Read(p []byte) (n int, err error) { @@ -24,12 +22,7 @@ func (oor *openOnRead) Read(p []byte) (n int, err error) { 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) + fh, err := os.OpenFile(oor.path, os.O_RDONLY, os.ModeNamedPipe) if err != nil { return 0, err } @@ -39,46 +32,28 @@ func (oor *openOnRead) Read(p []byte) (n int, err error) { } func (oor *openOnRead) Close() error { - defer os.Remove(oor.p) + defer os.Remove(oor.path) - // try from "init" to "close". - if !atomic.CompareAndSwapInt32(&oor.status, 0, 2) { - // 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 - } - } + fh, _ := os.OpenFile(oor.path, os.O_WRONLY|syscall.O_NONBLOCK, 0) + if fh != nil { + fh.Close() } return oor.rd.Close() } -func NamedPipe() (reader [2]io.ReadCloser, output [3]OutputRedirect, err error) { +func NamedPipe() (reader io.ReadCloser, output OutputRedirect, err error) { 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 - } + var path = filepath.Join(os.TempDir(), hex.EncodeToString(r)) - if err := syscall.Mkfifo(stderrPath, 0o600); err != nil { - _ = os.Remove(stdoutPath) + if err = syscall.Mkfifo(path, 0o600); err != nil { + _ = os.Remove(path) return reader, output, err } - reader[0] = &openOnRead{p: stdoutPath} - reader[1] = &openOnRead{p: stderrPath} - - return reader, NewRedirectByPath([3]string{"", stdoutPath, stderrPath}), nil + return &openOnRead{path: path}, OutputRedirect{Path: path}, nil } diff --git a/pkg/proc/namedpipe_windows.go b/pkg/proc/namedpipe_windows.go index 3f70d7bf90..91dc6b2766 100644 --- a/pkg/proc/namedpipe_windows.go +++ b/pkg/proc/namedpipe_windows.go @@ -8,16 +8,8 @@ import ( "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 - } +func NamedPipe() (reader io.ReadCloser, output OutputRedirect, err error) { + reader, output.File, err = os.Pipe() - reader[1], output[2].File, err = os.Pipe() - if err != nil { - return reader, output, err - } - - return reader, output, nil + return reader, output, err } diff --git a/pkg/proc/native/nonative_darwin.go b/pkg/proc/native/nonative_darwin.go index 3c56159a4c..537c3be47d 100644 --- a/pkg/proc/native/nonative_darwin.go +++ b/pkg/proc/native/nonative_darwin.go @@ -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]proc.OutputRedirect) (*proc.TargetGroup, error) { +func Launch(_ []string, _ string, _ proc.LaunchFlags, _ []string, _ string, _ string, _ proc.OutputRedirect, _ proc.OutputRedirect) (*proc.TargetGroup, error) { return nil, ErrNativeBackendDisabled } diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go index 46832024a9..5ba0c26e5c 100644 --- a/pkg/proc/native/proc.go +++ b/pkg/proc/native/proc.go @@ -370,13 +370,13 @@ func (dbp *nativeProcess) writeSoftwareBreakpoint(thread *nativeThread, addr uin return err } -func openRedirects(redirects [3]proc.OutputRedirect, foreground bool) (stdin, stdout, stderr *os.File, closefn func(), err error) { +func openRedirects(stdinPath string, stdoutOR proc.OutputRedirect, stderrOR proc.OutputRedirect, foreground bool) (stdin, stdout, stderr *os.File, closefn func(), err error) { var ( toclose = []*os.File{} ) - if redirects[0].Path != "" { - stdin, err = os.Open(redirects[0].Path) + if stdinPath != "" { + stdin, err = os.Open(stdinPath) if err != nil { return nil, nil, nil, nil, err } @@ -402,12 +402,12 @@ func openRedirects(redirects [3]proc.OutputRedirect, foreground bool) (stdin, st return dflt } - stdout = create(redirects[1], os.Stdout) + stdout = create(stdoutOR, os.Stdout) if err != nil { return nil, nil, nil, nil, err } - stderr = create(redirects[2], os.Stderr) + stderr = create(stderrOR, os.Stderr) if err != nil { return nil, nil, nil, nil, err } diff --git a/pkg/proc/native/proc_linux.go b/pkg/proc/native/proc_linux.go index 3d29ea20ba..ffd6e1a73b 100644 --- a/pkg/proc/native/proc_linux.go +++ b/pkg/proc/native/proc_linux.go @@ -62,7 +62,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, redirect [3]proc.OutputRedirect) (*proc.TargetGroup, error) { +func Launch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []string, tty string, stdinPath string, stdoutOR proc.OutputRedirect, stderrOR proc.OutputRedirect) (*proc.TargetGroup, error) { var ( process *exec.Cmd err error @@ -70,7 +70,7 @@ func Launch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []str foreground := flags&proc.LaunchForeground != 0 - stdin, stdout, stderr, closefn, err := openRedirects(redirect, foreground) + stdin, stdout, stderr, closefn, err := openRedirects(stdinPath, stdoutOR, stderrOR, foreground) if err != nil { return nil, err } diff --git a/pkg/proc/native/proc_windows.go b/pkg/proc/native/proc_windows.go index ca0bb1279f..e1b603c6f8 100644 --- a/pkg/proc/native/proc_windows.go +++ b/pkg/proc/native/proc_windows.go @@ -23,12 +23,12 @@ 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]proc.OutputRedirect) (*proc.TargetGroup, error) { +func Launch(cmd []string, wd string, flags proc.LaunchFlags, _ []string, _ string, stdinPath string, stdoutOR proc.OutputRedirect, stderrOR proc.OutputRedirect) (*proc.TargetGroup, error) { argv0Go := cmd[0] env := proc.DisableAsyncPreemptEnv() - stdin, stdout, stderr, closefn, err := openRedirects(redirects, true) + stdin, stdout, stderr, closefn, err := openRedirects(stdinPath, stdoutOR, stderrOR, true) if err != nil { return nil, err } diff --git a/pkg/proc/proc_linux_test.go b/pkg/proc/proc_linux_test.go index 381c014ba0..f105ce9148 100644 --- a/pkg/proc/proc_linux_test.go +++ b/pkg/proc/proc_linux_test.go @@ -15,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)}, "", proc.NewEmptyRedirect()) + p, err := native.Launch(append([]string{fixture.Path}, ""), "", 0, []string{filepath.Dir(fixture.Path)}, "", "", proc.OutputRedirect{}, proc.OutputRedirect{}) if err != nil { t.Fatal(err) } diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 7c69c089a5..25fff18ed6 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -8,7 +8,6 @@ import ( "go/ast" "go/constant" "go/token" - "io" "io/ioutil" "math/rand" "net" @@ -104,13 +103,13 @@ func withTestProcessArgs(name string, t testing.TB, wd string, args []string, bu switch testBackend { case "native": - grp, err = native.Launch(append([]string{fixture.Path}, args...), wd, 0, []string{}, "", proc.NewEmptyRedirect()) + grp, err = native.Launch(append([]string{fixture.Path}, args...), wd, 0, []string{}, "", "", proc.OutputRedirect{}, proc.OutputRedirect{}) case "lldb": grp, err = gdbserial.LLDBLaunch(append([]string{fixture.Path}, args...), wd, 0, []string{}, "", [3]string{}) case "rr": protest.MustHaveRecordingAllowed(t) t.Log("recording") - grp, tracedir, err = gdbserial.RecordAndReplay(append([]string{fixture.Path}, args...), wd, true, []string{}, proc.NewEmptyRedirect()) + grp, tracedir, err = gdbserial.RecordAndReplay(append([]string{fixture.Path}, args...), wd, true, []string{}, "", proc.OutputRedirect{}, proc.OutputRedirect{}) t.Logf("replaying %q", tracedir) default: t.Fatal("unknown backend") @@ -2218,7 +2217,7 @@ func TestUnsupportedArch(t *testing.T) { switch testBackend { case "native": - p, err = native.Launch([]string{outfile}, ".", 0, []string{}, "", proc.NewEmptyRedirect()) + p, err = native.Launch([]string{outfile}, ".", 0, []string{}, "", "", proc.OutputRedirect{}, proc.OutputRedirect{}) case "lldb": p, err = gdbserial.LLDBLaunch([]string{outfile}, ".", 0, []string{}, "", [3]string{}) default: @@ -5927,120 +5926,6 @@ func TestStacktraceExtlinkMac(t *testing.T) { }) } -func testGenRedirect(t *testing.T, fixture protest.Fixture, expectStdout string, expectStderr string, errChan chan error) (redirect [3]proc.OutputRedirect, cancelFunc func(), err error) { - readers, redirect, err := proc.NamedPipe() - if err != nil { - return redirect, nil, err - } - - cancelFunc = func() { - for _, reader := range readers { - _ = reader.Close() - } - } - - go func() { - reader := func(mode string, f io.ReadCloser, expectOut string) { - out, err := io.ReadAll(f) - if err != nil { - errChan <- err - return - } - - if expectOut != string(out) { - errChan <- fmt.Errorf("%s,Not as expected!\nexpect:%s\nout:%s", mode, expectOut, out) - return - } - errChan <- nil - } - - go reader("stdout", readers[0], expectStdout) - go reader("stderr", readers[1], expectStderr) - }() - - return redirect, cancelFunc, nil -} - -func TestRedirect(t *testing.T) { - fixture := protest.BuildFixture("out_redirect", 0) - var ( - grp *proc.TargetGroup - tracedir string - err error - redirect [3]proc.OutputRedirect = proc.NewEmptyRedirect() - errChan = make(chan error, 2) - cancelFunc func() - needCheck = false - expectStdout = "hello world!\nhello world!" - expectStderr = "hello world!\nhello world! error!" - ) - switch testBackend { - case "native": - if runtime.GOOS == "linux" { - redirect, cancelFunc, err = testGenRedirect(t, fixture, expectStdout, expectStderr, errChan) - if err != nil { - break - } - needCheck = true - } else if runtime.GOOS == "windows" { - redirect, cancelFunc, err = testGenRedirect(t, fixture, expectStdout, expectStderr, errChan) - if err != nil { - break - } - needCheck = true - } - - grp, err = native.Launch([]string{fixture.Path}, ".", 0, []string{}, "", redirect) - case "lldb": - if runtime.GOOS == "darwin" { - redirect, cancelFunc, err = testGenRedirect(t, fixture, expectStdout, expectStderr, errChan) - if err != nil { - break - } - needCheck = true - } - grp, err = gdbserial.LLDBLaunch([]string{fixture.Path}, ".", 0, []string{}, "", [3]string{redirect[0].Path, redirect[1].Path, redirect[2].Path}) - case "rr": - protest.MustHaveRecordingAllowed(t) - t.Log("recording") - if runtime.GOOS != "windows" { - redirect, cancelFunc, err = testGenRedirect(t, fixture, expectStdout, expectStderr, errChan) - if err != nil { - break - } - needCheck = true - } - grp, tracedir, err = gdbserial.RecordAndReplay([]string{fixture.Path}, ".", true, []string{}, redirect) - t.Logf("replaying %q", tracedir) - default: - t.Fatal("unknown backend") - } - if err != nil { - // clear reader goroutine - if cancelFunc != nil { - cancelFunc() - _, _ = <-errChan, <-errChan - } - t.Fatal("Launch():", err) - } - _ = grp.Continue() - - if needCheck { - err1 := <-errChan - err2 := <-errChan - if err1 != nil { - t.Fatal("checkOut():", err1) - } - if err2 != nil { - t.Fatal("checkOut():", err2) - } - } - - defer func() { - grp.Detach(true) - }() -} - func TestFollowExec(t *testing.T) { skipUnlessOn(t, "follow exec only supported on linux", "linux") withTestProcessArgs("spawn", t, ".", []string{"spawn", "3"}, 0, func(p *proc.Target, grp *proc.TargetGroup, fixture protest.Fixture) { diff --git a/pkg/proc/redirect.go b/pkg/proc/redirect.go index d28f58a9e8..a9689cd76a 100644 --- a/pkg/proc/redirect.go +++ b/pkg/proc/redirect.go @@ -7,8 +7,8 @@ type OutputRedirect struct { File *os.File } -func NewEmptyRedirect() [3]OutputRedirect { - return [3]OutputRedirect{} +func NewEmptyRedirect() [2]OutputRedirect { + return [2]OutputRedirect{} } func NewRedirectByPath(paths [3]string) [3]OutputRedirect { diff --git a/service/dap/server.go b/service/dap/server.go index 3e7ce7b7b8..4e418581ac 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -1105,14 +1105,23 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { var clear func() if redirected { - var readers [2]io.ReadCloser - readers, s.config.Debugger.Redirect, err = proc.NamedPipe() - if err != nil { - s.sendShowUserErrorResponse(request.Request, InternalError, "Internal Error", - fmt.Sprintf("failed to generate stdio pipes - %v", err)) - return + var ( + readers [2]io.ReadCloser + outputRedirects [2]proc.OutputRedirect + ) + + for i := 0; i < 2; i++ { + readers[i], outputRedirects[i], err = proc.NamedPipe() + if err != nil { + s.sendShowUserErrorResponse(request.Request, InternalError, "Internal Error", + fmt.Sprintf("failed to generate stdio pipes - %v", err)) + return + } } + s.config.Debugger.Stdout = outputRedirects[0] + s.config.Debugger.Stderr = outputRedirects[1] + redirectedFunc(readers[0], readers[1]) clear = func() { for index := range readers { diff --git a/service/dap/server_test.go b/service/dap/server_test.go index af7a2f9fee..7bee9286cf 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -7408,7 +7408,7 @@ func TestRedirect(t *testing.T) { stderr = bytes.NewBufferString("") ) - TerminnatedPoint: + terminnatedPoint: for { message := client.ExpectMessage(t) switch m := message.(type) { @@ -7422,7 +7422,7 @@ func TestRedirect(t *testing.T) { t.Errorf("\ngot %#v\nwant Category='stdout' or 'stderr'", m) } case *dap.TerminatedEvent: - break TerminnatedPoint + break terminnatedPoint default: t.Errorf("\n got %#v, want *dap.OutputEvent or *dap.TerminateResponse", m) } diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index 0d243432a8..4bb6e95083 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -136,8 +136,14 @@ type Config struct { // ExecuteKind contains the kind of the executed program. ExecuteKind ExecuteKind - // Redirects specifies redirect rules for stdin, stdout and stderr - Redirect [3]proc.OutputRedirect + // Stdin Redirect file path for stdin + Stdin string + + // Redirects specifies redirect rules for stdout + Stdout proc.OutputRedirect + + // Redirects specifies redirect rules for stderr + Stderr proc.OutputRedirect // DisableASLR disables ASLR DisableASLR bool @@ -259,17 +265,16 @@ func (d *Debugger) Launch(processArgs []string, wd string) (*proc.TargetGroup, e switch d.config.Backend { case "native": - return native.Launch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, d.config.Redirect) + return native.Launch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, d.config.Stdin, d.config.Stdout, d.config.Stderr) case "lldb": - redirect := d.config.Redirect - return betterGdbserialLaunchError(gdbserial.LLDBLaunch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, [3]string{redirect[0].Path, redirect[1].Path, redirect[2].Path})) + return betterGdbserialLaunchError(gdbserial.LLDBLaunch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, [3]string{d.config.Stdin, d.config.Stdout.Path, d.config.Stderr.Path})) case "rr": if d.target != nil { // restart should not call us if the backend is 'rr' panic("internal error: call to Launch with rr backend and target already exists") } - run, stop, err := gdbserial.RecordAsync(processArgs, wd, false, d.config.Redirect) + run, stop, err := gdbserial.RecordAsync(processArgs, wd, false, d.config.Stdin, d.config.Stdout, d.config.Stderr) if err != nil { return nil, err } @@ -304,10 +309,9 @@ func (d *Debugger) Launch(processArgs []string, wd string) (*proc.TargetGroup, e case "default": if runtime.GOOS == "darwin" { - redirect := d.config.Redirect - return betterGdbserialLaunchError(gdbserial.LLDBLaunch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, [3]string{redirect[0].Path, redirect[1].Path, redirect[2].Path})) + return betterGdbserialLaunchError(gdbserial.LLDBLaunch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, [3]string{d.config.Stdin, d.config.Stdout.Path, d.config.Stderr.Path})) } - return native.Launch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, d.config.Redirect) + return native.Launch(processArgs, wd, launchFlags, d.config.DebugInfoDirectories, d.config.TTY, d.config.Stdin, d.config.Stdout, d.config.Stderr) default: return nil, fmt.Errorf("unknown backend %q", d.config.Backend) } @@ -479,7 +483,9 @@ func (d *Debugger) Restart(rerecord bool, pos string, resetArgs bool, newArgs [] } if resetArgs { d.processArgs = append([]string{d.processArgs[0]}, newArgs...) - d.config.Redirect = proc.NewRedirectByPath(newRedirects) + d.config.Stdin = newRedirects[0] + d.config.Stdout = proc.OutputRedirect{Path: newRedirects[1]} + d.config.Stderr = proc.OutputRedirect{Path: newRedirects[2]} } var grp *proc.TargetGroup var err error @@ -503,7 +509,7 @@ func (d *Debugger) Restart(rerecord bool, pos string, resetArgs bool, newArgs [] } if recorded { - run, stop, err2 := gdbserial.RecordAsync(d.processArgs, d.config.WorkingDir, false, d.config.Redirect) + run, stop, err2 := gdbserial.RecordAsync(d.processArgs, d.config.WorkingDir, false, d.config.Stdin, d.config.Stdout, d.config.Stderr) if err2 != nil { return nil, err2 } diff --git a/service/test/integration2_test.go b/service/test/integration2_test.go index c4e4b94fcd..6049e7b873 100644 --- a/service/test/integration2_test.go +++ b/service/test/integration2_test.go @@ -85,7 +85,9 @@ func startServer(name string, buildFlags protest.BuildFlags, t *testing.T, redir Packages: []string{fixture.Source}, BuildFlags: "", // build flags can be an empty string here because the only test that uses it, does not set special flags. ExecuteKind: debugger.ExecutingGeneratedFile, - Redirect: proc.NewRedirectByPath(redirects), + Stdin: redirects[0], + Stdout: proc.OutputRedirect{Path: redirects[1]}, + Stderr: proc.OutputRedirect{Path: redirects[2]}, }, }) if err := server.Run(); err != nil { From 10e0a66d0c105e56a4c9039585bafd6c8991bb3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Wed, 5 Apr 2023 03:30:12 +0800 Subject: [PATCH 45/53] fix: freebsd compile failure --- pkg/proc/native/proc_freebsd.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/proc/native/proc_freebsd.go b/pkg/proc/native/proc_freebsd.go index c236871526..2787426980 100644 --- a/pkg/proc/native/proc_freebsd.go +++ b/pkg/proc/native/proc_freebsd.go @@ -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]proc.OutputRedirect) (*proc.TargetGroup, error) { +func Launch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []string, tty string, stdinPath string, stdoutOR proc.OutputRedirect, stderrOR proc.OutputRedirect) (*proc.TargetGroup, error) { var ( process *exec.Cmd err error @@ -63,7 +63,7 @@ func Launch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []str foreground := flags&proc.LaunchForeground != 0 - stdin, stdout, stderr, closefn, err := openRedirects(redirects, foreground) + stdin, stdout, stderr, closefn, err := openRedirects(stdinPath, stdoutOR, stderrOR, foreground) if err != nil { return nil, err } From 1e3701b5a45980708f795f7f7ee30ab0d345bde7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Wed, 5 Apr 2023 03:49:46 +0800 Subject: [PATCH 46/53] fix: proc_darwin compile failure --- pkg/proc/native/proc_darwin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/proc/native/proc_darwin.go b/pkg/proc/native/proc_darwin.go index 194ef10120..62865b289d 100644 --- a/pkg/proc/native/proc_darwin.go +++ b/pkg/proc/native/proc_darwin.go @@ -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]proc.OutputRedirect) (*proc.TargetGroup, error) { +func Launch(cmd []string, wd string, flags proc.LaunchFlags, _ []string, _ string, _ string, _ proc.OutputRedirect, _ proc.OutputRedirect) (*proc.TargetGroup, error) { argv0Go, err := filepath.Abs(cmd[0]) if err != nil { return nil, err From 4c8f86bb4c7f6133120d064a6e309b0623025fc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Wed, 5 Apr 2023 04:00:41 +0800 Subject: [PATCH 47/53] style: remove useless code --- pkg/proc/redirect.go | 12 ------------ service/dap/server.go | 1 - 2 files changed, 13 deletions(-) diff --git a/pkg/proc/redirect.go b/pkg/proc/redirect.go index a9689cd76a..df5cd11488 100644 --- a/pkg/proc/redirect.go +++ b/pkg/proc/redirect.go @@ -6,15 +6,3 @@ type OutputRedirect struct { Path string File *os.File } - -func NewEmptyRedirect() [2]OutputRedirect { - return [2]OutputRedirect{} -} - -func NewRedirectByPath(paths [3]string) [3]OutputRedirect { - return [3]OutputRedirect{{Path: paths[0]}, {Path: paths[1]}, {Path: paths[2]}} -} - -func NewRedirectByFile(writerFiles [3]*os.File) [3]OutputRedirect { - return [3]OutputRedirect{{File: writerFiles[0]}, {File: writerFiles[1]}, {File: writerFiles[2]}} -} diff --git a/service/dap/server.go b/service/dap/server.go index 4e418581ac..8e750a04b3 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -1171,7 +1171,6 @@ func (s *Session) getPackageDir(pkg string) string { func (s *Session) newNoDebugProcess(program string, targetArgs []string, wd string, redirected bool) (cmd *exec.Cmd, err error) { if s.noDebugProcess != nil { return nil, fmt.Errorf("another launch request is in progress") - } cmd = exec.Command(program, targetArgs...) From a4f0491d4b2570b2e0f87b0472e86498e3c1f3ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Sat, 8 Apr 2023 15:07:31 +0800 Subject: [PATCH 48/53] feat: add d.config.Stdxx check on debug.Restart --- service/dap/server_test.go | 4 ++-- service/debugger/debugger.go | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 7bee9286cf..766cdc4ee2 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -7408,7 +7408,7 @@ func TestRedirect(t *testing.T) { stderr = bytes.NewBufferString("") ) - terminnatedPoint: + terminatedPoint: for { message := client.ExpectMessage(t) switch m := message.(type) { @@ -7422,7 +7422,7 @@ func TestRedirect(t *testing.T) { t.Errorf("\ngot %#v\nwant Category='stdout' or 'stderr'", m) } case *dap.TerminatedEvent: - break terminnatedPoint + break terminatedPoint default: t.Errorf("\n got %#v, want *dap.OutputEvent or *dap.TerminateResponse", m) } diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index 4bb6e95083..cff5c272d3 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -478,6 +478,11 @@ func (d *Debugger) Restart(rerecord bool, pos string, resetArgs bool, newArgs [] return nil, ErrCanNotRestart } + if !resetArgs && (d.config.Stdout.File != nil || d.config.Stderr.File != nil) { + return nil, ErrCanNotRestart + + } + if err := d.detach(true); err != nil { return nil, err } From 0c2f3adc770c61151a81cc5c8ec962d5914ec58c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Tue, 2 May 2023 02:51:55 +0800 Subject: [PATCH 49/53] style: formatting and adding comments --- pkg/proc/gdbserial/rr.go | 4 +--- pkg/proc/native/proc.go | 4 +--- pkg/proc/redirect.go | 4 ++++ 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/proc/gdbserial/rr.go b/pkg/proc/gdbserial/rr.go index 0492968a3e..13105e47a1 100644 --- a/pkg/proc/gdbserial/rr.go +++ b/pkg/proc/gdbserial/rr.go @@ -64,9 +64,7 @@ func RecordAsync(cmd []string, wd string, quiet bool, stdin string, stdout proc. } func openRedirects(stdinPath string, stdoutOR proc.OutputRedirect, stderrOR proc.OutputRedirect, quiet bool) (stdin, stdout, stderr *os.File, closefn func(), err error) { - var ( - toclose = []*os.File{} - ) + toclose := []*os.File{} if stdinPath != "" { stdin, err = os.Open(stdinPath) diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go index 5ba0c26e5c..618fb890d3 100644 --- a/pkg/proc/native/proc.go +++ b/pkg/proc/native/proc.go @@ -371,9 +371,7 @@ func (dbp *nativeProcess) writeSoftwareBreakpoint(thread *nativeThread, addr uin } func openRedirects(stdinPath string, stdoutOR proc.OutputRedirect, stderrOR proc.OutputRedirect, foreground bool) (stdin, stdout, stderr *os.File, closefn func(), err error) { - var ( - toclose = []*os.File{} - ) + toclose := []*os.File{} if stdinPath != "" { stdin, err = os.Open(stdinPath) diff --git a/pkg/proc/redirect.go b/pkg/proc/redirect.go index df5cd11488..278d8853ff 100644 --- a/pkg/proc/redirect.go +++ b/pkg/proc/redirect.go @@ -2,7 +2,11 @@ package proc import "os" +// OutputRedirect Specifies where the target program output will be redirected to. +// Only one of "Path" and "File" should be set. type OutputRedirect struct { + // Path File path. Path string + // File Redirect file. File *os.File } From 02d96ecb4835ed5a62b358f1298ab4a1fb762d35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Tue, 2 May 2023 02:51:55 +0800 Subject: [PATCH 50/53] style: formatting and adding comments --- pkg/proc/gdbserial/rr.go | 4 +--- pkg/proc/native/proc.go | 4 +--- pkg/proc/redirect.go | 4 ++++ 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/proc/gdbserial/rr.go b/pkg/proc/gdbserial/rr.go index 0492968a3e..13105e47a1 100644 --- a/pkg/proc/gdbserial/rr.go +++ b/pkg/proc/gdbserial/rr.go @@ -64,9 +64,7 @@ func RecordAsync(cmd []string, wd string, quiet bool, stdin string, stdout proc. } func openRedirects(stdinPath string, stdoutOR proc.OutputRedirect, stderrOR proc.OutputRedirect, quiet bool) (stdin, stdout, stderr *os.File, closefn func(), err error) { - var ( - toclose = []*os.File{} - ) + toclose := []*os.File{} if stdinPath != "" { stdin, err = os.Open(stdinPath) diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go index 5ba0c26e5c..618fb890d3 100644 --- a/pkg/proc/native/proc.go +++ b/pkg/proc/native/proc.go @@ -371,9 +371,7 @@ func (dbp *nativeProcess) writeSoftwareBreakpoint(thread *nativeThread, addr uin } func openRedirects(stdinPath string, stdoutOR proc.OutputRedirect, stderrOR proc.OutputRedirect, foreground bool) (stdin, stdout, stderr *os.File, closefn func(), err error) { - var ( - toclose = []*os.File{} - ) + toclose := []*os.File{} if stdinPath != "" { stdin, err = os.Open(stdinPath) diff --git a/pkg/proc/redirect.go b/pkg/proc/redirect.go index df5cd11488..278d8853ff 100644 --- a/pkg/proc/redirect.go +++ b/pkg/proc/redirect.go @@ -2,7 +2,11 @@ package proc import "os" +// OutputRedirect Specifies where the target program output will be redirected to. +// Only one of "Path" and "File" should be set. type OutputRedirect struct { + // Path File path. Path string + // File Redirect file. File *os.File } From 44fa28361d9a0729fda306dc9fcd41a1f3ebe2cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Sat, 8 Apr 2023 15:07:31 +0800 Subject: [PATCH 51/53] feat: add d.config.Stdxx check on debug.Restart --- service/dap/server_test.go | 4 ++-- service/debugger/debugger.go | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 7bee9286cf..766cdc4ee2 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -7408,7 +7408,7 @@ func TestRedirect(t *testing.T) { stderr = bytes.NewBufferString("") ) - terminnatedPoint: + terminatedPoint: for { message := client.ExpectMessage(t) switch m := message.(type) { @@ -7422,7 +7422,7 @@ func TestRedirect(t *testing.T) { t.Errorf("\ngot %#v\nwant Category='stdout' or 'stderr'", m) } case *dap.TerminatedEvent: - break terminnatedPoint + break terminatedPoint default: t.Errorf("\n got %#v, want *dap.OutputEvent or *dap.TerminateResponse", m) } diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index 4bb6e95083..cff5c272d3 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -478,6 +478,11 @@ func (d *Debugger) Restart(rerecord bool, pos string, resetArgs bool, newArgs [] return nil, ErrCanNotRestart } + if !resetArgs && (d.config.Stdout.File != nil || d.config.Stderr.File != nil) { + return nil, ErrCanNotRestart + + } + if err := d.detach(true); err != nil { return nil, err } From 768fad81ea67720a2765f6de2d4586447b35c0e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Fri, 12 May 2023 23:33:24 +0800 Subject: [PATCH 52/53] style: namedpipe->redirector --- pkg/proc/{namedpipe_other.go => redirector_other.go} | 2 +- pkg/proc/{namedpipe_windows.go => redirector_windows.go} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename pkg/proc/{namedpipe_other.go => redirector_other.go} (92%) rename pkg/proc/{namedpipe_windows.go => redirector_windows.go} (65%) diff --git a/pkg/proc/namedpipe_other.go b/pkg/proc/redirector_other.go similarity index 92% rename from pkg/proc/namedpipe_other.go rename to pkg/proc/redirector_other.go index ba76c95087..2203eb71f8 100644 --- a/pkg/proc/namedpipe_other.go +++ b/pkg/proc/redirector_other.go @@ -42,7 +42,7 @@ func (oor *openOnRead) Close() error { return oor.rd.Close() } -func NamedPipe() (reader io.ReadCloser, output OutputRedirect, err error) { +func Redirector() (reader io.ReadCloser, output OutputRedirect, err error) { r := make([]byte, 4) if _, err = rand.Read(r); err != nil { return reader, output, err diff --git a/pkg/proc/namedpipe_windows.go b/pkg/proc/redirector_windows.go similarity index 65% rename from pkg/proc/namedpipe_windows.go rename to pkg/proc/redirector_windows.go index 91dc6b2766..edd74a7ac3 100644 --- a/pkg/proc/namedpipe_windows.go +++ b/pkg/proc/redirector_windows.go @@ -8,7 +8,7 @@ import ( "os" ) -func NamedPipe() (reader io.ReadCloser, output OutputRedirect, err error) { +func Redirector() (reader io.ReadCloser, output OutputRedirect, err error) { reader, output.File, err = os.Pipe() return reader, output, err From 7dd6f49a57995941a11858dbb70939296d52dc56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E7=BF=94?= Date: Fri, 12 May 2023 23:45:03 +0800 Subject: [PATCH 53/53] style: namedPipe->redirector --- service/dap/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/dap/server.go b/service/dap/server.go index 8e750a04b3..80619a95cd 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -1111,7 +1111,7 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { ) for i := 0; i < 2; i++ { - readers[i], outputRedirects[i], err = proc.NamedPipe() + readers[i], outputRedirects[i], err = proc.Redirector() if err != nil { s.sendShowUserErrorResponse(request.Request, InternalError, "Internal Error", fmt.Sprintf("failed to generate stdio pipes - %v", err))