Skip to content

Conversation

@JDevlieghere
Copy link
Member

Return an llvm::Error from LaunchRunInTerminalTarget instead of calling exit() directly.

Return an `llvm::Error` from `LaunchRunInTerminalTarget` instead of
calling `exit()` directly.
@llvmbot
Copy link
Member

llvmbot commented Feb 26, 2025

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

Return an llvm::Error from LaunchRunInTerminalTarget instead of calling exit() directly.


Full diff: https://github.com/llvm/llvm-project/pull/128951.diff

1 Files Affected:

  • (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+19-16)
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 1c6bd7e903409..6b31f9d3cfe54 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -197,12 +197,13 @@ static void printHelp(LLDBDAPOptTable &table, llvm::StringRef tool_name) {
 //
 // In case of errors launching the target, a suitable error message will be
 // emitted to the debug adaptor.
-static void LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg,
-                                      llvm::StringRef comm_file,
-                                      lldb::pid_t debugger_pid, char *argv[]) {
+static llvm::Error LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg,
+                                             llvm::StringRef comm_file,
+                                             lldb::pid_t debugger_pid,
+                                             char *argv[]) {
 #if defined(_WIN32)
-  llvm::errs() << "runInTerminal is only supported on POSIX systems\n";
-  exit(EXIT_FAILURE);
+  return llvm::createStringError(
+      "runInTerminal is only supported on POSIX systems");
 #else
 
   // On Linux with the Yama security module enabled, a process can only attach
@@ -214,10 +215,8 @@ static void LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg,
 #endif
 
   RunInTerminalLauncherCommChannel comm_channel(comm_file);
-  if (llvm::Error err = comm_channel.NotifyPid()) {
-    llvm::errs() << llvm::toString(std::move(err)) << "\n";
-    exit(EXIT_FAILURE);
-  }
+  if (llvm::Error err = comm_channel.NotifyPid())
+    return err;
 
   // We will wait to be attached with a timeout. We don't wait indefinitely
   // using a signal to prevent being paused forever.
@@ -228,8 +227,7 @@ static void LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg,
       timeout_env_var != nullptr ? atoi(timeout_env_var) : 20000;
   if (llvm::Error err = comm_channel.WaitUntilDebugAdaptorAttaches(
           std::chrono::milliseconds(timeout_in_ms))) {
-    llvm::errs() << llvm::toString(std::move(err)) << "\n";
-    exit(EXIT_FAILURE);
+    return err;
   }
 
   const char *target = target_arg.getValue();
@@ -237,8 +235,8 @@ static void LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg,
 
   std::string error = std::strerror(errno);
   comm_channel.NotifyError(error);
-  llvm::errs() << error << "\n";
-  exit(EXIT_FAILURE);
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                 std::move(error));
 #endif
 }
 
@@ -469,13 +467,18 @@ int main(int argc, char *argv[]) {
         }
       }
       int target_args_pos = argc;
-      for (int i = 0; i < argc; i++)
+      for (int i = 0; i < argc; i++) {
         if (strcmp(argv[i], "--launch-target") == 0) {
           target_args_pos = i + 1;
           break;
         }
-      LaunchRunInTerminalTarget(*target_arg, comm_file->getValue(), pid,
-                                argv + target_args_pos);
+      }
+      if (llvm::Error err =
+              LaunchRunInTerminalTarget(*target_arg, comm_file->getValue(), pid,
+                                        argv + target_args_pos)) {
+        llvm::errs() << llvm::toString(std::move(err)) << '\n';
+        return EXIT_FAILURE;
+      }
     } else {
       llvm::errs() << "\"--launch-target\" requires \"--comm-file\" to be "
                       "specified\n";

Copy link
Collaborator

@slackito slackito left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@JDevlieghere JDevlieghere merged commit f409340 into llvm:main Feb 27, 2025
13 checks passed
@JDevlieghere JDevlieghere deleted the lldb-dap-exit branch February 27, 2025 00:34
joaosaffran pushed a commit to joaosaffran/llvm-project that referenced this pull request Mar 3, 2025
…C) (llvm#128951)

Return an `llvm::Error` from `LaunchRunInTerminalTarget` instead of
calling `exit()` directly.
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Jun 13, 2025
…C) (llvm#128951)

Return an `llvm::Error` from `LaunchRunInTerminalTarget` instead of
calling `exit()` directly.
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Jul 14, 2025
…C) (llvm#128951)

Return an `llvm::Error` from `LaunchRunInTerminalTarget` instead of
calling `exit()` directly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants