[Lldb-commits] [lldb] fa5fa63 - [lldb] Port lldb gdb-server to libOption
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Wed Oct 21 07:16:32 PDT 2020
Author: Pavel Labath
Date: 2020-10-21T16:16:18+02:00
New Revision: fa5fa63fd140f2d4bad0357839378606a583b32c
URL: https://github.com/llvm/llvm-project/commit/fa5fa63fd140f2d4bad0357839378606a583b32c
DIFF: https://github.com/llvm/llvm-project/commit/fa5fa63fd140f2d4bad0357839378606a583b32c.diff
LOG: [lldb] Port lldb gdb-server to libOption
The existing help text was very terse and was missing several important
options. In the new version, I add a short description of each option
and a slightly longer description of the tool as a whole.
The new option list does not include undocumented no-op options:
--debug and --verbose. It also does not include undocumented short
aliases for long options, with two exceptions: -h, because it's
well-known; and -S (--setsid), as it's used in one test. Using these
options will now produce an error. I believe that is acceptable as users
aren't generally invoking lldb-server directly, and the only way to
learn about the short aliases was by looking at the source.
Differential Revision: https://reviews.llvm.org/D89477
Added:
lldb/test/Shell/lldb-server/TestErrorMessages.test
lldb/tools/lldb-server/LLGSOptions.td
Modified:
lldb/include/lldb/Utility/Args.h
lldb/source/Utility/Args.cpp
lldb/tools/lldb-server/CMakeLists.txt
lldb/tools/lldb-server/lldb-gdbserver.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Utility/Args.h b/lldb/include/lldb/Utility/Args.h
index 2cce7d0c697c..82e6d147ae56 100644
--- a/lldb/include/lldb/Utility/Args.h
+++ b/lldb/include/lldb/Utility/Args.h
@@ -66,6 +66,7 @@ class Args {
Args(const Args &rhs);
explicit Args(const StringList &list);
+ explicit Args(llvm::ArrayRef<llvm::StringRef> args);
Args &operator=(const Args &rhs);
diff --git a/lldb/source/Utility/Args.cpp b/lldb/source/Utility/Args.cpp
index 4f3285404b6d..2cbe727ed240 100644
--- a/lldb/source/Utility/Args.cpp
+++ b/lldb/source/Utility/Args.cpp
@@ -175,6 +175,11 @@ Args::Args(const StringList &list) : Args() {
AppendArgument(arg);
}
+Args::Args(llvm::ArrayRef<llvm::StringRef> args) : Args() {
+ for (llvm::StringRef arg : args)
+ AppendArgument(arg);
+}
+
Args &Args::operator=(const Args &rhs) {
Clear();
diff --git a/lldb/test/Shell/lldb-server/TestErrorMessages.test b/lldb/test/Shell/lldb-server/TestErrorMessages.test
new file mode 100644
index 000000000000..ef64ec6e5aba
--- /dev/null
+++ b/lldb/test/Shell/lldb-server/TestErrorMessages.test
@@ -0,0 +1,14 @@
+RUN: lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
+FD1: error: --fd: missing argument
+
+RUN: lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,ALL %s
+FD2: error: invalid '--fd' argument
+
+RUN: lldb-server gdbserver --bogus 2>&1 | FileCheck --check-prefixes=BOGUS,ALL %s
+BOGUS: error: unknown argument '--bogus'
+
+RUN: lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
+CONN: error: no connection arguments
+
+ALL: Use '{{.*}} g[dbserver] --help' for a complete list of options.
+
diff --git a/lldb/tools/lldb-server/CMakeLists.txt b/lldb/tools/lldb-server/CMakeLists.txt
index 6e7b30df5c58..930c327cf072 100644
--- a/lldb/tools/lldb-server/CMakeLists.txt
+++ b/lldb/tools/lldb-server/CMakeLists.txt
@@ -1,3 +1,8 @@
+set(LLVM_TARGET_DEFINITIONS LLGSOptions.td)
+tablegen(LLVM LLGSOptions.inc -gen-opt-parser-defs)
+add_public_tablegen_target(LLGSOptionsTableGen)
+set_target_properties(LLGSOptionsTableGen PROPERTIES FOLDER "lldb misc")
+
set(LLDB_PLUGINS)
if(CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
@@ -53,8 +58,13 @@ add_lldb_tool(lldb-server
${LLDB_SYSTEM_LIBS}
LINK_COMPONENTS
+ Option
Support
)
+add_dependencies(lldb-server
+ LLGSOptionsTableGen
+ ${tablegen_deps}
+)
target_include_directories(lldb-server PRIVATE "${LLDB_SOURCE_DIR}/source")
target_link_libraries(lldb-server PRIVATE ${LLDB_SYSTEM_LIBS})
diff --git a/lldb/tools/lldb-server/LLGSOptions.td b/lldb/tools/lldb-server/LLGSOptions.td
new file mode 100644
index 000000000000..429a4671764f
--- /dev/null
+++ b/lldb/tools/lldb-server/LLGSOptions.td
@@ -0,0 +1,62 @@
+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">;
+
+defm fd: SJ<"fd", "Communicate over the given file descriptor.">,
+ MetaVarName<"<fd>">,
+ Group<grp_connect>;
+
+defm named_pipe: SJ<"named-pipe", "Write port lldb-server will listen on to the given named pipe.">,
+ MetaVarName<"<name>">,
+ Group<grp_connect>;
+
+defm pipe: SJ<"pipe", "Write port lldb-server will listen on to the given file descriptor.">,
+ MetaVarName<"<fd>">,
+ Group<grp_connect>;
+
+def reverse_connect: F<"reverse-connect">,
+ HelpText<"Connect to the client instead of passively waiting for a connection. In this case [host]:port denotes the remote address to connect to.">,
+ Group<grp_connect>;
+
+def grp_general : OptionGroup<"general options">, HelpText<"GENERAL OPTIONS">;
+
+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.">,
+ MetaVarName<"<channel1 categories...:channel2 categories...>">,
+ Group<grp_general>;
+
+defm log_file: SJ<"log-file", "Destination file to log to. If empty, log to stderr.">,
+ MetaVarName<"<file>">,
+ Group<grp_general>;
+
+def setsid: F<"setsid">, HelpText<"Run lldb-server in a new session.">,
+ Group<grp_general>;
+def: Flag<["-"], "S">, Alias<setsid>,
+ Group<grp_general>;
+
+def help: F<"help">, HelpText<"Prints out the usage information for lldb-server.">,
+ Group<grp_general>;
+def: Flag<["-"], "h">, Alias<help>,
+ Group<grp_general>;
+
+def grp_target : OptionGroup<"target selection">, HelpText<"TARGET SELECTION">;
+
+defm attach: SJ<"attach", "Attach to the process given by a (numeric) process id or a name.">,
+ MetaVarName<"<pid-or-name>">,
+ Group<grp_target>;
+
+def REM : R<["--"], "">, HelpText<"Launch program for debugging.">,
+ MetaVarName<"program args">,
+ Group<grp_target>;
+
+def: F<"native-regs">; // Noop. Present for backwards compatibility only.
diff --git a/lldb/tools/lldb-server/lldb-gdbserver.cpp b/lldb/tools/lldb-server/lldb-gdbserver.cpp
index 633e37c3a043..0fbb13800bf7 100644
--- a/lldb/tools/lldb-server/lldb-gdbserver.cpp
+++ b/lldb/tools/lldb-server/lldb-gdbserver.cpp
@@ -17,7 +17,6 @@
#include <unistd.h>
#endif
-
#include "Acceptor.h"
#include "LLDBServerUtilities.h"
#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h"
@@ -25,8 +24,6 @@
#include "lldb/Host/Config.h"
#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Host/FileSystem.h"
-#include "lldb/Host/HostGetOpt.h"
-#include "lldb/Host/OptionParser.h"
#include "lldb/Host/Pipe.h"
#include "lldb/Host/Socket.h"
#include "lldb/Host/StringConvert.h"
@@ -34,7 +31,11 @@
#include "lldb/Target/Process.h"
#include "lldb/Utility/Status.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Option/OptTable.h"
+#include "llvm/Option/Option.h"
#include "llvm/Support/Errno.h"
+#include "llvm/Support/WithColor.h"
#if defined(__linux__)
#include "Plugins/Process/Linux/NativeProcessLinux.h"
@@ -88,31 +89,6 @@ class NativeProcessFactory : public NativeProcessProtocol::Factory {
#endif
}
-// option descriptors for getopt_long_only()
-
-static int g_debug = 0;
-static int g_verbose = 0;
-
-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'},
- {"attach", required_argument, nullptr, 'a'},
- {"named-pipe", required_argument, nullptr, 'N'},
- {"pipe", required_argument, nullptr, 'U'},
- {"native-regs", no_argument, nullptr,
- 'r'}, // Specify to use the native registers instead of the gdb defaults
- // for the architecture. NOTE: this is a do-nothing arg as it's
- // behavior is default now. FIXME remove call from lldb-platform.
- {"reverse-connect", no_argument, nullptr,
- 'R'}, // Specifies that llgs attaches to the client address:port rather
- // than llgs listening for a connection from address on port.
- {"setsid", no_argument, nullptr,
- 'S'}, // Call setsid() to make llgs run in its own session.
- {"fd", required_argument, nullptr, 'F'},
- {nullptr, 0, nullptr, 0}};
-
#ifndef _WIN32
// Watch for signals
static int g_sighup_received_count = 0;
@@ -129,20 +105,6 @@ static void sighup_handler(MainLoopBase &mainloop) {
}
#endif // #ifndef _WIN32
-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] "
- "[--setsid] "
- "[--fd file-descriptor]"
- "[--named-pipe named-pipe-path] "
- "[--native-regs] "
- "[--attach pid] "
- "[[HOST]:PORT] "
- "[-- PROGRAM ARG1 ARG2 ...]\n",
- progname, subcommand);
-}
-
void handle_attach_to_pid(GDBRemoteCommunicationServerLLGS &gdb_server,
lldb::pid_t pid) {
Status error = gdb_server.AttachToProcess(pid);
@@ -176,12 +138,12 @@ void handle_attach(GDBRemoteCommunicationServerLLGS &gdb_server,
handle_attach_to_process_name(gdb_server, attach_target);
}
-void handle_launch(GDBRemoteCommunicationServerLLGS &gdb_server, int argc,
- const char *const argv[]) {
+void handle_launch(GDBRemoteCommunicationServerLLGS &gdb_server,
+ llvm::ArrayRef<llvm::StringRef> Arguments) {
ProcessLaunchInfo info;
info.GetFlags().Set(eLaunchFlagStopAtEntry | eLaunchFlagDebug |
eLaunchFlagDisableASLR);
- info.SetArguments(const_cast<const char **>(argv), true);
+ info.SetArguments(Args(Arguments), true);
llvm::SmallString<64> cwd;
if (std::error_code ec = llvm::sys::fs::current_path(cwd)) {
@@ -198,7 +160,7 @@ void handle_launch(GDBRemoteCommunicationServerLLGS &gdb_server, int argc,
Status error = gdb_server.LaunchProcess();
if (error.Fail()) {
llvm::errs() << llvm::formatv("error: failed to launch '{0}': {1}\n",
- argv[0], error);
+ Arguments[0], error);
exit(1);
}
}
@@ -229,7 +191,7 @@ Status writeSocketIdToPipe(lldb::pipe_t unnamed_pipe,
void ConnectToRemote(MainLoop &mainloop,
GDBRemoteCommunicationServerLLGS &gdb_server,
- bool reverse_connect, const char *const host_and_port,
+ bool reverse_connect, llvm::StringRef host_and_port,
const char *const progname, const char *const subcommand,
const char *const named_pipe_path, pipe_t unnamed_pipe,
int connection_fd) {
@@ -258,7 +220,7 @@ void ConnectToRemote(MainLoop &mainloop,
connection_url, error.AsCString());
exit(-1);
}
- } else if (host_and_port && host_and_port[0]) {
+ } else if (!host_and_port.empty()) {
// Parse out host and port.
std::string final_host_and_port;
std::string connection_host;
@@ -269,7 +231,7 @@ void ConnectToRemote(MainLoop &mainloop,
// expect the remainder to be the port.
if (host_and_port[0] == ':')
final_host_and_port.append("localhost");
- final_host_and_port.append(host_and_port);
+ final_host_and_port.append(host_and_port.str());
// Note: use rfind, because the host/port may look like "[::1]:12345".
const std::string::size_type colon_pos = final_host_and_port.rfind(':');
@@ -361,7 +323,57 @@ void ConnectToRemote(MainLoop &mainloop,
printf("Connection established.\n");
}
-// main
+namespace {
+enum ID {
+ OPT_INVALID = 0, // This is not an option ID.
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
+ HELPTEXT, METAVAR, VALUES) \
+ OPT_##ID,
+#include "LLGSOptions.inc"
+#undef OPTION
+};
+
+#define PREFIX(NAME, VALUE) const char *const NAME[] = VALUE;
+#include "LLGSOptions.inc"
+#undef PREFIX
+
+const opt::OptTable::Info InfoTable[] = {
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
+ HELPTEXT, METAVAR, VALUES) \
+ { \
+ PREFIX, NAME, HELPTEXT, \
+ METAVAR, OPT_##ID, opt::Option::KIND##Class, \
+ PARAM, FLAGS, OPT_##GROUP, \
+ OPT_##ALIAS, ALIASARGS, VALUES},
+#include "LLGSOptions.inc"
+#undef OPTION
+};
+
+class LLGSOptTable : public opt::OptTable {
+public:
+ LLGSOptTable() : OptTable(InfoTable) {}
+
+ void PrintHelp(llvm::StringRef Name) {
+ std::string Usage =
+ (Name + " [options] [[host]:port] [[--] program args...]").str();
+ OptTable::PrintHelp(llvm::outs(), Usage.c_str(), "lldb-server");
+ llvm::outs() << R"(
+DESCRIPTION
+ lldb-server connects to the LLDB client, which drives the debugging session.
+ If no connection options are given, the [host]:port argument must be present
+ and will denote the address that lldb-server will listen on. [host] defaults
+ to "localhost" if empty. Port can be zero, in which case the port number will
+ be chosen dynamically and written to destinations given by --named-pipe and
+ --pipe arguments.
+
+ If no target is selected at startup, lldb-server can be directed by the LLDB
+ client to launch or attach to a process.
+
+)";
+ }
+};
+} // namespace
+
int main_gdbserver(int argc, char *argv[]) {
Status error;
MainLoop mainloop;
@@ -374,10 +386,6 @@ int main_gdbserver(int argc, char *argv[]) {
const char *progname = argv[0];
const char *subcommand = argv[1];
- argc--;
- argv++;
- int long_option_index = 0;
- int ch;
std::string attach_target;
std::string named_pipe_path;
std::string log_file;
@@ -390,94 +398,69 @@ int main_gdbserver(int argc, char *argv[]) {
// ProcessLaunchInfo launch_info;
ProcessAttachInfo attach_info;
- bool show_usage = false;
- int option_error = 0;
-#if __GLIBC__
- optind = 0;
-#else
- optreset = 1;
- optind = 1;
-#endif
-
- std::string short_options(OptionParser::GetShortOptionString(g_long_options));
-
- 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;
-
- case 'l': // Set Log File
- if (optarg && optarg[0])
- log_file.assign(optarg);
- break;
-
- case 'c': // Log Channels
- if (optarg && optarg[0])
- log_channels = StringRef(optarg);
- break;
-
- case 'N': // named pipe
- if (optarg && optarg[0])
- named_pipe_path = optarg;
- break;
-
- case 'U': // unnamed pipe
- if (optarg && optarg[0])
- unnamed_pipe = (pipe_t)StringConvert::ToUInt64(optarg, -1);
- break;
-
- case 'r':
- // Do nothing, native regs is the default these days
- break;
-
- case 'R':
- reverse_connect = true;
- break;
+ LLGSOptTable Opts;
+ llvm::BumpPtrAllocator Alloc;
+ llvm::StringSaver Saver(Alloc);
+ bool HasError = false;
+ opt::InputArgList Args = Opts.parseArgs(argc - 1, argv + 1, OPT_UNKNOWN,
+ Saver, [&](llvm::StringRef Msg) {
+ WithColor::error() << Msg << "\n";
+ HasError = true;
+ });
+ std::string Name =
+ (llvm::sys::path::filename(argv[0]) + " g[dbserver]").str();
+ std::string HelpText =
+ "Use '" + Name + " --help' for a complete list of options.\n";
+ if (HasError) {
+ llvm::errs() << HelpText;
+ return 1;
+ }
- case 'F':
- connection_fd = StringConvert::ToUInt32(optarg, -1);
- break;
+ if (Args.hasArg(OPT_help)) {
+ Opts.PrintHelp(Name);
+ return 0;
+ }
#ifndef _WIN32
- case 'S':
- // Put llgs into a new session. Terminals group processes
- // into sessions and when a special terminal key sequences
- // (like control+c) are typed they can cause signals to go out to
- // all processes in a session. Using this --setsid (-S) option
- // will cause debugserver to run in its own sessions and be free
- // from such issues.
- //
- // This is useful when llgs is spawned from a command
- // line application that uses llgs to do the debugging,
- // yet that application doesn't want llgs receiving the
- // signals sent to the session (i.e. dying when anyone hits ^C).
- {
- const ::pid_t new_sid = setsid();
- if (new_sid == -1) {
- llvm::errs() << llvm::formatv(
- "failed to set new session id for {0} ({1})\n", LLGS_PROGRAM_NAME,
- llvm::sys::StrError());
- }
+ if (Args.hasArg(OPT_setsid)) {
+ // Put llgs into a new session. Terminals group processes
+ // into sessions and when a special terminal key sequences
+ // (like control+c) are typed they can cause signals to go out to
+ // all processes in a session. Using this --setsid (-S) option
+ // will cause debugserver to run in its own sessions and be free
+ // from such issues.
+ //
+ // This is useful when llgs is spawned from a command
+ // line application that uses llgs to do the debugging,
+ // yet that application doesn't want llgs receiving the
+ // signals sent to the session (i.e. dying when anyone hits ^C).
+ {
+ const ::pid_t new_sid = setsid();
+ if (new_sid == -1) {
+ WithColor::warning()
+ << llvm::formatv("failed to set new session id for {0} ({1})\n",
+ LLGS_PROGRAM_NAME, llvm::sys::StrError());
}
- break;
+ }
+ }
#endif
- case 'a': // attach {pid|process_name}
- if (optarg && optarg[0])
- attach_target = optarg;
- break;
-
- case 'h': /* fall-through is intentional */
- case '?':
- show_usage = true;
- break;
+ log_file = Args.getLastArgValue(OPT_log_file).str();
+ log_channels = Args.getLastArgValue(OPT_log_channels);
+ named_pipe_path = Args.getLastArgValue(OPT_named_pipe).str();
+ reverse_connect = Args.hasArg(OPT_reverse_connect);
+ attach_target = Args.getLastArgValue(OPT_attach).str();
+ if (Args.hasArg(OPT_pipe)) {
+ if (!llvm::to_integer(Args.getLastArgValue(OPT_pipe), unnamed_pipe)) {
+ WithColor::error() << "invalid '--pipe' argument\n" << HelpText;
+ return 1;
}
}
-
- if (show_usage || option_error) {
- display_usage(progname, subcommand);
- exit(option_error);
+ if (Args.hasArg(OPT_fd)) {
+ if (!llvm::to_integer(Args.getLastArgValue(OPT_fd), connection_fd)) {
+ WithColor::error() << "invalid '--fd' argument\n" << HelpText;
+ return 1;
+ }
}
if (!LLDBServerUtilities::SetupLogging(
@@ -486,30 +469,26 @@ int main_gdbserver(int argc, char *argv[]) {
LLDB_LOG_OPTION_PREPEND_FILE_FUNCTION))
return -1;
- Log *log(lldb_private::GetLogIfAnyCategoriesSet(GDBR_LOG_PROCESS));
- if (log) {
- LLDB_LOGF(log, "lldb-server launch");
- for (int i = 0; i < argc; i++) {
- LLDB_LOGF(log, "argv[%i] = '%s'", i, argv[i]);
- }
+ 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;
-
- if (argc == 0 && connection_fd == -1) {
- fputs("No arguments\n", stderr);
- display_usage(progname, subcommand);
- exit(255);
+ if (Inputs.empty() && connection_fd == -1) {
+ WithColor::error() << "no connection arguments\n" << HelpText;
+ return 1;
}
NativeProcessFactory factory;
GDBRemoteCommunicationServerLLGS gdb_server(mainloop, factory);
- const char *const host_and_port = argv[0];
- argc -= 1;
- argv += 1;
+ llvm::StringRef host_and_port;
+ if (!Inputs.empty()) {
+ host_and_port = Inputs.front();
+ Inputs.erase(Inputs.begin());
+ }
// Any arguments left over are for the program that we need to launch. If
// there
@@ -520,8 +499,8 @@ int main_gdbserver(int argc, char *argv[]) {
// explicitly asked to attach with the --attach={pid|program_name} form.
if (!attach_target.empty())
handle_attach(gdb_server, attach_target);
- else if (argc > 0)
- handle_launch(gdb_server, argc, argv);
+ else if (!Inputs.empty())
+ handle_launch(gdb_server, Inputs);
// Print version info.
printf("%s-%s\n", LLGS_PROGRAM_NAME, LLGS_VERSION_STR);
@@ -532,7 +511,6 @@ int main_gdbserver(int argc, char *argv[]) {
if (!gdb_server.IsConnected()) {
fprintf(stderr, "no connection information provided, unable to run\n");
- display_usage(progname, subcommand);
return 1;
}
More information about the lldb-commits
mailing list