[lldb server] Tidy up LLDB server return codes and associated tests
authorSebastian Schwartz <saschwartz@fb.com>
Thu, 2 Sep 2021 09:14:49 +0000 (11:14 +0200)
committerRaphael Isemann <teemperor@gmail.com>
Thu, 2 Sep 2021 09:16:37 +0000 (11:16 +0200)
This diff modifies the LLDB server return codes to more accurately reflect usage
error paths. Specifically we always propagate the return codes from the main
entrypoints into GDB remote LLDB server, and platform LLDB server. This way, the
top-level caller of LLDB server will be able to correctly check whether the
executable exited with or without an error.

We additionally modify and extend the associated shell unit tests to expect
nonzero return codes on error conditions.

Test Plan:
LLDB tests pass:

```
ninja check-lldb
```

Reviewed By: teemperor

Differential Revision: https://reviews.llvm.org/D108351

lldb/test/Shell/lldb-server/TestErrorMessages.test
lldb/test/Shell/lldb-server/TestGdbserverPort.test
lldb/tools/lldb-server/lldb-platform.cpp
lldb/tools/lldb-server/lldb-server.cpp

index b9689fb..799c659 100644 (file)
@@ -1,14 +1,26 @@
-RUN: %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
+RUN: not %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,GDB_REMOTE_ALL %s
 FD1: error: --fd: missing argument
 
-RUN: %lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,ALL %s
+RUN: not %lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,GDB_REMOTE_ALL %s
 FD2: error: invalid '--fd' argument
 
-RUN: %lldb-server gdbserver --bogus 2>&1 | FileCheck --check-prefixes=BOGUS,ALL %s
+RUN: not %lldb-server gdbserver --bogus 2>&1 | FileCheck --check-prefixes=BOGUS,GDB_REMOTE_ALL %s
 BOGUS: error: unknown argument '--bogus'
 
-RUN: %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
+RUN: not %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,GDB_REMOTE_ALL %s
 CONN: error: no connection arguments
 
-ALL: Use '{{.*}} g[dbserver] --help' for a complete list of options.
+RUN: %lldb-server platform 2>&1 | FileCheck --check-prefixes=LLDB_PLATFORM_ALL %s
+
+RUN: %lldb-server platform --fd 2>&1 | FileCheck --check-prefixes=FD3,LLDB_PLATFORM_ALL %s
+FD3: lldb-server: unrecognized option `--fd'
+
+RUN: not %lldb-server platform --min-gdbserver-port 42 --max-gdbserver-port 43 2>&1 | FileCheck --check-prefixes=PORT1,LLDB_PLATFORM_ALL %s
+PORT1: error: port number 42 is not in the valid user port range of 1024 - 49152 
+
+RUN: %lldb-server version 2>&1 | FileCheck --check-prefixes=VERSION %s
+VERSION: lldb version
+
+GDB_REMOTE_ALL: Use '{{.*}} g[dbserver] --help' for a complete list of options.
+LLDB_PLATFORM_ALL: lldb-server platform [--log-file log-file-name] [--log-channels log-channel-list] [--port-file port-file-path] --server --listen port 
 
index 04facfe..a3dd1af 100644 (file)
@@ -1,4 +1,4 @@
 # Windows does not build lldb-server.
 # UNSUPPORTED: system-windows
-# RUN: %platformserver --server --listen :1234 --min-gdbserver-port 1234 --max-gdbserver-port 1234 2>&1 | FileCheck %s
+# RUN: not %platformserver --server --listen :1234 --min-gdbserver-port 1234 --max-gdbserver-port 1234 2>&1 | FileCheck %s
 # CHECK: error: --min-gdbserver-port (1234) is not lower than --max-gdbserver-port (1234)
index d4b5436..3cf8613 100644 (file)
@@ -92,7 +92,6 @@ static void display_usage(const char *progname, const char *subcommand) {
                   "log-channel-list] [--port-file port-file-path] --server "
                   "--listen port\n",
           progname, subcommand);
-  exit(0);
 }
 
 static Status save_socket_id_to_file(const std::string &socket_id,
@@ -165,7 +164,6 @@ int main_platform(int argc, char *argv[]) {
   FileSpec socket_file;
   bool show_usage = false;
   int option_error = 0;
-  int socket_error = -1;
 
   std::string short_options(OptionParser::GetShortOptionString(g_long_options));
 
@@ -249,7 +247,7 @@ int main_platform(int argc, char *argv[]) {
   }
 
   if (!LLDBServerUtilities::SetupLogging(log_file, log_channels, 0))
-    return -1;
+    return 1;
 
   // Make a port map for a port range that was specified.
   if (min_gdbserver_port && min_gdbserver_port < max_gdbserver_port) {
@@ -269,7 +267,7 @@ int main_platform(int argc, char *argv[]) {
 
   if (show_usage || option_error) {
     display_usage(progname, subcommand);
-    exit(option_error);
+    return option_error;
   }
 
   // Skip any options we consumed with getopt_long_only.
@@ -288,13 +286,13 @@ int main_platform(int argc, char *argv[]) {
       listen_host_port, children_inherit_listen_socket, error));
   if (error.Fail()) {
     fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-    exit(socket_error);
+    return 1;
   }
 
   error = acceptor_up->Listen(backlog);
   if (error.Fail()) {
     printf("failed to listen: %s\n", error.AsCString());
-    exit(socket_error);
+    return 1;
   }
   if (socket_file) {
     error =
@@ -322,7 +320,7 @@ int main_platform(int argc, char *argv[]) {
     error = acceptor_up->Accept(children_inherit_accept_socket, conn);
     if (error.Fail()) {
       WithColor::error() << error.AsCString() << '\n';
-      exit(socket_error);
+      return 1;
     }
     printf("Connection established.\n");
     if (g_server) {
index 1e001ac..89acb0f 100644 (file)
@@ -11,6 +11,7 @@
 #include "lldb/lldb-private.h"
 
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/ManagedStatic.h"
@@ -52,29 +53,30 @@ int main(int argc, char *argv[]) {
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
-  int option_error = 0;
   const char *progname = argv[0];
   if (argc < 2) {
     display_usage(progname);
-    exit(option_error);
+    return 1;
   }
 
   switch (argv[1][0]) {
-  case 'g':
+  case 'g': {
     llgs::initialize();
-    main_gdbserver(argc, argv);
-    llgs::terminate_debugger();
-    break;
-  case 'p':
+    auto deinit = llvm::make_scope_exit([]() { llgs::terminate_debugger(); });
+    return main_gdbserver(argc, argv);
+  }
+  case 'p': {
     llgs::initialize();
-    main_platform(argc, argv);
-    llgs::terminate_debugger();
-    break;
-  case 'v':
+    auto deinit = llvm::make_scope_exit([]() { llgs::terminate_debugger(); });
+    return main_platform(argc, argv);
+  }
+  case 'v': {
     fprintf(stderr, "%s\n", lldb_private::GetVersion());
-    break;
-  default:
+    return 0;
+  }
+  default: {
     display_usage(progname);
-    exit(option_error);
+    return 1;
+  }
   }
 }