From 50a34d85ddfe5769c8495bd7b37554c24630ef28 Mon Sep 17 00:00:00 2001 From: GoldenTails Date: Mon, 26 Dec 2022 21:06:46 -0600 Subject: [PATCH 1/5] Clean up the backtrace code and make it use write() more safely. --- src/sdl/i_system.c | 78 +++++++++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 32 deletions(-) diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index e328bedc2..076134d0a 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -258,68 +258,82 @@ SDL_bool framebuffer = SDL_FALSE; UINT8 keyboard_started = false; #ifdef UNIXBACKTRACE -#define STDERR_WRITE(string) if (fd != -1) I_OutputMsg("%s", string) -#define CRASHLOG_WRITE(string) if (fd != -1) write(fd, string, strlen(string)) -#define CRASHLOG_STDERR_WRITE(string) \ - if (fd != -1)\ - write(fd, string, strlen(string));\ - I_OutputMsg("%s", string) + +// NOTE: if written ends up ever being -1, all further writes end up being cancelled. +// i figure an error is a reason to stop writing... +#define WRITE_FILE(string) \ + sourcelen = strlen(string); \ + while (fd != -1 && (written != -1 && errno != EINTR) && written < sourcelen) \ + written = write(fd, string, sourcelen); + +#define WRITE_STDERR(string) \ + I_OutputMsg("%s", string); + +#define WRITE_ALL(string) \ + WRITE_FILE(string); \ + WRITE_STDERR(string); static void write_backtrace(INT32 signal) { int fd = -1; - size_t size; + ssize_t written = 0; + ssize_t sourcelen = 0; time_t rawtime; struct tm timeinfo; + size_t size; enum { BT_SIZE = 1024, STR_SIZE = 32 }; - void *array[BT_SIZE]; + void *funcptrs[BT_SIZE]; char timestr[STR_SIZE]; - const char *error = "An error occurred within SRB2! Send this stack trace to someone who can help!\n"; - const char *error2 = "(Or find crash-log.txt in your SRB2 directory.)\n"; // Shown only to stderr. + const char *filename = va("%s" PATHSEP "%s", srb2home, "crash-log.txt"); - fd = open(va("%s" PATHSEP "%s", srb2home, "crash-log.txt"), O_CREAT|O_APPEND|O_RDWR, S_IRUSR|S_IWUSR); + fd = open(filename, O_CREAT|O_APPEND|O_RDWR, S_IRUSR|S_IWUSR); - if (fd == -1) - I_OutputMsg("\nWARNING: Couldn't open crash log for writing! Make sure your permissions are correct. Please save the below report!\n"); + if (fd == -1) // File handle error + WRITE_STDERR("\nWARNING: Couldn't open crash log for writing! Make sure your permissions are correct. Please save the below report!\n"); // Get the current time as a string. time(&rawtime); localtime_r(&rawtime, &timeinfo); strftime(timestr, STR_SIZE, "%a, %d %b %Y %T %z", &timeinfo); - CRASHLOG_WRITE("------------------------\n"); // Nice looking seperator + WRITE_FILE("------------------------\n"); // Nice looking seperator - CRASHLOG_STDERR_WRITE("\n"); // Newline to look nice for both outputs. - CRASHLOG_STDERR_WRITE(error); // "Oops, SRB2 crashed" message - STDERR_WRITE(error2); // Tell the user where the crash log is. + WRITE_ALL("\n"); // Newline to look nice for both outputs. + WRITE_ALL("An error occurred within SRB2! Send this stack trace to someone who can help!\n"); + + if (fd != -1) // If the crash log exists, + WRITE_STDERR("(Or find crash-log.txt in your SRB2 directory.)\n"); // tell the user where the crash log is. // Tell the log when we crashed. - CRASHLOG_WRITE("Time of crash: "); - CRASHLOG_WRITE(timestr); - CRASHLOG_WRITE("\n"); + WRITE_FILE("Time of crash: "); + WRITE_FILE(timestr); + WRITE_FILE("\n"); // Give the crash log the cause and a nice 'Backtrace:' thing // The signal is given to the user when the parent process sees we crashed. - CRASHLOG_WRITE("Cause: "); - CRASHLOG_WRITE(strsignal(signal)); - CRASHLOG_WRITE("\n"); // Newline for the signal name + WRITE_FILE("Cause: "); + WRITE_FILE(strsignal(signal)); + WRITE_FILE("\n"); // Newline for the signal name - CRASHLOG_STDERR_WRITE("\nBacktrace:\n"); + WRITE_ALL("\nBacktrace:\n"); // Flood the output and log with the backtrace - size = backtrace(array, BT_SIZE); - backtrace_symbols_fd(array, size, fd); - backtrace_symbols_fd(array, size, STDERR_FILENO); + size = backtrace(funcptrs, BT_SIZE); + backtrace_symbols_fd(funcptrs, size, fd); + backtrace_symbols_fd(funcptrs, size, STDERR_FILENO); - CRASHLOG_WRITE("\n"); // Write another newline to the log so it looks nice :) + WRITE_FILE("\n"); // Write another newline to the log so it looks nice :) - close(fd); + if (fd != -1) { + fsync(fd); // reaaaaally make sure we got that data written. + close(fd); + } } -#undef STDERR_WRITE -#undef CRASHLOG_WRITE -#undef CRASHLOG_STDERR_WRITE +#undef WRITE_FILE +#undef WRITE_STDERR +#undef WRITE_ALL #endif // UNIXBACKTRACE static void I_ReportSignal(int num, int coredumped) From c5e69fcf94c8e3296b567a56efb9aca66352564d Mon Sep 17 00:00:00 2001 From: GoldenTails Date: Thu, 13 Apr 2023 01:19:12 -0500 Subject: [PATCH 2/5] Replace macros with static functions, rename size int to bt_size. --- src/sdl/i_system.c | 63 +++++++++++++++++++++++----------------------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index 076134d0a..d4055ba26 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -259,28 +259,29 @@ UINT8 keyboard_started = false; #ifdef UNIXBACKTRACE -// NOTE: if written ends up ever being -1, all further writes end up being cancelled. -// i figure an error is a reason to stop writing... -#define WRITE_FILE(string) \ - sourcelen = strlen(string); \ - while (fd != -1 && (written != -1 && errno != EINTR) && written < sourcelen) \ +static void bt_write_file(int fd, const char *string) { + ssize_t written = 0; + ssize_t sourcelen = strlen(string); + + while (fd != -1 && (written != -1 && errno != EINTR) && written < sourcelen) written = write(fd, string, sourcelen); +} -#define WRITE_STDERR(string) \ - I_OutputMsg("%s", string); +static void bt_write_stderr(const char *string) { + bt_write_file(STDERR_FILENO, string); +} -#define WRITE_ALL(string) \ - WRITE_FILE(string); \ - WRITE_STDERR(string); +static void bt_write_all(int fd, const char *string) { + bt_write_file(fd, string); + bt_write_file(STDERR_FILENO, string); +} static void write_backtrace(INT32 signal) { int fd = -1; - ssize_t written = 0; - ssize_t sourcelen = 0; time_t rawtime; struct tm timeinfo; - size_t size; + size_t bt_size; enum { BT_SIZE = 1024, STR_SIZE = 32 }; void *funcptrs[BT_SIZE]; @@ -291,49 +292,47 @@ static void write_backtrace(INT32 signal) fd = open(filename, O_CREAT|O_APPEND|O_RDWR, S_IRUSR|S_IWUSR); if (fd == -1) // File handle error - WRITE_STDERR("\nWARNING: Couldn't open crash log for writing! Make sure your permissions are correct. Please save the below report!\n"); + bt_write_stderr("\nWARNING: Couldn't open crash log for writing! Make sure your permissions are correct. Please save the below report!\n"); // Get the current time as a string. time(&rawtime); localtime_r(&rawtime, &timeinfo); strftime(timestr, STR_SIZE, "%a, %d %b %Y %T %z", &timeinfo); - WRITE_FILE("------------------------\n"); // Nice looking seperator + bt_write_file(fd, "------------------------\n"); // Nice looking seperator - WRITE_ALL("\n"); // Newline to look nice for both outputs. - WRITE_ALL("An error occurred within SRB2! Send this stack trace to someone who can help!\n"); + bt_write_all(fd, "\n"); // Newline to look nice for both outputs. + bt_write_all(fd, "An error occurred within SRB2! Send this stack trace to someone who can help!\n"); if (fd != -1) // If the crash log exists, - WRITE_STDERR("(Or find crash-log.txt in your SRB2 directory.)\n"); // tell the user where the crash log is. + bt_write_stderr("(Or find crash-log.txt in your SRB2 directory.)\n"); // tell the user where the crash log is. // Tell the log when we crashed. - WRITE_FILE("Time of crash: "); - WRITE_FILE(timestr); - WRITE_FILE("\n"); + bt_write_file(fd, "Time of crash: "); + bt_write_file(fd, timestr); + bt_write_file(fd, "\n"); // Give the crash log the cause and a nice 'Backtrace:' thing // The signal is given to the user when the parent process sees we crashed. - WRITE_FILE("Cause: "); - WRITE_FILE(strsignal(signal)); - WRITE_FILE("\n"); // Newline for the signal name + bt_write_file(fd, "Cause: "); + bt_write_file(fd, strsignal(signal)); + bt_write_file(fd, "\n"); // Newline for the signal name - WRITE_ALL("\nBacktrace:\n"); + bt_write_all(fd, "\nBacktrace:\n"); // Flood the output and log with the backtrace - size = backtrace(funcptrs, BT_SIZE); - backtrace_symbols_fd(funcptrs, size, fd); - backtrace_symbols_fd(funcptrs, size, STDERR_FILENO); + bt_size = backtrace(funcptrs, BT_SIZE); + backtrace_symbols_fd(funcptrs, bt_size, fd); + backtrace_symbols_fd(funcptrs, bt_size, STDERR_FILENO); - WRITE_FILE("\n"); // Write another newline to the log so it looks nice :) + bt_write_file(fd, "\n"); // Write another newline to the log so it looks nice :) if (fd != -1) { fsync(fd); // reaaaaally make sure we got that data written. close(fd); } } -#undef WRITE_FILE -#undef WRITE_STDERR -#undef WRITE_ALL + #endif // UNIXBACKTRACE static void I_ReportSignal(int num, int coredumped) From 5e2311c48db8c409e2571e8377e2f178fa4f7726 Mon Sep 17 00:00:00 2001 From: Logan Aerl Arias Date: Tue, 2 Jan 2024 22:03:51 -0500 Subject: [PATCH 3/5] Update i_system.c removed CRASHLOG_STDERR_WRITE --- src/sdl/i_system.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index 2307961d2..ca68d031a 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -339,14 +339,6 @@ static void write_backtrace(INT32 signal) bt_size = backtrace(funcptrs, BT_SIZE); backtrace_symbols_fd(funcptrs, bt_size, fd); backtrace_symbols_fd(funcptrs, bt_size, STDERR_FILENO); -#ifndef NOEXECINFO - CRASHLOG_STDERR_WRITE("\nBacktrace:\n"); - - // Flood the output and log with the backtrace - size = backtrace(array, BT_SIZE); - backtrace_symbols_fd(array, size, fd); - backtrace_symbols_fd(array, size, STDERR_FILENO); -#endif bt_write_file(fd, "\n"); // Write another newline to the log so it looks nice :) if (fd != -1) { From eae89efbb92574b325710c9a9f61cee313203f95 Mon Sep 17 00:00:00 2001 From: Logan Aerl Arias Date: Tue, 2 Jan 2024 22:06:00 -0500 Subject: [PATCH 4/5] Update i_system.c remove unused size_t size remove unused void *array[BT_SIZE]; --- src/sdl/i_system.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index ca68d031a..6e6031f4e 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -288,18 +288,12 @@ static void bt_write_all(int fd, const char *string) { static void write_backtrace(INT32 signal) { int fd = -1; -#ifndef NOEXECINFO - size_t size; -#endif time_t rawtime; struct tm timeinfo; size_t bt_size; enum { BT_SIZE = 1024, STR_SIZE = 32 }; void *funcptrs[BT_SIZE]; -#ifndef NOEXECINFO - void *array[BT_SIZE]; -#endif char timestr[STR_SIZE]; const char *filename = va("%s" PATHSEP "%s", srb2home, "crash-log.txt"); From 4fddc8fec71d9c6fd4915895b00fb6b20f6d4622 Mon Sep 17 00:00:00 2001 From: Logan Aerl Arias Date: Tue, 2 Jan 2024 22:23:52 -0500 Subject: [PATCH 5/5] Update i_system.c backtrace() doesn't exist in non-glibc systems --- src/sdl/i_system.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index 6e6031f4e..bc21134da 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -290,10 +290,12 @@ static void write_backtrace(INT32 signal) int fd = -1; time_t rawtime; struct tm timeinfo; - size_t bt_size; enum { BT_SIZE = 1024, STR_SIZE = 32 }; +#ifndef NOEXECINFO void *funcptrs[BT_SIZE]; + size_t bt_size; +#endif char timestr[STR_SIZE]; const char *filename = va("%s" PATHSEP "%s", srb2home, "crash-log.txt"); @@ -327,12 +329,16 @@ static void write_backtrace(INT32 signal) bt_write_file(fd, strsignal(signal)); bt_write_file(fd, "\n"); // Newline for the signal name +#ifdef NOEXECINFO + bt_write_all(fd, "\nNo Backtrace support\n"); +#else bt_write_all(fd, "\nBacktrace:\n"); // Flood the output and log with the backtrace bt_size = backtrace(funcptrs, BT_SIZE); backtrace_symbols_fd(funcptrs, bt_size, fd); backtrace_symbols_fd(funcptrs, bt_size, STDERR_FILENO); +#endif bt_write_file(fd, "\n"); // Write another newline to the log so it looks nice :) if (fd != -1) {