From d27cd49b0441f8fd1283ee340106853ae19c2ae1 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 19 Jul 2024 16:34:29 +0200 Subject: [PATCH 1/6] Inlined gRPC methods to implement GetDebugConfig and IsDebugSupported --- commands/service_debug.go | 11 ----------- commands/service_debug_config.go | 4 ++-- internal/cli/debug/debug.go | 2 +- internal/cli/debug/debug_check.go | 3 +-- 4 files changed, 4 insertions(+), 16 deletions(-) diff --git a/commands/service_debug.go b/commands/service_debug.go index d17c99622a6..a7950ab04ec 100644 --- a/commands/service_debug.go +++ b/commands/service_debug.go @@ -16,7 +16,6 @@ package commands import ( - "context" "errors" "os" @@ -64,13 +63,3 @@ func (s *arduinoCoreServerImpl) Debug(stream rpc.ArduinoCoreService_DebugServer) } return stream.Send(resp) } - -// GetDebugConfig return metadata about a debug session -func (s *arduinoCoreServerImpl) GetDebugConfig(ctx context.Context, req *rpc.GetDebugConfigRequest) (*rpc.GetDebugConfigResponse, error) { - return GetDebugConfig(ctx, req) -} - -// IsDebugSupported checks if debugging is supported for a given configuration -func (s *arduinoCoreServerImpl) IsDebugSupported(ctx context.Context, req *rpc.IsDebugSupportedRequest) (*rpc.IsDebugSupportedResponse, error) { - return IsDebugSupported(ctx, req) -} diff --git a/commands/service_debug_config.go b/commands/service_debug_config.go index 393a81811e6..018ae4033e8 100644 --- a/commands/service_debug_config.go +++ b/commands/service_debug_config.go @@ -38,7 +38,7 @@ import ( ) // GetDebugConfig returns metadata to start debugging with the specified board -func GetDebugConfig(ctx context.Context, req *rpc.GetDebugConfigRequest) (*rpc.GetDebugConfigResponse, error) { +func (s *arduinoCoreServerImpl) GetDebugConfig(ctx context.Context, req *rpc.GetDebugConfigRequest) (*rpc.GetDebugConfigResponse, error) { pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance()) if err != nil { return nil, err @@ -48,7 +48,7 @@ func GetDebugConfig(ctx context.Context, req *rpc.GetDebugConfigRequest) (*rpc.G } // IsDebugSupported checks if the given board/programmer configuration supports debugging. -func IsDebugSupported(ctx context.Context, req *rpc.IsDebugSupportedRequest) (*rpc.IsDebugSupportedResponse, error) { +func (s *arduinoCoreServerImpl) IsDebugSupported(ctx context.Context, req *rpc.IsDebugSupportedRequest) (*rpc.IsDebugSupportedResponse, error) { pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance()) if err != nil { return nil, err diff --git a/internal/cli/debug/debug.go b/internal/cli/debug/debug.go index 03c8dc8cfcb..c83a650842d 100644 --- a/internal/cli/debug/debug.go +++ b/internal/cli/debug/debug.go @@ -122,7 +122,7 @@ func runDebugCommand(ctx context.Context, srv rpc.ArduinoCoreServiceServer, args if printInfo { - if res, err := commands.GetDebugConfig(ctx, debugConfigRequested); err != nil { + if res, err := srv.GetDebugConfig(ctx, debugConfigRequested); err != nil { errcode := feedback.ErrBadArgument if errors.Is(err, &cmderrors.MissingProgrammerError{}) { errcode = feedback.ErrMissingProgrammer diff --git a/internal/cli/debug/debug_check.go b/internal/cli/debug/debug_check.go index cb749a2da0e..ea64390b311 100644 --- a/internal/cli/debug/debug_check.go +++ b/internal/cli/debug/debug_check.go @@ -19,7 +19,6 @@ import ( "context" "os" - "github.com/arduino/arduino-cli/commands" "github.com/arduino/arduino-cli/internal/cli/arguments" "github.com/arduino/arduino-cli/internal/cli/feedback" "github.com/arduino/arduino-cli/internal/cli/feedback/result" @@ -61,7 +60,7 @@ func runDebugCheckCommand(ctx context.Context, srv rpc.ArduinoCoreServiceServer, feedback.FatalError(err, feedback.ErrBadArgument) } fqbn := fqbnArg.String() - resp, err := commands.IsDebugSupported(ctx, &rpc.IsDebugSupportedRequest{ + resp, err := srv.IsDebugSupported(ctx, &rpc.IsDebugSupportedRequest{ Instance: instance, Fqbn: fqbn, Port: port, From ab34688fe1166ef5833faf76d30f403e3a02df47 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 22 Jul 2024 14:33:02 +0200 Subject: [PATCH 2/6] Inlined function --- commands/service_debug.go | 89 ++++++++++++++++++++++++++------- commands/service_debug_run.go | 94 ----------------------------------- 2 files changed, 71 insertions(+), 112 deletions(-) diff --git a/commands/service_debug.go b/commands/service_debug.go index a7950ab04ec..a397f49aef6 100644 --- a/commands/service_debug.go +++ b/commands/service_debug.go @@ -21,12 +21,30 @@ import ( "github.com/arduino/arduino-cli/internal/i18n" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" + + "fmt" + "io" + "time" + + "github.com/arduino/arduino-cli/commands/cmderrors" + "github.com/arduino/arduino-cli/commands/internal/instances" + paths "github.com/arduino/go-paths-helper" + "github.com/sirupsen/logrus" ) // Debug returns a stream response that can be used to fetch data from the // target. The first message passed through the `Debug` request must // contain DebugRequest configuration params, not data. func (s *arduinoCoreServerImpl) Debug(stream rpc.ArduinoCoreService_DebugServer) error { + // Utility functions + syncSend := NewSynchronizedSend(stream.Send) + sendResult := func(res *rpc.DebugResponse_Result) error { + return syncSend.Send(&rpc.DebugResponse{Message: &rpc.DebugResponse_Result_{Result: res}}) + } + sendData := func(data []byte) { + _ = syncSend.Send(&rpc.DebugResponse{Message: &rpc.DebugResponse_Data{Data: data}}) + } + // Grab the first message msg, err := stream.Recv() if err != nil { @@ -42,24 +60,59 @@ func (s *arduinoCoreServerImpl) Debug(stream rpc.ArduinoCoreService_DebugServer) // Launch debug recipe attaching stdin and out to grpc streaming signalChan := make(chan os.Signal) defer close(signalChan) - outStream := feedStreamTo(func(data []byte) { - stream.Send(&rpc.DebugResponse{Message: &rpc.DebugResponse_Data{ - Data: data, - }}) + outStream := feedStreamTo(sendData) + defer outStream.Close() + inStream := consumeStreamFrom(func() ([]byte, error) { + command, err := stream.Recv() + if command.GetSendInterrupt() { + signalChan <- os.Interrupt + } + return command.GetData(), err }) - resp, debugErr := Debug(stream.Context(), req, - consumeStreamFrom(func() ([]byte, error) { - command, err := stream.Recv() - if command.GetSendInterrupt() { - signalChan <- os.Interrupt - } - return command.GetData(), err - }), - outStream, - signalChan) - outStream.Close() - if debugErr != nil { - return debugErr + + pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance()) + if err != nil { + return err + } + defer release() + + // Exec debugger + commandLine, err := getCommandLine(req, pme) + if err != nil { + return err + } + entry := logrus.NewEntry(logrus.StandardLogger()) + for i, param := range commandLine { + entry = entry.WithField(fmt.Sprintf("param%d", i), param) + } + entry.Debug("Executing debugger") + cmd, err := paths.NewProcess(pme.GetEnvVarsForSpawnedProcess(), commandLine...) + if err != nil { + return &cmderrors.FailedDebugError{Message: i18n.Tr("Cannot execute debug tool"), Cause: err} + } + in, err := cmd.StdinPipe() + if err != nil { + return sendResult(&rpc.DebugResponse_Result{Error: err.Error()}) + } + defer in.Close() + cmd.RedirectStdoutTo(io.Writer(outStream)) + cmd.RedirectStderrTo(io.Writer(outStream)) + if err := cmd.Start(); err != nil { + return sendResult(&rpc.DebugResponse_Result{Error: err.Error()}) + } + + go func() { + for sig := range signalChan { + cmd.Signal(sig) + } + }() + go func() { + io.Copy(in, inStream) + time.Sleep(time.Second) + cmd.Kill() + }() + if err := cmd.Wait(); err != nil { + return sendResult(&rpc.DebugResponse_Result{Error: err.Error()}) } - return stream.Send(resp) + return sendResult(&rpc.DebugResponse_Result{}) } diff --git a/commands/service_debug_run.go b/commands/service_debug_run.go index 94cdc43efa8..65251b9ae60 100644 --- a/commands/service_debug_run.go +++ b/commands/service_debug_run.go @@ -16,111 +16,17 @@ package commands import ( - "context" "fmt" - "io" - "os" "path/filepath" "runtime" - "time" "github.com/arduino/arduino-cli/commands/cmderrors" - "github.com/arduino/arduino-cli/commands/internal/instances" "github.com/arduino/arduino-cli/internal/arduino/cores/packagemanager" "github.com/arduino/arduino-cli/internal/i18n" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" "github.com/arduino/go-paths-helper" - "github.com/sirupsen/logrus" ) -// Debug command launches a debug tool for a sketch. -// It also implements streams routing: -// gRPC In -> tool stdIn -// grpc Out <- tool stdOut -// grpc Out <- tool stdErr -// It also implements tool process lifecycle management -func Debug(ctx context.Context, req *rpc.GetDebugConfigRequest, inStream io.Reader, out io.Writer, interrupt <-chan os.Signal) (*rpc.DebugResponse, error) { - - // Get debugging command line to run debugger - pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance()) - if err != nil { - return nil, err - } - defer release() - - commandLine, err := getCommandLine(req, pme) - if err != nil { - return nil, err - } - - for i, arg := range commandLine { - fmt.Printf("%2d: %s\n", i, arg) - } - - // Run Tool - entry := logrus.NewEntry(logrus.StandardLogger()) - for i, param := range commandLine { - entry = entry.WithField(fmt.Sprintf("param%d", i), param) - } - entry.Debug("Executing debugger") - - cmd, err := paths.NewProcess(pme.GetEnvVarsForSpawnedProcess(), commandLine...) - if err != nil { - return nil, &cmderrors.FailedDebugError{Message: i18n.Tr("Cannot execute debug tool"), Cause: err} - } - - // Get stdIn pipe from tool - in, err := cmd.StdinPipe() - if err != nil { - return &rpc.DebugResponse{Message: &rpc.DebugResponse_Result_{ - Result: &rpc.DebugResponse_Result{Error: err.Error()}, - }}, nil - } - defer in.Close() - - // Merge tool StdOut and StdErr to stream them in the io.Writer passed stream - cmd.RedirectStdoutTo(out) - cmd.RedirectStderrTo(out) - - // Start the debug command - if err := cmd.Start(); err != nil { - return &rpc.DebugResponse{Message: &rpc.DebugResponse_Result_{ - Result: &rpc.DebugResponse_Result{Error: err.Error()}, - }}, nil - } - - if interrupt != nil { - go func() { - for { - sig, ok := <-interrupt - if !ok { - break - } - cmd.Signal(sig) - } - }() - } - - go func() { - // Copy data from passed inStream into command stdIn - io.Copy(in, inStream) - // In any case, try process termination after a second to avoid leaving - // zombie process. - time.Sleep(time.Second) - cmd.Kill() - }() - - // Wait for process to finish - if err := cmd.Wait(); err != nil { - return &rpc.DebugResponse{Message: &rpc.DebugResponse_Result_{ - Result: &rpc.DebugResponse_Result{Error: err.Error()}, - }}, nil - } - return &rpc.DebugResponse{Message: &rpc.DebugResponse_Result_{ - Result: &rpc.DebugResponse_Result{}, - }}, nil -} - // getCommandLine compose a debug command represented by a core recipe func getCommandLine(req *rpc.GetDebugConfigRequest, pme *packagemanager.Explorer) ([]string, error) { debugInfo, err := getDebugProperties(req, pme, false) From 69836730ec3770b3a65e24915ca7dbe27d330cef Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 22 Jul 2024 14:33:49 +0200 Subject: [PATCH 3/6] Renamed vars for clarity --- commands/service_debug.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/commands/service_debug.go b/commands/service_debug.go index a397f49aef6..7ee076a08e3 100644 --- a/commands/service_debug.go +++ b/commands/service_debug.go @@ -46,14 +46,14 @@ func (s *arduinoCoreServerImpl) Debug(stream rpc.ArduinoCoreService_DebugServer) } // Grab the first message - msg, err := stream.Recv() + debugConfReqMsg, err := stream.Recv() if err != nil { return err } // Ensure it's a config message and not data - req := msg.GetDebugRequest() - if req == nil { + debugConfReq := debugConfReqMsg.GetDebugRequest() + if debugConfReq == nil { return errors.New(i18n.Tr("First message must contain debug request, not data")) } @@ -70,14 +70,14 @@ func (s *arduinoCoreServerImpl) Debug(stream rpc.ArduinoCoreService_DebugServer) return command.GetData(), err }) - pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance()) + pme, release, err := instances.GetPackageManagerExplorer(debugConfReq.GetInstance()) if err != nil { return err } defer release() // Exec debugger - commandLine, err := getCommandLine(req, pme) + commandLine, err := getCommandLine(debugConfReq, pme) if err != nil { return err } From a88fe8c58dcf2c9310b5340872bbbaaccb8f7465 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 22 Jul 2024 14:58:36 +0200 Subject: [PATCH 4/6] Added Debug gRPC adapter function --- commands/service_debug.go | 68 +++++++++++++++++++++++++++++++++++-- internal/cli/debug/debug.go | 5 ++- 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/commands/service_debug.go b/commands/service_debug.go index 7ee076a08e3..d78ce384479 100644 --- a/commands/service_debug.go +++ b/commands/service_debug.go @@ -16,11 +16,14 @@ package commands import ( + "context" "errors" "os" + "sync/atomic" "github.com/arduino/arduino-cli/internal/i18n" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" + "google.golang.org/grpc/metadata" "fmt" "io" @@ -32,9 +35,68 @@ import ( "github.com/sirupsen/logrus" ) -// Debug returns a stream response that can be used to fetch data from the -// target. The first message passed through the `Debug` request must -// contain DebugRequest configuration params, not data. +type debugServer struct { + ctx context.Context + req atomic.Pointer[rpc.GetDebugConfigRequest] + in io.Reader + out io.Writer + resultCB func(*rpc.DebugResponse_Result) +} + +func (s *debugServer) Send(resp *rpc.DebugResponse) error { + if len(resp.GetData()) > 0 { + if _, err := s.out.Write(resp.GetData()); err != nil { + return err + } + } + if res := resp.GetResult(); res != nil { + s.resultCB(res) + } + return nil +} + +func (s *debugServer) Recv() (r *rpc.DebugRequest, e error) { + if conf := s.req.Swap(nil); conf != nil { + return &rpc.DebugRequest{DebugRequest: conf}, nil + } + buff := make([]byte, 4096) + n, err := s.in.Read(buff) + if err != nil { + return nil, err + } + return &rpc.DebugRequest{Data: buff[:n]}, nil +} + +func (s *debugServer) Context() context.Context { return s.ctx } +func (s *debugServer) RecvMsg(m any) error { return nil } +func (s *debugServer) SendHeader(metadata.MD) error { return nil } +func (s *debugServer) SendMsg(m any) error { return nil } +func (s *debugServer) SetHeader(metadata.MD) error { return nil } +func (s *debugServer) SetTrailer(metadata.MD) {} + +// DebugServerToStreams creates a debug server that proxies the data to the given io streams. +// The GetDebugConfigRequest is used to configure the debbuger. sig is a channel that can be +// used to send os.Interrupt to the debug process. resultCB is a callback function that will +// receive the Debug result. +func DebugServerToStreams( + ctx context.Context, + req *rpc.GetDebugConfigRequest, + in io.Reader, out io.Writer, + sig chan os.Signal, + resultCB func(*rpc.DebugResponse_Result), +) rpc.ArduinoCoreService_DebugServer { + server := &debugServer{ + ctx: ctx, + in: in, + out: out, + resultCB: resultCB, + } + server.req.Store(req) + return server +} + +// Debug starts a debugging session. The first message passed through the `Debug` request must +// contain DebugRequest configuration params and no data. func (s *arduinoCoreServerImpl) Debug(stream rpc.ArduinoCoreService_DebugServer) error { // Utility functions syncSend := NewSynchronizedSend(stream.Send) diff --git a/internal/cli/debug/debug.go b/internal/cli/debug/debug.go index c83a650842d..84ac83c8cad 100644 --- a/internal/cli/debug/debug.go +++ b/internal/cli/debug/debug.go @@ -142,7 +142,10 @@ func runDebugCommand(ctx context.Context, srv rpc.ArduinoCoreServiceServer, args if err != nil { feedback.FatalError(err, feedback.ErrBadArgument) } - if _, err := commands.Debug(ctx, debugConfigRequested, in, out, ctrlc); err != nil { + + resultCB := func(dr *rpc.DebugResponse_Result) {} + stream := commands.DebugServerToStreams(ctx, debugConfigRequested, in, out, ctrlc, resultCB) + if err := srv.Debug(stream); err != nil { errcode := feedback.ErrGeneric if errors.Is(err, &cmderrors.MissingProgrammerError{}) { errcode = feedback.ErrMissingProgrammer From d14711ed629d0fa1f38f16f4da7b98f3f3657c1f Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 22 Jul 2024 15:00:48 +0200 Subject: [PATCH 5/6] Moved function and removed useless file --- commands/service_debug.go | 82 ++++++++++++++++++++++++++ commands/service_debug_run.go | 107 ---------------------------------- 2 files changed, 82 insertions(+), 107 deletions(-) delete mode 100644 commands/service_debug_run.go diff --git a/commands/service_debug.go b/commands/service_debug.go index d78ce384479..191bf193695 100644 --- a/commands/service_debug.go +++ b/commands/service_debug.go @@ -19,8 +19,11 @@ import ( "context" "errors" "os" + "path/filepath" + "runtime" "sync/atomic" + "github.com/arduino/arduino-cli/internal/arduino/cores/packagemanager" "github.com/arduino/arduino-cli/internal/i18n" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" "google.golang.org/grpc/metadata" @@ -178,3 +181,82 @@ func (s *arduinoCoreServerImpl) Debug(stream rpc.ArduinoCoreService_DebugServer) } return sendResult(&rpc.DebugResponse_Result{}) } + +// getCommandLine compose a debug command represented by a core recipe +func getCommandLine(req *rpc.GetDebugConfigRequest, pme *packagemanager.Explorer) ([]string, error) { + debugInfo, err := getDebugProperties(req, pme, false) + if err != nil { + return nil, err + } + + cmdArgs := []string{} + add := func(s string) { cmdArgs = append(cmdArgs, s) } + + // Add path to GDB Client to command line + var gdbPath *paths.Path + switch debugInfo.GetToolchain() { + case "gcc": + gdbexecutable := debugInfo.GetToolchainPrefix() + "-gdb" + if runtime.GOOS == "windows" { + gdbexecutable += ".exe" + } + gdbPath = paths.New(debugInfo.GetToolchainPath()).Join(gdbexecutable) + default: + return nil, &cmderrors.FailedDebugError{Message: i18n.Tr("Toolchain '%s' is not supported", debugInfo.GetToolchain())} + } + add(gdbPath.String()) + + // Set GDB interpreter (default value should be "console") + gdbInterpreter := req.GetInterpreter() + if gdbInterpreter == "" { + gdbInterpreter = "console" + } + add("--interpreter=" + gdbInterpreter) + if gdbInterpreter != "console" { + add("-ex") + add("set pagination off") + } + + // Add extra GDB execution commands + add("-ex") + add("set remotetimeout 5") + + // Extract path to GDB Server + switch debugInfo.GetServer() { + case "openocd": + var openocdConf rpc.DebugOpenOCDServerConfiguration + if err := debugInfo.GetServerConfiguration().UnmarshalTo(&openocdConf); err != nil { + return nil, err + } + + serverCmd := fmt.Sprintf(`target extended-remote | "%s"`, debugInfo.GetServerPath()) + + if cfg := openocdConf.GetScriptsDir(); cfg != "" { + serverCmd += fmt.Sprintf(` -s "%s"`, cfg) + } + + for _, script := range openocdConf.GetScripts() { + serverCmd += fmt.Sprintf(` --file "%s"`, script) + } + + serverCmd += ` -c "gdb_port pipe"` + serverCmd += ` -c "telnet_port 0"` + + add("-ex") + add(serverCmd) + + default: + return nil, &cmderrors.FailedDebugError{Message: i18n.Tr("GDB server '%s' is not supported", debugInfo.GetServer())} + } + + // Add executable + add(debugInfo.GetExecutable()) + + // Transform every path to forward slashes (on Windows some tools further + // escapes the command line so the backslash "\" gets in the way). + for i, param := range cmdArgs { + cmdArgs[i] = filepath.ToSlash(param) + } + + return cmdArgs, nil +} diff --git a/commands/service_debug_run.go b/commands/service_debug_run.go deleted file mode 100644 index 65251b9ae60..00000000000 --- a/commands/service_debug_run.go +++ /dev/null @@ -1,107 +0,0 @@ -// This file is part of arduino-cli. -// -// Copyright 2020 ARDUINO SA (http://www.arduino.cc/) -// -// This software is released under the GNU General Public License version 3, -// which covers the main part of arduino-cli. -// The terms of this license can be found at: -// https://www.gnu.org/licenses/gpl-3.0.en.html -// -// You can be released from the requirements of the above licenses by purchasing -// a commercial license. Buying such a license is mandatory if you want to -// modify or otherwise use the software for commercial activities involving the -// Arduino software without disclosing the source code of your own applications. -// To purchase a commercial license, send an email to license@arduino.cc. - -package commands - -import ( - "fmt" - "path/filepath" - "runtime" - - "github.com/arduino/arduino-cli/commands/cmderrors" - "github.com/arduino/arduino-cli/internal/arduino/cores/packagemanager" - "github.com/arduino/arduino-cli/internal/i18n" - rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" - "github.com/arduino/go-paths-helper" -) - -// getCommandLine compose a debug command represented by a core recipe -func getCommandLine(req *rpc.GetDebugConfigRequest, pme *packagemanager.Explorer) ([]string, error) { - debugInfo, err := getDebugProperties(req, pme, false) - if err != nil { - return nil, err - } - - cmdArgs := []string{} - add := func(s string) { cmdArgs = append(cmdArgs, s) } - - // Add path to GDB Client to command line - var gdbPath *paths.Path - switch debugInfo.GetToolchain() { - case "gcc": - gdbexecutable := debugInfo.GetToolchainPrefix() + "-gdb" - if runtime.GOOS == "windows" { - gdbexecutable += ".exe" - } - gdbPath = paths.New(debugInfo.GetToolchainPath()).Join(gdbexecutable) - default: - return nil, &cmderrors.FailedDebugError{Message: i18n.Tr("Toolchain '%s' is not supported", debugInfo.GetToolchain())} - } - add(gdbPath.String()) - - // Set GDB interpreter (default value should be "console") - gdbInterpreter := req.GetInterpreter() - if gdbInterpreter == "" { - gdbInterpreter = "console" - } - add("--interpreter=" + gdbInterpreter) - if gdbInterpreter != "console" { - add("-ex") - add("set pagination off") - } - - // Add extra GDB execution commands - add("-ex") - add("set remotetimeout 5") - - // Extract path to GDB Server - switch debugInfo.GetServer() { - case "openocd": - var openocdConf rpc.DebugOpenOCDServerConfiguration - if err := debugInfo.GetServerConfiguration().UnmarshalTo(&openocdConf); err != nil { - return nil, err - } - - serverCmd := fmt.Sprintf(`target extended-remote | "%s"`, debugInfo.GetServerPath()) - - if cfg := openocdConf.GetScriptsDir(); cfg != "" { - serverCmd += fmt.Sprintf(` -s "%s"`, cfg) - } - - for _, script := range openocdConf.GetScripts() { - serverCmd += fmt.Sprintf(` --file "%s"`, script) - } - - serverCmd += ` -c "gdb_port pipe"` - serverCmd += ` -c "telnet_port 0"` - - add("-ex") - add(serverCmd) - - default: - return nil, &cmderrors.FailedDebugError{Message: i18n.Tr("GDB server '%s' is not supported", debugInfo.GetServer())} - } - - // Add executable - add(debugInfo.GetExecutable()) - - // Transform every path to forward slashes (on Windows some tools further - // escapes the command line so the backslash "\" gets in the way). - for i, param := range cmdArgs { - cmdArgs[i] = filepath.ToSlash(param) - } - - return cmdArgs, nil -} From 13f87dd860d1cc2f8e568239cbadeeab93d1036d Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 7 Aug 2024 17:43:18 +0200 Subject: [PATCH 6/6] Forward os.interrupt (aka CTRL-C) signal to the gdb process This a challenging problem because we must wait on both an io.Read(...) and a channel-read but, unfortunately, go native select can wait only on channels. To overcome this limitation I had to resort to a conditional variable and write some boilerplate code to make everything synchronized. --- commands/service_debug.go | 108 +++++++++++++++++++++++++++++++------- 1 file changed, 90 insertions(+), 18 deletions(-) diff --git a/commands/service_debug.go b/commands/service_debug.go index 191bf193695..31123cc9ae1 100644 --- a/commands/service_debug.go +++ b/commands/service_debug.go @@ -18,32 +18,38 @@ package commands import ( "context" "errors" + "fmt" + "io" "os" "path/filepath" "runtime" + "sync" "sync/atomic" - - "github.com/arduino/arduino-cli/internal/arduino/cores/packagemanager" - "github.com/arduino/arduino-cli/internal/i18n" - rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" - "google.golang.org/grpc/metadata" - - "fmt" - "io" "time" "github.com/arduino/arduino-cli/commands/cmderrors" "github.com/arduino/arduino-cli/commands/internal/instances" + "github.com/arduino/arduino-cli/internal/arduino/cores/packagemanager" + "github.com/arduino/arduino-cli/internal/i18n" + rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" paths "github.com/arduino/go-paths-helper" + "github.com/djherbis/buffer" + "github.com/djherbis/nio/v3" "github.com/sirupsen/logrus" + "google.golang.org/grpc/metadata" ) type debugServer struct { ctx context.Context req atomic.Pointer[rpc.GetDebugConfigRequest] in io.Reader + inSignal bool + inData bool + inEvent *sync.Cond + inLock sync.Mutex out io.Writer resultCB func(*rpc.DebugResponse_Result) + done chan bool } func (s *debugServer) Send(resp *rpc.DebugResponse) error { @@ -54,6 +60,7 @@ func (s *debugServer) Send(resp *rpc.DebugResponse) error { } if res := resp.GetResult(); res != nil { s.resultCB(res) + s.close() } return nil } @@ -62,12 +69,33 @@ func (s *debugServer) Recv() (r *rpc.DebugRequest, e error) { if conf := s.req.Swap(nil); conf != nil { return &rpc.DebugRequest{DebugRequest: conf}, nil } - buff := make([]byte, 4096) - n, err := s.in.Read(buff) - if err != nil { - return nil, err + + s.inEvent.L.Lock() + for !s.inSignal && !s.inData { + s.inEvent.Wait() + } + defer s.inEvent.L.Unlock() + + if s.inSignal { + s.inSignal = false + return &rpc.DebugRequest{SendInterrupt: true}, nil + } + + if s.inData { + s.inData = false + buff := make([]byte, 4096) + n, err := s.in.Read(buff) + if err != nil { + return nil, err + } + return &rpc.DebugRequest{Data: buff[:n]}, nil } - return &rpc.DebugRequest{Data: buff[:n]}, nil + + panic("invalid state in debug") +} + +func (s *debugServer) close() { + close(s.done) } func (s *debugServer) Context() context.Context { return s.ctx } @@ -80,7 +108,7 @@ func (s *debugServer) SetTrailer(metadata.MD) {} // DebugServerToStreams creates a debug server that proxies the data to the given io streams. // The GetDebugConfigRequest is used to configure the debbuger. sig is a channel that can be // used to send os.Interrupt to the debug process. resultCB is a callback function that will -// receive the Debug result. +// receive the Debug result and closes the debug server. func DebugServerToStreams( ctx context.Context, req *rpc.GetDebugConfigRequest, @@ -93,8 +121,45 @@ func DebugServerToStreams( in: in, out: out, resultCB: resultCB, + done: make(chan bool), } + serverIn, clientOut := nio.Pipe(buffer.New(32 * 1024)) + server.in = serverIn + server.inEvent = sync.NewCond(&server.inLock) server.req.Store(req) + go func() { + for { + select { + case <-sig: + server.inEvent.L.Lock() + server.inSignal = true + server.inEvent.Broadcast() + server.inEvent.L.Unlock() + case <-server.done: + return + } + } + }() + go func() { + defer clientOut.Close() + buff := make([]byte, 4096) + for { + n, readErr := in.Read(buff) + + server.inEvent.L.Lock() + var writeErr error + if readErr == nil { + _, writeErr = clientOut.Write(buff[:n]) + } + server.inData = true + server.inEvent.Broadcast() + server.inEvent.L.Unlock() + if readErr != nil || writeErr != nil { + // exit on error + return + } + } + }() return server } @@ -128,11 +193,18 @@ func (s *arduinoCoreServerImpl) Debug(stream rpc.ArduinoCoreService_DebugServer) outStream := feedStreamTo(sendData) defer outStream.Close() inStream := consumeStreamFrom(func() ([]byte, error) { - command, err := stream.Recv() - if command.GetSendInterrupt() { - signalChan <- os.Interrupt + for { + req, err := stream.Recv() + if err != nil { + return nil, err + } + if req.GetSendInterrupt() { + signalChan <- os.Interrupt + } + if data := req.GetData(); len(data) > 0 { + return data, nil + } } - return command.GetData(), err }) pme, release, err := instances.GetPackageManagerExplorer(debugConfReq.GetInstance())