[lldb] update lldb-server platform help parsing (attempt 3) (#164904)

* original change #162730
* with windows fix #164843
* remove timeout that was pointed out in the comment above
* Remove test that starts and listens on a socket to avoid timeout
issues

lldb/test/Shell/lldb-server/TestPlatformErrorMessages.test

@@ -0,0 +1,25 @@
+RUN: %platformserver 2>&1 | FileCheck --check-prefixes=NO_LISTEN,ALL %s
+NO_LISTEN: error: either --listen or --child-platform-fd is required
+
+RUN: %lldb-server platform --listen 2>&1 | FileCheck --check-prefixes=LISTEN_MISSING,ALL %s
+LISTEN_MISSING: error: --listen: missing argument
+
+RUN: %lldb-server p --bogus 2>&1 | FileCheck --check-prefixes=BOGUS,ALL %s
+BOGUS: error: unknown argument '--bogus'
+
+RUN: %platformserver --gdbserver-port 2>&1 | FileCheck --check-prefixes=GDBPORT_MISSING,ALL %s
+GDBPORT_MISSING: error: --gdbserver-port: missing argument
+
+RUN: %platformserver --gdbserver-port notanumber --listen :1234 2>&1 | FileCheck --check-prefixes=GDBPORT_INVALID %s
+GDBPORT_INVALID: error: invalid --gdbserver-port value
+
+RUN: %platformserver --socket-file 2>&1 | FileCheck --check-prefixes=SOCKETFILE_MISSING,ALL %s
+SOCKETFILE_MISSING: error: --socket-file: missing argument
+
+RUN: %platformserver --log-file 2>&1 | FileCheck --check-prefixes=LOGFILE_MISSING,ALL %s
+LOGFILE_MISSING: error: --log-file: missing argument
+
+RUN: %platformserver --log-channels 2>&1 | FileCheck --check-prefixes=LOGCHANNELS_MISSING,ALL %s
+LOGCHANNELS_MISSING: error: --log-channels: missing argument
+
+ALL: Use 'lldb-server{{(\.exe)?}} {{p|platform}} --help' for a complete list of options.

lldb/test/Shell/lldb-server/TestPlatformHelp.test

@@ -0,0 +1,40 @@
+RUN: %platformserver --help 2>&1 | FileCheck %s
+RUN: %platformserver -h 2>&1 | FileCheck %s
+RUN: %lldb-server p --help 2>&1 | FileCheck %s
+RUN: %lldb-server p -h 2>&1 | FileCheck %s
+RUN: %lldb-server platform --help 2>&1 | FileCheck %s
+RUN: %lldb-server platform -h 2>&1 | FileCheck %s
+
+CHECK: OVERVIEW: lldb-server{{(\.exe)?}} platform
+
+CHECK: USAGE: lldb-server{{(\.exe)?}} {{p|platform}} [options] --listen <[host]:port> {{\[}}[--] program args...]
+
+CHECK: CONNECTION OPTIONS:
+CHECK: --gdbserver-port <port>
+CHECK-SAME: Short form: -P
+CHECK: --listen <[host]:port>
+CHECK-SAME: Short form: -L
+CHECK: --socket-file <path>
+CHECK-SAME: Short form: -f
+
+CHECK: GENERAL OPTIONS:
+CHECK: --help
+CHECK: --log-channels <channel1 categories...:channel2 categories...>
+CHECK: Short form: -c
+CHECK: --log-file <file>
+CHECK-SAME: Short form: -l
+CHECK: --server
+
+CHECK: OPTIONS:
+CHECK: -- program args
+
+CHECK: DESCRIPTION
+CHECK: Acts as a platform server for remote debugging
+
+CHECK: EXAMPLES
+CHECK: # Listen on port 1234, exit after first connection
+CHECK: lldb-server{{(\.exe)?}} platform --listen tcp://0.0.0.0:1234
+CHECK: # Listen on port 5555, accept multiple connections
+CHECK: lldb-server{{(\.exe)?}} platform --server --listen tcp://localhost:5555
+CHECK: # Listen on Unix domain socket
+CHECK: lldb-server{{(\.exe)?}} platform --listen unix:///tmp/lldb-server.sock

lldb/tools/lldb-server/CMakeLists.txt

@@ -2,6 +2,10 @@ set(LLVM_TARGET_DEFINITIONS LLGSOptions.td)
 tablegen(LLVM LLGSOptions.inc -gen-opt-parser-defs)
 add_public_tablegen_target(LLGSOptionsTableGen)
 
+set(LLVM_TARGET_DEFINITIONS PlatformOptions.td)
+tablegen(LLVM PlatformOptions.inc -gen-opt-parser-defs)
+add_public_tablegen_target(PlatformOptionsTableGen)
+
 set(LLDB_PLUGINS)
 
 if(CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
@@ -67,6 +71,7 @@ add_lldb_tool(lldb-server
 
 add_dependencies(lldb-server
   LLGSOptionsTableGen
+  PlatformOptionsTableGen
   ${tablegen_deps}
 )
 target_include_directories(lldb-server PRIVATE "${LLDB_SOURCE_DIR}/source")

lldb/tools/lldb-server/PlatformOptions.td

@@ -0,0 +1,75 @@
+include "llvm/Option/OptParser.td"
+
+class F<string name>: Flag<["--", "-"], name>;
+class R<list<string> prefixes, string name>
+  : Option<prefixes, name, KIND_REMAINING_ARGS>;
+
+multiclass SJ<string name, string help> {
+  def NAME: Separate<["--", "-"], name>,
+    HelpText<help>;
+  def NAME # _eq: Joined<["--", "-"], name # "=">,
+    Alias<!cast<Separate>(NAME)>;
+}
+
+def grp_connect : OptionGroup<"connection">, HelpText<"CONNECTION OPTIONS">;
+
+defm listen: SJ<"listen", "Host and port to listen on. Format: [host]:port or protocol://[host]:port (e.g., tcp://localhost:1234, unix:///path/to/socket). Short form: -L">,
+  MetaVarName<"<[host]:port>">,
+  Group<grp_connect>;
+def: Separate<["-"], "L">, Alias<listen>,
+  Group<grp_connect>;
+
+defm socket_file: SJ<"socket-file", "Write listening socket information (port number for TCP or path for Unix domain sockets) to the specified file. Short form: -f">,
+  MetaVarName<"<path>">,
+  Group<grp_connect>;
+def: Separate<["-"], "f">, Alias<socket_file>,
+  Group<grp_connect>;
+
+defm gdbserver_port: SJ<"gdbserver-port", "Port to use for spawned gdbserver instances. If 0 or unspecified, a port will be chosen automatically. Short form: -P">,
+  MetaVarName<"<port>">,
+  Group<grp_connect>;
+def: Separate<["-"], "P">, Alias<gdbserver_port>,
+  Group<grp_connect>;
+
+defm child_platform_fd: SJ<"child-platform-fd", "File descriptor for communication with parent platform process (internal use only).">,
+  MetaVarName<"<fd>">,
+  Group<grp_connect>,
+  Flags<[HelpHidden]>;
+
+def grp_general : OptionGroup<"general options">, HelpText<"GENERAL OPTIONS">;
+
+def server: F<"server">,
+  HelpText<"Run in server mode, accepting multiple client connections sequentially. Without this flag, the server exits after handling the first connection.">,
+  Group<grp_general>;
+
+defm log_channels: SJ<"log-channels", "Channels to log. A colon-separated list of entries. Each entry starts with a channel followed by a space-separated list of categories. Common channels: lldb, gdb-remote, platform, process. Short form: -c">,
+  MetaVarName<"<channel1 categories...:channel2 categories...>">,
+  Group<grp_general>;
+def: Separate<["-"], "c">, Alias<log_channels>,
+  Group<grp_general>;
+
+defm log_file: SJ<"log-file", "Destination file to log to. If empty, log to stderr. Short form: -l">,
+  MetaVarName<"<file>">,
+  Group<grp_general>;
+def: Separate<["-"], "l">, Alias<log_file>,
+  Group<grp_general>;
+
+def debug: F<"debug">,
+  HelpText<"(Unused, kept for backward compatibility)">,
+  Group<grp_general>,
+  Flags<[HelpHidden]>;
+
+def verbose: F<"verbose">,
+  HelpText<"(Unused, kept for backward compatibility)">,
+  Group<grp_general>,
+  Flags<[HelpHidden]>;
+
+def help: F<"help">, 
+  HelpText<"Display this help message and exit.">,
+  Group<grp_general>;
+def: Flag<["-"], "h">, Alias<help>,
+  Group<grp_general>;
+
+def REM : R<["--"], "">, 
+  HelpText<"Arguments to pass to launched gdbserver instances.">,
+  MetaVarName<"program args">;

lldb/tools/lldb-server/lldb-platform.cpp

@@ -21,6 +21,9 @@
 #include <fstream>
 #include <optional>
 
+#include "llvm/Option/ArgList.h"
+#include "llvm/Option/OptTable.h"
+#include "llvm/Option/Option.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/WithColor.h"
@@ -56,22 +59,69 @@ using namespace llvm;
 // of target CPUs. For now, let's just use 100.
 static const int backlog = 100;
 static const int socket_error = -1;
-static int g_debug = 0;
-static int g_verbose = 0;
-static int g_server = 0;
 
-// option descriptors for getopt_long_only()
-static struct option g_long_options[] = {
-    {"debug", no_argument, &g_debug, 1},
-    {"verbose", no_argument, &g_verbose, 1},
-    {"log-file", required_argument, nullptr, 'l'},
-    {"log-channels", required_argument, nullptr, 'c'},
-    {"listen", required_argument, nullptr, 'L'},
-    {"gdbserver-port", required_argument, nullptr, 'P'},
-    {"socket-file", required_argument, nullptr, 'f'},
-    {"server", no_argument, &g_server, 1},
-    {"child-platform-fd", required_argument, nullptr, 2},
-    {nullptr, 0, nullptr, 0}};
+namespace {
+using namespace llvm::opt;
+
+enum ID {
+  OPT_INVALID = 0, // This is not an option ID.
+#define OPTION(...) LLVM_MAKE_OPT_ID(__VA_ARGS__),
+#include "PlatformOptions.inc"
+#undef OPTION
+};
+
+#define OPTTABLE_STR_TABLE_CODE
+#include "PlatformOptions.inc"
+#undef OPTTABLE_STR_TABLE_CODE
+
+#define OPTTABLE_PREFIXES_TABLE_CODE
+#include "PlatformOptions.inc"
+#undef OPTTABLE_PREFIXES_TABLE_CODE
+
+static constexpr opt::OptTable::Info InfoTable[] = {
+#define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__),
+#include "PlatformOptions.inc"
+#undef OPTION
+};
+
+class PlatformOptTable : public opt::GenericOptTable {
+public:
+  PlatformOptTable()
+      : opt::GenericOptTable(OptionStrTable, OptionPrefixesTable, InfoTable) {}
+
+  void PrintHelp(llvm::StringRef Name) {
+    std::string Usage =
+        (Name + " [options] --listen <[host]:port> [[--] program args...]")
+            .str();
+
+    std::string Title = "lldb-server platform";
+
+    OptTable::printHelp(llvm::outs(), Usage.c_str(), Title.c_str());
+
+    llvm::outs() << R"(
+DESCRIPTION
+  Acts as a platform server for remote debugging. When LLDB clients connect,
+  the platform server handles platform operations (file transfers, process
+  launching) and spawns debug server instances (lldb-server gdbserver) to
+  handle actual debugging sessions.
+
+  By default, the server exits after handling one connection. Use --server
+  to keep running and accept multiple connections sequentially.
+
+EXAMPLES
+  # Listen on port 1234, exit after first connection
+  lldb-server platform --listen tcp://0.0.0.0:1234
+
+  # Listen on port 5555, accept multiple connections
+  lldb-server platform --server --listen tcp://localhost:5555
+
+  # Listen on Unix domain socket
+  lldb-server platform --listen unix:///tmp/lldb-server.sock
+
+)";
+  }
+};
+} // namespace
 
 #if defined(__APPLE__)
 #define LOW_PORT (IPPORT_RESERVED)
@@ -97,12 +147,11 @@ static void signal_handler(int signo) {
 }
 #endif
 
-static void display_usage(const char *progname, const char *subcommand) {
-  fprintf(stderr, "Usage:\n  %s %s [--log-file log-file-name] [--log-channels "
-                  "log-channel-list] [--port-file port-file-path] --server "
-                  "--listen port\n",
-          progname, subcommand);
-  exit(0);
+static void display_usage(PlatformOptTable &Opts, const char *progname,
+                          const char *subcommand) {
+  std::string Name =
+      (llvm::sys::path::filename(progname) + " " + subcommand).str();
+  Opts.PrintHelp(Name);
 }
 
 static Status parse_listen_host_port(Socket::SocketProtocol &protocol,
@@ -261,7 +310,8 @@ static Status spawn_process(const char *progname, const FileSpec &prog,
                             const Socket *conn_socket, uint16_t gdb_port,
                             const lldb_private::Args &args,
                             const std::string &log_file,
-                            const StringRef log_channels, MainLoop &main_loop) {
+                            const StringRef log_channels, MainLoop &main_loop,
+                            bool multi_client) {
   Status error;
   SharedSocket shared_socket(conn_socket, error);
   if (error.Fail())
@@ -297,9 +347,12 @@ static Status spawn_process(const char *progname, const FileSpec &prog,
 
   launch_info.SetLaunchInSeparateProcessGroup(false);
 
-  if (g_server)
+  // Set up process monitor callback based on whether we're in server mode.
+  if (multi_client)
+    // In server mode: empty callback (don't terminate when child exits).
     launch_info.SetMonitorProcessCallback([](lldb::pid_t, int, int) {});
   else
+    // In single-client mode: terminate main loop when child exits.
     launch_info.SetMonitorProcessCallback([&main_loop](lldb::pid_t, int, int) {
       main_loop.AddPendingCallback(
           [](MainLoopBase &loop) { loop.RequestTermination(); });
@@ -371,107 +424,101 @@ int main_platform(int argc, char *argv[]) {
   signal(SIGPIPE, SIG_IGN);
   signal(SIGHUP, signal_handler);
 #endif
-  int long_option_index = 0;
+
+  // Special handling for 'help' as first argument.
+  if (argc > 0 && strcmp(argv[0], "help") == 0) {
+    PlatformOptTable Opts;
+    display_usage(Opts, progname, subcommand);
+    return EXIT_SUCCESS;
+  }
+
   Status error;
-  std::string listen_host_port;
-  int ch;
-
-  std::string log_file;
-  StringRef
-      log_channels; // e.g. "lldb process threads:gdb-remote default:linux all"
-
   shared_fd_t fd = SharedSocket::kInvalidFD;
-
   uint16_t gdbserver_port = 0;
-
   FileSpec socket_file;
-  bool show_usage = false;
-  int option_error = 0;
 
-  std::string short_options(OptionParser::GetShortOptionString(g_long_options));
+  PlatformOptTable Opts;
+  BumpPtrAllocator Alloc;
+  StringSaver Saver(Alloc);
+  bool HasError = false;
 
-#if __GLIBC__
-  optind = 0;
-#else
-  optreset = 1;
-  optind = 1;
-#endif
+  opt::InputArgList Args =
+      Opts.parseArgs(argc, argv, OPT_UNKNOWN, Saver, [&](llvm::StringRef Msg) {
+        WithColor::error() << Msg << "\n";
+        HasError = true;
+      });
 
-  while ((ch = getopt_long_only(argc, argv, short_options.c_str(),
-                                g_long_options, &long_option_index)) != -1) {
-    switch (ch) {
-    case 0: // Any optional that auto set themselves will return 0
-      break;
+  std::string Name =
+      (llvm::sys::path::filename(progname) + " " + subcommand).str();
+  std::string HelpText =
+      "Use '" + Name + " --help' for a complete list of options.\n";
 
-    case 'L':
-      listen_host_port.append(optarg);
-      break;
+  if (HasError) {
+    llvm::errs() << HelpText;
+    return EXIT_FAILURE;
+  }
 
-    case 'l': // Set Log File
-      if (optarg && optarg[0])
-        log_file.assign(optarg);
-      break;
+  if (Args.hasArg(OPT_help)) {
+    display_usage(Opts, progname, subcommand);
+    return EXIT_SUCCESS;
+  }
 
-    case 'c': // Log Channels
-      if (optarg && optarg[0])
-        log_channels = StringRef(optarg);
-      break;
+  // Parse arguments.
+  std::string listen_host_port = Args.getLastArgValue(OPT_listen).str();
+  std::string log_file = Args.getLastArgValue(OPT_log_file).str();
+  StringRef log_channels = Args.getLastArgValue(OPT_log_channels);
+  bool multi_client = Args.hasArg(OPT_server);
+  [[maybe_unused]] bool debug = Args.hasArg(OPT_debug);
+  [[maybe_unused]] bool verbose = Args.hasArg(OPT_verbose);
 
-    case 'f': // Socket file
-      if (optarg && optarg[0])
-        socket_file.SetFile(optarg, FileSpec::Style::native);
-      break;
+  if (Args.hasArg(OPT_socket_file)) {
+    socket_file.SetFile(Args.getLastArgValue(OPT_socket_file),
+                        FileSpec::Style::native);
+  }
 
-    case 'P':
-    case 'm':
-    case 'M': {
-      uint16_t portnum;
-      if (!llvm::to_integer(optarg, portnum)) {
-        WithColor::error() << "invalid port number string " << optarg << "\n";
-        option_error = 2;
-        break;
-      }
-      // Note the condition gdbserver_port > HIGH_PORT is valid in case of using
-      // --child-platform-fd. Check gdbserver_port later.
-      if (ch == 'P')
-        gdbserver_port = portnum;
-      else if (gdbserver_port == 0)
-        gdbserver_port = portnum;
-    } break;
-
-    case 2: {
-      uint64_t _fd;
-      if (!llvm::to_integer(optarg, _fd)) {
-        WithColor::error() << "invalid fd " << optarg << "\n";
-        option_error = 6;
-      } else
-        fd = (shared_fd_t)_fd;
-    } break;
-
-    case 'h': /* fall-through is intentional */
-    case '?':
-      show_usage = true;
-      break;
+  if (Args.hasArg(OPT_gdbserver_port)) {
+    if (!llvm::to_integer(Args.getLastArgValue(OPT_gdbserver_port),
+                          gdbserver_port)) {
+      WithColor::error() << "invalid --gdbserver-port value\n";
+      return EXIT_FAILURE;
     }
   }
 
+  if (Args.hasArg(OPT_child_platform_fd)) {
+    uint64_t _fd;
+    if (!llvm::to_integer(Args.getLastArgValue(OPT_child_platform_fd), _fd)) {
+      WithColor::error() << "invalid --child-platform-fd value\n";
+      return EXIT_FAILURE;
+    }
+    fd = (shared_fd_t)_fd;
+  }
+
   if (!LLDBServerUtilities::SetupLogging(log_file, log_channels, 0))
     return -1;
 
   // Print usage and exit if no listening port is specified.
-  if (listen_host_port.empty() && fd == SharedSocket::kInvalidFD)
-    show_usage = true;
-
-  if (show_usage || option_error) {
-    display_usage(progname, subcommand);
-    exit(option_error);
+  if (listen_host_port.empty() && fd == SharedSocket::kInvalidFD) {
+    WithColor::error() << "either --listen or --child-platform-fd is required\n"
+                       << HelpText;
+    return EXIT_FAILURE;
+  }
+
+  // Get remaining arguments for inferior.
+  std::vector<llvm::StringRef> Inputs;
+  for (opt::Arg *Arg : Args.filtered(OPT_INPUT))
+    Inputs.push_back(Arg->getValue());
+  if (opt::Arg *Arg = Args.getLastArg(OPT_REM)) {
+    for (const char *Val : Arg->getValues())
+      Inputs.push_back(Val);
   }
 
-  // Skip any options we consumed with getopt_long_only.
-  argc -= optind;
-  argv += optind;
   lldb_private::Args inferior_arguments;
-  inferior_arguments.SetArguments(argc, const_cast<const char **>(argv));
+  if (!Inputs.empty()) {
+    std::vector<const char *> args_ptrs;
+    for (const auto &Input : Inputs)
+      args_ptrs.push_back(Input.data());
+    inferior_arguments.SetArguments(args_ptrs.size(), args_ptrs.data());
+  }
 
   FileSpec debugserver_path = GetDebugserverPath();
   if (!debugserver_path) {
@@ -514,7 +561,7 @@ int main_platform(int argc, char *argv[]) {
     platform.SetConnection(
         std::make_unique<ConnectionFileDescriptor>(std::move(socket)));
     client_handle(platform, inferior_arguments);
-    return 0;
+    return EXIT_SUCCESS;
   }
 
   if (gdbserver_port != 0 &&
@@ -522,7 +569,7 @@ int main_platform(int argc, char *argv[]) {
     WithColor::error() << llvm::formatv("Port number {0} is not in the "
                                         "valid user port range of {1} - {2}\n",
                                         gdbserver_port, LOW_PORT, HIGH_PORT);
-    return 1;
+    return EXIT_FAILURE;
   }
 
   Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
@@ -559,7 +606,7 @@ int main_platform(int argc, char *argv[]) {
     if (error.Fail()) {
       fprintf(stderr, "failed to write socket id to %s: %s\n",
               socket_file.GetPath().c_str(), error.AsCString());
-      return 1;
+      return EXIT_FAILURE;
     }
   }
 
@@ -577,22 +624,22 @@ int main_platform(int argc, char *argv[]) {
     llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>> platform_handles =
         platform_sock->Accept(
             main_loop, [progname, gdbserver_port, &inferior_arguments, log_file,
-                        log_channels, &main_loop,
+                        log_channels, &main_loop, multi_client,
                         &platform_handles](std::unique_ptr<Socket> sock_up) {
               printf("Connection established.\n");
               Status error = spawn_process(
                   progname, HostInfo::GetProgramFileSpec(), sock_up.get(),
                   gdbserver_port, inferior_arguments, log_file, log_channels,
-                  main_loop);
+                  main_loop, multi_client);
               if (error.Fail()) {
                 Log *log = GetLog(LLDBLog::Platform);
                 LLDB_LOGF(log, "spawn_process failed: %s", error.AsCString());
                 WithColor::error()
                     << "spawn_process failed: " << error.AsCString() << "\n";
-                if (!g_server)
+                if (!multi_client)
                   main_loop.RequestTermination();
               }
-              if (!g_server)
+              if (!multi_client)
                 platform_handles->clear();
             });
     if (!platform_handles) {
@@ -616,5 +663,5 @@ int main_platform(int argc, char *argv[]) {
 
   fprintf(stderr, "lldb-server exiting...\n");
 
-  return 0;
+  return EXIT_SUCCESS;
 }