From 87d509e4625df2d76a80f14cab3d420bd58ba20a Mon Sep 17 00:00:00 2001 From: sfan5 Date: Mon, 23 Jan 2023 00:19:30 +0100 Subject: [PATCH] Implement --debugger option to improve UX when debugging crashes (#13157) --- src/filesys.cpp | 11 ++++ src/filesys.h | 2 + src/gettext.cpp | 19 +++--- src/main.cpp | 128 +++++++++++++++++++++++++++++++++++++++ src/porting.cpp | 32 ++++++++++ src/porting.h | 10 +++ util/test_multiplayer.sh | 8 +-- 7 files changed, 193 insertions(+), 17 deletions(-) diff --git a/src/filesys.cpp b/src/filesys.cpp index 7edb60bcd..d610c2311 100644 --- a/src/filesys.cpp +++ b/src/filesys.cpp @@ -127,6 +127,12 @@ bool IsDir(const std::string &path) (attr & FILE_ATTRIBUTE_DIRECTORY)); } +bool IsExecutable(const std::string &path) +{ + DWORD type; + return GetBinaryType(path.c_str(), &type) != 0; +} + bool IsDirDelimiter(char c) { return c == '/' || c == '\\'; @@ -309,6 +315,11 @@ bool IsDir(const std::string &path) return ((statbuf.st_mode & S_IFDIR) == S_IFDIR); } +bool IsExecutable(const std::string &path) +{ + return access(path.c_str(), X_OK) == 0; +} + bool IsDirDelimiter(char c) { return c == '/'; diff --git a/src/filesys.h b/src/filesys.h index a26fe28d6..fa9ddcaa8 100644 --- a/src/filesys.h +++ b/src/filesys.h @@ -60,6 +60,8 @@ bool IsPathAbsolute(const std::string &path); bool IsDir(const std::string &path); +bool IsExecutable(const std::string &path); + inline bool IsFile(const std::string &path) { return PathExists(path) && !IsDir(path); diff --git a/src/gettext.cpp b/src/gettext.cpp index de042cf35..f56738d98 100644 --- a/src/gettext.cpp +++ b/src/gettext.cpp @@ -149,28 +149,25 @@ void init_gettext(const char *path, const std::string &configured_language, "Restarting " PROJECT_NAME_C " in a new environment!" << std::endl; std::string parameters; - - for (unsigned int i = 1; i < argc; i++) { - if (!parameters.empty()) + for (int i = 1; i < argc; i++) { + if (i > 1) parameters += ' '; - - parameters += argv[i]; + parameters += porting::QuoteArgv(argv[i]); } - const char *ptr_parameters = NULL; - + char *ptr_parameters = nullptr; if (!parameters.empty()) - ptr_parameters = parameters.c_str(); + ptr_parameters = ¶meters[0]; // Allow calling without an extension std::string app_name = argv[0]; if (app_name.compare(app_name.size() - 4, 4, ".exe") != 0) app_name += ".exe"; - STARTUPINFO startup_info = {0}; - PROCESS_INFORMATION process_info = {0}; + STARTUPINFO startup_info = {}; + PROCESS_INFORMATION process_info = {}; - bool success = CreateProcess(app_name.c_str(), (char *)ptr_parameters, + bool success = CreateProcess(app_name.c_str(), ptr_parameters, NULL, NULL, false, DETACHED_PROCESS | CREATE_UNICODE_ENVIRONMENT, NULL, NULL, &startup_info, &process_info); diff --git a/src/main.cpp b/src/main.cpp index 35dcb708b..ec805f5d3 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -102,6 +102,7 @@ static void list_game_ids(); static void list_worlds(bool print_name, bool print_path); static bool setup_log_params(const Settings &cmd_args); static bool create_userdata_path(); +static bool use_debugger(int argc, char *argv[]); static bool init_common(const Settings &cmd_args, int argc, char *argv[]); static void uninit_common(); static void startup_message(); @@ -162,6 +163,11 @@ int main(int argc, char *argv[]) if (!setup_log_params(cmd_args)) return 1; + if (cmd_args.getFlag("debugger")) { + if (!use_debugger(argc, argv)) + warningstream << "Continuing without debugger" << std::endl; + } + porting::signal_handler_init(); #ifdef __ANDROID__ @@ -341,6 +347,8 @@ static void set_allowed_options(OptionList *allowed_options) _("Print even more information to console")))); allowed_options->insert(std::make_pair("trace", ValueSpec(VALUETYPE_FLAG, _("Print enormous amounts of information to log and console")))); + allowed_options->insert(std::make_pair("debugger", ValueSpec(VALUETYPE_FLAG, + _("Try to automatically attach a debugger before starting (convenience option)")))); allowed_options->insert(std::make_pair("logfile", ValueSpec(VALUETYPE_STRING, _("Set logfile path ('' = no logging)")))); allowed_options->insert(std::make_pair("gameid", ValueSpec(VALUETYPE_STRING, @@ -535,6 +543,126 @@ static bool create_userdata_path() return success; } +namespace { + std::string findProgram(const char *name) + { + char *path_c = getenv("PATH"); + if (!path_c) + return ""; + std::istringstream iss(path_c); + std::string checkpath; + while (!iss.eof()) { + std::getline(iss, checkpath, PATH_DELIM[0]); + if (!checkpath.empty() && checkpath.back() != DIR_DELIM_CHAR) + checkpath.push_back(DIR_DELIM_CHAR); + checkpath.append(name); + if (fs::IsExecutable(checkpath)) + return checkpath; + } + return ""; + } + +#ifdef _WIN32 + const char *debuggerNames[] = {"gdb.exe", "lldb.exe"}; +#else + const char *debuggerNames[] = {"gdb", "lldb"}; +#endif + template + void getDebuggerArgs(T &out, int i) { + if (i == 0) { + for (auto s : {"-q", "--batch", "-iex", "set confirm off", + "-ex", "run", "-ex", "bt", "--args"}) + out.push_back(s); + } else if (i == 1) { + for (auto s : {"-Q", "-b", "-o", "run", "-k", "bt\nq", "--"}) + out.push_back(s); + } + } +} + +static bool use_debugger(int argc, char *argv[]) +{ +#if defined(__ANDROID__) + return false; +#else +#ifdef _WIN32 + if (IsDebuggerPresent()) { + warningstream << "Process is already being debugged." << std::endl; + return false; + } +#endif + + char exec_path[1024]; + if (!porting::getCurrentExecPath(exec_path, sizeof(exec_path))) + return false; + + int debugger = -1; + std::string debugger_path; + for (u32 i = 0; i < ARRLEN(debuggerNames); i++) { + debugger_path = findProgram(debuggerNames[i]); + if (!debugger_path.empty()) { + debugger = i; + break; + } + } + if (debugger == -1) { + warningstream << "Couldn't find a debugger to use. Try installing gdb or lldb." << std::endl; + return false; + } + + // Try to be helpful +#ifdef NDEBUG + if (strcmp(BUILD_TYPE, "RelWithDebInfo") != 0) { + warningstream << "It looks like your " PROJECT_NAME_C " executable was built without " + "debug symbols (BUILD_TYPE=" BUILD_TYPE "), so you won't get useful backtraces." + << std::endl; + } +#endif + + std::vector new_args; + new_args.push_back(debugger_path.c_str()); + getDebuggerArgs(new_args, debugger); + // Copy the existing arguments + new_args.push_back(exec_path); + for (int i = 1; i < argc; i++) { + if (!strcmp(argv[i], "--debugger")) + continue; + new_args.push_back(argv[i]); + } + new_args.push_back(nullptr); + +#ifdef _WIN32 + // Special treatment for Windows + std::string cmdline; + for (int i = 1; new_args[i]; i++) { + if (i > 1) + cmdline += ' '; + cmdline += porting::QuoteArgv(new_args[i]); + } + + STARTUPINFO startup_info = {}; + PROCESS_INFORMATION process_info = {}; + bool ok = CreateProcess(new_args[0], cmdline.empty() ? nullptr : &cmdline[0], + nullptr, nullptr, false, CREATE_UNICODE_ENVIRONMENT, + nullptr, nullptr, &startup_info, &process_info); + if (!ok) { + warningstream << "CreateProcess: " << GetLastError() << std::endl; + return false; + } + DWORD exitcode = 0; + WaitForSingleObject(process_info.hProcess, INFINITE); + GetExitCodeProcess(process_info.hProcess, &exitcode); + exit(exitcode); + // not reached +#else + errno = 0; + execv(new_args[0], const_cast(new_args.data())); + warningstream << "execv: " << strerror(errno) << std::endl; + return false; +#endif +#endif +} + static bool init_common(const Settings &cmd_args, int argc, char *argv[]) { startup_message(); diff --git a/src/porting.cpp b/src/porting.cpp index 475292b97..629f00c37 100644 --- a/src/porting.cpp +++ b/src/porting.cpp @@ -729,6 +729,38 @@ void attachOrCreateConsole() #endif } +#ifdef _WIN32 +std::string QuoteArgv(const std::string &arg) +{ + // Quoting rules on Windows are batshit insane, can differ between applications + // and there isn't even a stdlib function to deal with it. + // Ref: https://learn.microsoft.com/archive/blogs/twistylittlepassagesallalike/everyone-quotes-command-line-arguments-the-wrong-way + if (!arg.empty() && arg.find_first_of(" \t\n\v\"") == std::string::npos) + return arg; + + std::string ret; + ret.reserve(arg.size()+2); + ret.push_back('"'); + for (auto it = arg.begin(); it != arg.end(); ++it) { + u32 back = 0; + while (it != arg.end() && *it == '\\') + ++back, ++it; + + if (it == arg.end()) { + ret.append(2 * back, '\\'); + break; + } else if (*it == '"') { + ret.append(2 * back + 1, '\\'); + } else { + ret.append(back, '\\'); + } + ret.push_back(*it); + } + ret.push_back('"'); + return ret; +} +#endif + int mt_snprintf(char *buf, const size_t buf_size, const char *fmt, ...) { // https://msdn.microsoft.com/en-us/library/bt7tawza.aspx diff --git a/src/porting.h b/src/porting.h index 93932e1d9..cc23e3c62 100644 --- a/src/porting.h +++ b/src/porting.h @@ -155,6 +155,11 @@ extern std::string path_locale; */ extern std::string path_cache; +/* + Gets the path of our executable. +*/ +bool getCurrentExecPath(char *buf, size_t len); + /* Get full path of stuff in data directory. Example: "stone.png" -> "../data/stone.png" @@ -330,6 +335,11 @@ bool secure_rand_fill_buf(void *buf, size_t len); // This attaches to the parents process console, or creates a new one if it doesnt exist. void attachOrCreateConsole(); +#ifdef _WIN32 +// Quotes an argument for use in a CreateProcess() commandline (not cmd.exe!!) +std::string QuoteArgv(const std::string &arg); +#endif + int mt_snprintf(char *buf, const size_t buf_size, const char *fmt, ...); /** diff --git a/util/test_multiplayer.sh b/util/test_multiplayer.sh index 1fcf298e8..e3fc79353 100755 --- a/util/test_multiplayer.sh +++ b/util/test_multiplayer.sh @@ -19,10 +19,6 @@ waitfor () { exit 1 } -gdbrun () { - gdb -q -batch -ex 'set confirm off' -ex 'r' -ex 'bt' --args "$@" -} - [ -e "$minetest" ] || { echo "executable $minetest missing"; exit 1; } rm -rf "$worldpath" @@ -39,11 +35,11 @@ printf '%s\n' >"$testspath/server.conf" \ ln -s "$dir/helper_mod" "$worldpath/worldmods/" echo "Starting server" -gdbrun "$minetest" --server --config "$conf_server" --world "$worldpath" --gameid $gameid 2>&1 | sed -u 's/^/(server) /' & +"$minetest" --debugger --server --config "$conf_server" --world "$worldpath" --gameid $gameid 2>&1 | sed -u 's/^/(server) /' & waitfor "$worldpath/startup" echo "Starting client" -gdbrun "$minetest" --config "$conf_client1" --go --address 127.0.0.1 2>&1 | sed -u 's/^/(client) /' & +"$minetest" --debugger --config "$conf_client1" --go --address 127.0.0.1 2>&1 | sed -u 's/^/(client) /' & waitfor "$worldpath/done" echo "Waiting for client and server to exit"