Implement --debugger option to improve UX when debugging crashes (#13157)

diff --git a/src/filesys.cpp b/src/filesys.cpp
index 7edb60bcd19a566a7da40c29b296282598d02a07..d610c2311b15a6dd29835d2e39a8e02ab259d5fc 100644 (file)
--- 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 a26fe28d6f21f41c795e3118e20a20e7c2b08dd7..fa9ddcaa8914d62e2b2882afe911f42e19a1703d 100644 (file)
--- 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 de042cf3528a1e3968cc0e7fe918525690cc53f5..f56738d9810045a7db4ad234fd661fcd5cf9c77c 100644 (file)
--- 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 = &parameters[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 35dcb708b2634ffe7ef933e61746c141aefba13f..ec805f5d39e0b8fcfd9e2681f17a75162e2841c3 100644 (file)
--- 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 <class T>
+       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<const char*> 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<char**>(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 475292b9762aeacd40a455d215d26c61561d1107..629f00c371ed8d34a208283f468402a5e4c66f47 100644 (file)
--- 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 93932e1d9ed3207a82bd680e6a7de0a32de4cc66..cc23e3c62f52e3838eb613fc94a2e8cba407a2a3 100644 (file)
--- 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 1fcf298e873ffd355b9759b690c560e5db842d39..e3fc79353957bea287cea7d9e496ba2431f0fce8 100755 (executable)
--- 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"