From d679e393c3f345efd181d4fc153f2f494274ed57 Mon Sep 17 00:00:00 2001 From: Daniel Gibson Date: Mon, 10 Jan 2022 04:06:54 +0100 Subject: [PATCH 1/8] Add D3_(v)snprintfC99() for C99-compatible implementations These are now used by idStr::(v)snPrintf(), and in the future can be used if a (v)snprintf() that's guaranteed not to call common->Warning() or similar is needed (e.g. used during early startup) --- neo/idlib/Str.cpp | 84 +++++++++++++++++++++++++++++++++++++---------- neo/idlib/Str.h | 11 +++++++ 2 files changed, 77 insertions(+), 18 deletions(-) diff --git a/neo/idlib/Str.cpp b/neo/idlib/Str.cpp index 9bd6b1b9..e7302333 100644 --- a/neo/idlib/Str.cpp +++ b/neo/idlib/Str.cpp @@ -30,6 +30,7 @@ If you have questions concerning this license or the applicable additional terms #include "idlib/math/Vector.h" #include "idlib/Heap.h" #include "framework/Common.h" +#include #include "idlib/Str.h" @@ -1513,21 +1514,25 @@ idStr::snPrintf ================ */ int idStr::snPrintf( char *dest, int size, const char *fmt, ...) { - int len; va_list argptr; - char buffer[32000]; // big, but small enough to fit in PPC stack - + int len; va_start( argptr, fmt ); - len = vsprintf( buffer, fmt, argptr ); + len = D3_vsnprintfC99(dest, size, fmt, argptr); va_end( argptr ); - if ( len >= sizeof( buffer ) ) { + if ( len >= 32000 ) { + // TODO: Previously this function used a 32000 byte buffer to write into + // with vsprintf(), and raised this error if that was overflowed + // (more likely that'd have lead to a crash..). + // Technically we don't have that restriction anymore, so I'm unsure + // if this error should really still be raised to preserve + // the old intended behavior, maybe for compat with mod DLLs using + // the old version of the function or something? idLib::common->Error( "idStr::snPrintf: overflowed buffer" ); } if ( len >= size ) { idLib::common->Warning( "idStr::snPrintf: overflow of %i in %i\n", len, size ); len = size; } - idStr::Copynz( dest, buffer, size ); return len; } @@ -1550,18 +1555,7 @@ or returns -1 on failure or if the buffer would be overflowed. ============ */ int idStr::vsnPrintf( char *dest, int size, const char *fmt, va_list argptr ) { - int ret; - -#ifdef _WIN32 -#undef _vsnprintf - ret = _vsnprintf( dest, size-1, fmt, argptr ); -#define _vsnprintf use_idStr_vsnPrintf -#else -#undef vsnprintf - ret = vsnprintf( dest, size, fmt, argptr ); -#define vsnprintf use_idStr_vsnPrintf -#endif - dest[size-1] = '\0'; + int ret = D3_vsnprintfC99(dest, size, fmt, argptr); if ( ret < 0 || ret >= size ) { return -1; } @@ -1790,3 +1784,57 @@ idStr idStr::FormatNumber( int number ) { return string; } + +// behaves like C99's vsnprintf() by returning the amount of bytes that +// *would* have been written into a big enough buffer, even if that's > size +// unlike idStr::vsnPrintf() which returns -1 in that case +int D3_vsnprintfC99(char *dst, size_t size, const char *format, va_list ap) +{ + // before VS2015, it didn't have a standards-conforming (v)snprintf()-implementation + // same might be true for other windows compilers if they use old CRT versions, like MinGW does +#if defined(_WIN32) && (!defined(_MSC_VER) || _MSC_VER < 1900) + #undef _vsnprintf + // based on DG_vsnprintf() from https://github.com/DanielGibson/Snippets/blob/master/DG_misc.h + int ret = -1; + if(dst != NULL && size > 0) + { +#if defined(_MSC_VER) && _MSC_VER >= 1400 + // I think MSVC2005 introduced _vsnprintf_s(). + // this shuts up _vsnprintf() security/deprecation warnings. + ret = _vsnprintf_s(dst, size, _TRUNCATE, format, ap); +#else + ret = _vsnprintf(dst, size, format, ap); + dst[size-1] = '\0'; // ensure '\0'-termination +#endif + } + + if(ret == -1) + { + // _vsnprintf() returns -1 if the output is truncated + // it's also -1 if dst or size were NULL/0, so the user didn't want to write + // we want to return the number of characters that would've been + // needed, though.. fortunately _vscprintf() calculates that. + ret = _vscprintf(format, ap); + } + return ret; + #define _vsnprintf use_idStr_vsnPrintf +#else // other operating systems and VisualC++ >= 2015 should have a proper vsnprintf() + #undef vsnprintf + return vsnprintf(dst, size, format, ap); + #define vsnprintf use_idStr_vsnPrintf +#endif +} + +// behaves like C99's snprintf() by returning the amount of bytes that +// *would* have been written into a big enough buffer, even if that's > size +// unlike idStr::snPrintf() which returns the written bytes in that case +// and also calls common->Warning() in case of overflows +int D3_snprintfC99(char *dst, size_t size, const char *format, ...) +{ + int ret = 0; + va_list argptr; + va_start( argptr, format ); + ret = D3_vsnprintfC99(dst, size, format, argptr); + va_end( argptr ); + return ret; +} diff --git a/neo/idlib/Str.h b/neo/idlib/Str.h index 5dfabe9b..a44bab2b 100644 --- a/neo/idlib/Str.h +++ b/neo/idlib/Str.h @@ -1068,4 +1068,15 @@ ID_INLINE int idStr::DynamicMemoryUsed() const { return ( data == baseBuffer ) ? 0 : alloced; } +// behaves like C99's snprintf() by returning the amount of bytes that +// *would* have been written into a big enough buffer, even if that's > size +// unlike idStr::snPrintf() which returns the written bytes in that case +// and also calls common->Warning() in case of overflows +int D3_snprintfC99(char *dst, size_t size, const char *format, ...) id_attribute((format(printf,3,4))); + +// behaves like C99's vsnprintf() by returning the amount of bytes that +// *would* have been written into a big enough buffer, even if that's > size +// unlike idStr::vsnPrintf() which returns -1 in that case +int D3_vsnprintfC99(char *dst, size_t size, const char *format, va_list ap); + #endif /* !__STR_H__ */ From 4f74c15afe53aa3cd7bf88f7be07179f5391cafe Mon Sep 17 00:00:00 2001 From: Daniel Gibson Date: Sun, 16 Jan 2022 06:02:40 +0100 Subject: [PATCH 2/8] Make sure MAX_OSPATH has sane size; fix some typos --- neo/framework/Common.cpp | 2 +- neo/framework/FileSystem.h | 5 ++++- neo/sys/platform.h | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/neo/framework/Common.cpp b/neo/framework/Common.cpp index 25637641..ed519650 100644 --- a/neo/framework/Common.cpp +++ b/neo/framework/Common.cpp @@ -2592,7 +2592,7 @@ void idCommonLocal::Async( void ) { ================= idCommonLocal::LoadGameDLLbyName -Helper for LoadGameDLL() to make it less painfull to try different dll names. +Helper for LoadGameDLL() to make it less painful to try different dll names. ================= */ void idCommonLocal::LoadGameDLLbyName( const char *dll, idStr& s ) { diff --git a/neo/framework/FileSystem.h b/neo/framework/FileSystem.h index ad1a8de2..6857fb89 100644 --- a/neo/framework/FileSystem.h +++ b/neo/framework/FileSystem.h @@ -61,7 +61,10 @@ If you have questions concerning this license or the applicable additional terms // => change it (to -1?) or does that break anything? static const ID_TIME_T FILE_NOT_FOUND_TIMESTAMP = 0xFFFFFFFF; static const int MAX_PURE_PAKS = 128; -static const int MAX_OSPATH = FILENAME_MAX; +// DG: https://www.gnu.org/software/libc/manual/html_node/Limits-for-Files.html says +// that FILENAME_MAX can be *really* big on some systems and thus is not suitable +// for buffer lengths. So limit it to prevent stack overflow/out of memory issues +static const int MAX_OSPATH = (FILENAME_MAX < 32000) ? FILENAME_MAX : 32000; // modes for OpenFileByMode. used as bit mask internally typedef enum { diff --git a/neo/sys/platform.h b/neo/sys/platform.h index ac27a08e..e4ae34ce 100644 --- a/neo/sys/platform.h +++ b/neo/sys/platform.h @@ -40,7 +40,7 @@ If you have questions concerning this license or the applicable additional terms =============================================================================== */ -// Win32 +// AROS #if defined(__AROS__) #define _alloca alloca From 952292b4a6a39e64164cd49760ce2c6242e6095c Mon Sep 17 00:00:00 2001 From: Daniel Gibson Date: Sun, 16 Jan 2022 06:11:20 +0100 Subject: [PATCH 3/8] POSIX: log output to save_path/dhewm3log.txt and refactorings needed for that (I want to create the log right at the beginning before much else has been initialized, so using things like idStr or Sys_GetPath() isn't safe) save_path being $XDG_DATA_HOME/dhewm3/ (usually ~/.local/share/dhewm3/) on most POSIX systems, $HOME/Library/Application Support/dhewm3/ on Mac If the log file already exists, it's renamed to dhewm3log-old.txt first, so there's always the most recent and the last log available. --- neo/sys/linux/main.cpp | 30 +++++-- neo/sys/osx/DOOMController.mm | 32 ++++--- neo/sys/posix/posix_main.cpp | 160 +++++++++++++++++++++++++++++----- neo/sys/posix/posix_public.h | 3 +- 4 files changed, 183 insertions(+), 42 deletions(-) diff --git a/neo/sys/linux/main.cpp b/neo/sys/linux/main.cpp index 0f1717d7..28a8a9b5 100644 --- a/neo/sys/linux/main.cpp +++ b/neo/sys/linux/main.cpp @@ -78,6 +78,21 @@ If you have questions concerning this license or the applicable additional terms static char path_argv[PATH_MAX]; static char path_exe[PATH_MAX]; +static char save_path[PATH_MAX]; + +const char* Posix_GetSavePath() +{ + return save_path; +} + +static void SetSavePath() +{ + const char* s = getenv("XDG_DATA_HOME"); + if (s) + D3_snprintfC99(save_path, sizeof(save_path), "%s/dhewm3", s); + else + D3_snprintfC99(save_path, sizeof(save_path), "%s/.local/share/dhewm3", getenv("HOME")); +} const char* Posix_GetExePath() { @@ -225,14 +240,11 @@ bool Sys_GetPath(sysPath_t type, idStr &path) { return true; case PATH_SAVE: - s = getenv("XDG_DATA_HOME"); - if (s) - idStr::snPrintf(buf, sizeof(buf), "%s/dhewm3", s); - else - idStr::snPrintf(buf, sizeof(buf), "%s/.local/share/dhewm3", getenv("HOME")); - - path = buf; - return true; + if(save_path[0] != '\0') { + path = save_path; + return true; + } + return false; case PATH_EXE: if (path_exe[0] != '\0') { @@ -419,6 +431,8 @@ int main(int argc, char **argv) { memcpy(path_exe, path_argv, sizeof(path_exe)); } + SetSavePath(); + // some ladspa-plugins (that may be indirectly loaded by doom3 if they're // used by alsa) call setlocale(LC_ALL, ""); This sets LC_ALL to $LANG or // $LC_ALL which usually is not "C" and will fuck up scanf, strtod diff --git a/neo/sys/osx/DOOMController.mm b/neo/sys/osx/DOOMController.mm index 8b25bdb2..8360de74 100644 --- a/neo/sys/osx/DOOMController.mm +++ b/neo/sys/osx/DOOMController.mm @@ -45,30 +45,28 @@ If you have questions concerning this license or the applicable additional terms #include "sys/posix/posix_public.h" +static char base_path[MAXPATHLEN]; static char exe_path[MAXPATHLEN]; +static char save_path[MAXPATHLEN]; + const char* Posix_GetExePath() { return exe_path; } -bool Sys_GetPath(sysPath_t type, idStr &path) { - char buf[MAXPATHLEN]; - char *snap; +const char* Posix_GetSavePath() { + return save_path; +} +bool Sys_GetPath(sysPath_t type, idStr &path) { switch(type) { case PATH_BASE: - SDL_strlcpy(buf, [ [ [ NSBundle mainBundle ] bundlePath ] cString ], MAXPATHLEN ); - snap = strrchr(buf, '/'); - if (snap) - *snap = '\0'; - - path = buf; + path = base_path; return true; case PATH_CONFIG: case PATH_SAVE: - sprintf(buf, "%s/Library/Application Support/dhewm3", [NSHomeDirectory() cString]); - path = buf; + path = save_path; return true; case PATH_EXE: @@ -196,7 +194,17 @@ int SDL_main( int argc, char *argv[] ) { Sys_Error("Could not access application resources"); // DG: set exe_path so Posix_InitSignalHandlers() can call Posix_GetExePath() - SDL_strlcpy(exe_path, [ [ [ NSBundle mainBundle ] bundlePath ] cString ], MAXPATHLEN); + SDL_strlcpy(exe_path, [ [ [ NSBundle mainBundle ] bundlePath ] cString ], sizeof(exe_path)); + // same for save_path for Posix_GetSavePath() + snprintf(save_path, sizeof(save_path), "%s/Library/Application Support/dhewm3", [NSHomeDirectory() cString]); + // and preinitializing basepath is easy enough so do that as well + { + char* snap; + SDL_strlcpy(base_path, exe_path, sizeof(base_path)); + snap = strrchr(base_path, '/'); + if (snap) + *snap = '\0'; + } Posix_InitSignalHandlers(); // DG: added signal handlers for POSIX platforms diff --git a/neo/sys/posix/posix_main.cpp b/neo/sys/posix/posix_main.cpp index 2027a6dc..ee975d32 100644 --- a/neo/sys/posix/posix_main.cpp +++ b/neo/sys/posix/posix_main.cpp @@ -78,6 +78,15 @@ idCVar com_pid( "com_pid", "0", CVAR_INTEGER | CVAR_INIT | CVAR_SYSTEM, "process static int set_exit = 0; static char exit_spawn[ 1024 ] = { 0 }; +static FILE* consoleLog = NULL; +void Sys_VPrintf(const char *msg, va_list arg); + +#ifdef snprintf + // I actually wanna use real snprintf here, not idStr:snPrintf(), + // so get rid of the use_idStr_snPrintf #define + #undef snprintf +#endif + /* ================ Posix_Exit @@ -95,6 +104,12 @@ void Posix_Exit(int ret) { if ( exit_spawn[0] ) { Sys_DoStartProcess( exit_spawn, false ); } + + if(consoleLog != NULL) { + fclose(consoleLog); + consoleLog = NULL; + } + // in case of signal, handler tries a common->Quit // we use set_exit to maintain a correct exit code if ( set_exit ) { @@ -246,6 +261,9 @@ Sys_Init ================= */ void Sys_Init( void ) { + if(consoleLog != NULL) + common->Printf("Logging console output to %s/dhewm3log.txt\n", Posix_GetSavePath()); + Posix_InitConsoleInput(); com_pid.SetInteger( getpid() ); common->Printf( "pid: %d\n", com_pid.GetInteger() ); @@ -406,6 +424,17 @@ static const char* crashSigNames[] = { "SIGILL", "SIGABRT", "SIGFPE", "SIGSEGV" #include #endif +// unlike Sys_Printf() this doesn't call tty_Hide(); and tty_Show(); +// to minimize interaction with broken dhewm3 state +// (but unlike regular printf() it'll also write to dhewm3log.txt) +static void CrashPrintf(const char* msg, ...) +{ + va_list argptr; + va_start( argptr, msg ); + Sys_VPrintf( msg, argptr ); + va_end( argptr ); +} + #ifdef D3_HAVE_LIBBACKTRACE // non-ancient versions of GCC and clang include libbacktrace // for ancient versions it can be built from https://github.com/ianlancetaylor/libbacktrace @@ -416,7 +445,7 @@ static struct backtrace_state *bt_state = NULL; static void bt_error_callback( void *data, const char *msg, int errnum ) { - printf("libbacktrace ERROR: %d - %s\n", errnum, msg); + CrashPrintf("libbacktrace ERROR: %d - %s\n", errnum, msg); } static void bt_syminfo_callback( void *data, uintptr_t pc, const char *symname, @@ -429,10 +458,10 @@ static void bt_syminfo_callback( void *data, uintptr_t pc, const char *symname, if (name != NULL) { symname = name; } - printf(" %zu %s\n", pc, symname); + CrashPrintf(" %zu %s\n", pc, symname); free(name); } else { - printf(" %zu (unknown symbol)\n", pc); + CrashPrintf(" %zu (unknown symbol)\n", pc); } } @@ -455,7 +484,7 @@ static int bt_pcinfo_callback( void *data, uintptr_t pc, const char *filename, i if (fileNameNeo != NULL) { filename = fileNameNeo+1; // I want "neo/bla/blub.cpp:42" } - printf(" %zu %s:%d %s\n", pc, filename, lineno, function); + CrashPrintf(" %zu %s:%d %s\n", pc, filename, lineno, function); free(name); } @@ -464,7 +493,7 @@ static int bt_pcinfo_callback( void *data, uintptr_t pc, const char *filename, i static void bt_error_dummy( void *data, const char *msg, int errnum ) { - //printf("ERROR-DUMMY: %d - %s\n", errnum, msg); + //CrashPrintf("ERROR-DUMMY: %d - %s\n", errnum, msg); } static int bt_simple_callback(void *data, uintptr_t pc) @@ -495,14 +524,14 @@ static void signalhandlerCrash(int sig) // TODO: should probably use a custom print function around write(STDERR_FILENO, ...) // because printf() could allocate which is not good if processes state is fscked // (could use backtrace_symbols_fd() then) - printf("Looks like %s crashed with signal %s (%d) - sorry!\n", ENGINE_VERSION, name, sig); + CrashPrintf("\n\nLooks like %s crashed with signal %s (%d) - sorry!\n", ENGINE_VERSION, name, sig); #ifdef D3_HAVE_LIBBACKTRACE if (bt_state != NULL) { int skip = 1; // skip this function in backtrace backtrace_simple(bt_state, skip, bt_simple_callback, bt_error_callback, NULL); } else { - printf("(No backtrace because libbacktrace state is NULL)\n"); + CrashPrintf("(No backtrace because libbacktrace state is NULL)\n"); } #elif defined(D3_HAVE_BACKTRACE) // this is partly based on Yamagi Quake II code @@ -510,21 +539,26 @@ static void signalhandlerCrash(int sig) int size = backtrace(array, sizeof(array)/sizeof(array[0])); char** strings = backtrace_symbols(array, size); - printf("\nBacktrace:\n"); + CrashPrintf("\nBacktrace:\n"); for(int i = 0; i < size; i++) { - printf(" %s\n", strings[i]); + CrashPrintf(" %s\n", strings[i]); } - printf("\n(Sorry it's not overly useful, build with libbacktrace support to get function names)\n"); + CrashPrintf("\n(Sorry it's not overly useful, build with libbacktrace support to get function names)\n"); free(strings); #else - printf("(No Backtrace on this platform)\n"); + CrashPrintf("(No Backtrace on this platform)\n"); #endif fflush(stdout); + if(consoleLog != NULL) { + fflush(consoleLog); + // TODO: fclose(consoleLog); ? + // consoleLog = NULL; + } raise(sig); // pass it on to system } @@ -565,6 +599,33 @@ static void installSigHandler(int sig, int flags, void (*handler)(int)) sigaction(sig, &sigact, NULL); } +static bool dirExists(const char* dirPath) +{ + struct stat buf = {}; + if(stat(dirPath, &buf) == 0) { + return (buf.st_mode & S_IFMT) == S_IFDIR; + } + return false; +} + +static bool createPathRecursive(char* path) +{ + if(!dirExists(path)) { + char* lastDirSep = strrchr(path, '/'); + if(lastDirSep != NULL) { + *lastDirSep = '\0'; // cut off last part of the path and try first with parent directory + bool ok = createPathRecursive(path); + *lastDirSep = '/'; // restore path + // if parent dir was successfully created (or already existed), create this dir + if(ok && mkdir(path, 0755) == 0) { + return true; + } + } + return false; + } + return true; +} + void Posix_InitSignalHandlers( void ) { #ifdef D3_HAVE_LIBBACKTRACE @@ -580,6 +641,53 @@ void Posix_InitSignalHandlers( void ) installSigHandler(SIGTTIN, 0, signalhandlerConsoleStuff); installSigHandler(SIGTTOU, 0, signalhandlerConsoleStuff); + + // this is also a good place to open dhewm3log.txt for Sys_VPrintf() + + const char* savePath = Posix_GetSavePath(); + size_t savePathLen = strlen(savePath); + if(savePathLen > 0 && savePathLen < PATH_MAX) { + char logPath[PATH_MAX] = {}; + if(savePath[savePathLen-1] == '/') { + --savePathLen; + } + memcpy(logPath, savePath, savePathLen); + logPath[savePathLen] = '\0'; + if(!createPathRecursive(logPath)) { + printf("WARNING: Couldn't create save path '%s'!\n", logPath); + return; + } + char logFileName[PATH_MAX] = {}; + int fullLogLen = snprintf(logFileName, sizeof(logFileName), "%s/dhewm3log.txt", logPath); + // cast to size_t which is unsigned and would get really big if fullLogLen < 0 (=> error in snprintf()) + if((size_t)fullLogLen >= sizeof(logFileName)) { + printf("WARNING: Couldn't create dhewm3log.txt at '%s' because its length would be '%d' which is > PATH_MAX (%zd) or < 0!\n", + logPath, fullLogLen, (size_t)PATH_MAX); + return; + } + struct stat buf; + if(stat(logFileName, &buf) == 0) { + // logfile exists, rename to dhewm3log-old.txt + char oldLogFileName[PATH_MAX] = {}; + if((size_t)snprintf(oldLogFileName, sizeof(oldLogFileName), "%s/dhewm3log-old.txt", logPath) < sizeof(logFileName)) + { + rename(logFileName, oldLogFileName); + } + } + consoleLog = fopen(logFileName, "w"); + if(consoleLog == NULL) { + printf("WARNING: Couldn't open/create '%s', error was: %d (%s)\n", logFileName, errno, strerror(errno)); + } else { + time_t tt = time(NULL); + const struct tm* tms = localtime(&tt); + char timeStr[64] = {}; + strftime(timeStr, sizeof(timeStr), "%F %H:%M:%S", tms); + fprintf(consoleLog, "Opened this log at %s\n", timeStr); + } + + } else { + printf("WARNING: Posix_GetSavePath() returned path with invalid length '%zd'!\n", savePathLen); + } } // ----------- signal handling stuff done ------------ @@ -1005,38 +1113,48 @@ low level output =============== */ +void Sys_VPrintf( const char *msg, va_list arg ) { + // gonna use arg twice, so copy it + va_list arg2; + va_copy(arg2, arg); + + // first print to stdout() + vprintf(msg, arg2); + + va_end(arg2); // arg2 is not needed anymore + + // then print to the log, if any + if(consoleLog != NULL) + { + vfprintf(consoleLog, msg, arg); + } +} + void Sys_DebugPrintf( const char *fmt, ... ) { va_list argptr; tty_Hide(); va_start( argptr, fmt ); - vprintf( fmt, argptr ); + Sys_VPrintf( fmt, argptr ); va_end( argptr ); tty_Show(); } void Sys_DebugVPrintf( const char *fmt, va_list arg ) { tty_Hide(); - vprintf( fmt, arg ); + Sys_VPrintf( fmt, arg ); tty_Show(); } void Sys_Printf(const char *msg, ...) { va_list argptr; - tty_Hide(); va_start( argptr, msg ); - vprintf( msg, argptr ); + Sys_VPrintf( msg, argptr ); va_end( argptr ); tty_Show(); } -void Sys_VPrintf(const char *msg, va_list arg) { - tty_Hide(); - vprintf(msg, arg); - tty_Show(); -} - /* ================ Sys_Error diff --git a/neo/sys/posix/posix_public.h b/neo/sys/posix/posix_public.h index e97d01b2..0a33f89f 100644 --- a/neo/sys/posix/posix_public.h +++ b/neo/sys/posix/posix_public.h @@ -36,12 +36,13 @@ If you have questions concerning this license or the applicable additional terms const char* Posix_Cwd( void ); const char* Posix_GetExePath(); +const char* Posix_GetSavePath(); void Posix_Exit( int ret ); void Posix_SetExit(int ret); // override the exit code void Posix_SetExitSpawn( const char *exeName ); // set the process to be spawned when we quit -void Posix_InitSignalHandlers( void ); +void Posix_InitSignalHandlers( void ); // also opens/creates dhewm3.log void Posix_InitConsoleInput( void ); void Posix_Shutdown( void ); From 5438c9409f6edad67a56e32b4c59fe6cbb60586c Mon Sep 17 00:00:00 2001 From: Daniel Gibson Date: Sun, 16 Jan 2022 06:12:51 +0100 Subject: [PATCH 4/8] dhewm3log.txt for AROS basically the same as for POSIX, except I don't know where the save dir is. I hope this works, can't test it myself.. --- neo/sys/aros/aros_dos.cpp | 35 +++++++++++++------ neo/sys/aros/aros_main.cpp | 69 +++++++++++++++++++++++++++++++++----- 2 files changed, 84 insertions(+), 20 deletions(-) diff --git a/neo/sys/aros/aros_dos.cpp b/neo/sys/aros/aros_dos.cpp index 679e70b2..bcdd8338 100644 --- a/neo/sys/aros/aros_dos.cpp +++ b/neo/sys/aros/aros_dos.cpp @@ -475,10 +475,27 @@ void AROS_OpenURL( const char *url ) { URL_OpenA( (char *)url, tags ); } +bool AROS_GetSavePath(char buf[1024]) +{ + static const size_t bufSize = 1024; // NOTE: keep in sync with caller/function sig! + BPTR pathlock; + bool ret = false; + if ((pathlock = Lock("PROGDIR:", SHARED_LOCK)) != BNULL) + { + if ( NameFromLock( pathlock, buf, bufSize ) ) + { + D(bug("[ADoom3] Sys_GetPath: using '%s'\n", buf)); + ret = true; + } + UnLock(pathlock); + } + return ret; +} bool Sys_GetPath(sysPath_t type, idStr &path) { char buf[1024]; BPTR pathlock; + bool ret = false; D(bug("[ADoom3] Sys_GetPath(%d)\n", type)); @@ -488,16 +505,11 @@ bool Sys_GetPath(sysPath_t type, idStr &path) { case PATH_BASE: case PATH_CONFIG: case PATH_SAVE: - if ((pathlock = Lock("PROGDIR:", SHARED_LOCK)) != BNULL) - { - if ( NameFromLock( pathlock, buf, sizeof( buf ) ) ) - { - D(bug("[ADoom3] Sys_GetPath: using '%s'\n", buf)); - path = buf; - } - UnLock(pathlock); + if(AROS_GetSavePath(buf)) { + path = buf; + ret = true; } - return true; + break; case PATH_EXE: if ((pathlock = Lock("PROGDIR:", SHARED_LOCK)) != BNULL) @@ -510,11 +522,12 @@ bool Sys_GetPath(sysPath_t type, idStr &path) { D(bug("[ADoom3] Sys_GetPath: using '%s'\n", buf)); path = buf; + ret = true; } UnLock(pathlock); } - return true; + break; } - return false; + return ret; } diff --git a/neo/sys/aros/aros_main.cpp b/neo/sys/aros/aros_main.cpp index cdaa872a..afad94f7 100644 --- a/neo/sys/aros/aros_main.cpp +++ b/neo/sys/aros/aros_main.cpp @@ -92,6 +92,8 @@ idCVar in_tty( "in_tty", "1", CVAR_BOOL | CVAR_INIT | CVAR_SYSTEM, "terminal tab static bool tty_enabled = false; static struct termios tty_tc; +static FILE* consoleLog = NULL; + // pid - useful when you attach to gdb.. idCVar com_pid( "com_pid", "0", CVAR_INTEGER | CVAR_INIT | CVAR_SYSTEM, "process id" ); @@ -119,6 +121,11 @@ void AROS_Exit(int ret) { // at this point, too late to catch signals AROS_ClearSigs(); + if( consoleLog != NULL ) { + fclose(consoleLog); + consoleLog = NULL; + } + // process spawning. it's best when it happens after everything has shut down if ( exit_spawn[0] ) { Sys_DoStartProcess( exit_spawn, false ); @@ -309,6 +316,35 @@ void Sys_SetPhysicalWorkMemory( int minBytes, int maxBytes ) { common->DPrintf( "TODO: Sys_SetPhysicalWorkMemory\n" ); } +extern bool AROS_GetSavePath(char buf[1024]); + +static void initLog() +{ + char logPath[1024]; + if(!AROS_GetSavePath(logPath)) + return; + + // TODO: create savePath directory if it doesn't exist.. + + char logBkPath[1024]; + strcpy(logBkPath, logPath); + idStr::Append(logBkPath, sizeof(logBkPath), PATHSEPERATOR_STR "dhewm3log-old.txt"); + idStr::Append(logPath, sizeof(logPath), PATHSEPERATOR_STR "dhewm3log.txt"); + + rename(logPath, logBkPath); // I hope AROS supports this, but it's standard C89 so it should + + consoleLog = fopen(logPath, "w"); + if(consoleLog == NULL) { + printf("WARNING: Couldn't open/create '%s', error was: %d (%s)\n", logPath, errno, strerror(errno)); + } else { + time_t tt = time(NULL); + const struct tm* tms = localtime(&tt); + char timeStr[64] = {}; + strftime(timeStr, sizeof(timeStr), "%F %H:%M:%S", tms); + fprintf(consoleLog, "Opened this log at %s\n", timeStr); + } +} + /* =============== AROS_EarlyInit @@ -317,9 +353,13 @@ AROS_EarlyInit void AROS_EarlyInit( void ) { bug("[ADoom3] %s()\n", __PRETTY_FUNCTION__); + initLog(); + exit_spawn[0] = '\0'; AROS_InitLibs(); AROS_InitSigs(); + + // TODO: logfile } /* @@ -736,19 +776,36 @@ low level output =============== */ +void Sys_VPrintf(const char *msg, va_list arg) { + // gonna use arg twice, so copy it + va_list arg2; + va_copy(arg2, arg); + + // first print to stdout() + vprintf(msg, arg2); + + va_end(arg2); // arg2 is not needed anymore + + // then print to the log, if any + if(consoleLog != NULL) + { + vfprintf(consoleLog, msg, arg); + } +} + void Sys_DebugPrintf( const char *fmt, ... ) { va_list argptr; tty_Hide(); va_start( argptr, fmt ); - vprintf( fmt, argptr ); + Sys_VPrintf( fmt, argptr ); va_end( argptr ); tty_Show(); } void Sys_DebugVPrintf( const char *fmt, va_list arg ) { tty_Hide(); - vprintf( fmt, arg ); + Sys_VPrintf( fmt, arg ); tty_Show(); } @@ -757,17 +814,11 @@ void Sys_Printf(const char *msg, ...) { tty_Hide(); va_start( argptr, msg ); - vprintf( msg, argptr ); + Sys_VPrintf( msg, argptr ); va_end( argptr ); tty_Show(); } -void Sys_VPrintf(const char *msg, va_list arg) { - tty_Hide(); - vprintf(msg, arg); - tty_Show(); -} - /* ================ Sys_Error From 67d0b7cf01509c70653393cbbdf63c75501d2b50 Mon Sep 17 00:00:00 2001 From: Daniel Gibson Date: Sun, 16 Jan 2022 06:22:44 +0100 Subject: [PATCH 5/8] dhewm3log.txt for Windows, update changelog I was lazy and just renamed SDL_win32_main's stdout.txt - but I still added the dhewm3log-old.txt backup function. I also renamed Sys_GetHomeDir() to Win_GetHomeDir() as it's Win32-only On Windows it's in Documents\My Games\dhewm3\dhewm3log.txt --- Changelog.md | 14 ++++++++++++-- neo/sys/win32/SDL_win32_main.c | 35 ++++++++++++++++++++++++---------- neo/sys/win32/win_main.cpp | 21 +++++++++++++++++--- 3 files changed, 55 insertions(+), 15 deletions(-) diff --git a/Changelog.md b/Changelog.md index e666f8d6..855d5b45 100644 --- a/Changelog.md +++ b/Changelog.md @@ -50,14 +50,24 @@ Note: Numbers starting with a "#" like #330 refer to the bugreport with that num * `s_alReverbGain` CVar to reduce EFX reverb effect intensity (#365) * Pause (looped) sounds when entering menu (#330) * Fixes for looped sounds (#390) -* (Optionally) use libbacktrace on non-Windows platforms for more useful - backtraces in case of crashes (usually linked statically) * Replace libjpeg with stb_image and libogg/libvorbis(file) with stb_vorbis - Now the only required external dependencies should be OpenAL, SDL, zlib and optionally libCURL (and of course the C and C++ runtimes) +* (Optionally) use libbacktrace on non-Windows platforms for more useful + backtraces in case of crashes (usually linked statically) * Fixed a deadlock (freeze) on Windows when printing messages from another thread * Fixed some warnings and uninitialized variables (thanks *turol*!) * Work around dmap bug caused by GCC using FMA "optimizations" (#147) +* Prevent dhewm3 from being run as root on Unix-like systems to improve security +* Replaced most usages of `strncpy()` with something safer to prevent buffer overflows + (remaining cases should be safe). + - Just a precaution, I don't know if any of them could actually be exploited, + but there were some compiler warnings in newer GCC versions. +* Console output is now logged to `dhewm3log.txt` (last log is renamed to `dhewm3log-old.txt`) + - On Windows it's in `My Documents/My Games/dhewm3/` + - On Mac it's in `$HOME/Library/Application Support/dhewm3/` + - On other Unix-like systems like Linux it's in `$XDG_DATA_HOME/dhewm3/` + (usually `$HOME/.local/share/dhewm3/`) 1.5.1 (2021-03-14) diff --git a/neo/sys/win32/SDL_win32_main.c b/neo/sys/win32/SDL_win32_main.c index 5f243a65..2dce3193 100644 --- a/neo/sys/win32/SDL_win32_main.c +++ b/neo/sys/win32/SDL_win32_main.c @@ -37,7 +37,7 @@ #endif /* main */ /* The standard output files */ -#define STDOUT_FILE TEXT("stdout.txt") +#define STDOUT_FILE TEXT("dhewm3log.txt") /* DG: renamed this */ #define STDERR_FILE TEXT("stderr.txt") /* Set a variable to tell if the stdio redirect has been enabled. */ @@ -197,7 +197,7 @@ static void cleanup_output(void) { } } -extern int Sys_GetHomeDir(char *dst, size_t size); +extern int Win_GetHomeDir(char *dst, size_t size); /* Redirect the output (stdout and stderr) to a file */ static void redirect_output(void) @@ -209,31 +209,32 @@ static void redirect_output(void) char path[MAX_PATH]; struct _stat st; - // DG: use "My Documents/My Games/dhewm3" to write stdout.txt and stderr.txt - // instead of the binary, which might not be writable - Sys_GetHomeDir(path, sizeof(path)); + /* DG: use "My Documents/My Games/dhewm3" to write stdout.txt and stderr.txt + * instead of the binary, which might not be writable */ + Win_GetHomeDir(path, sizeof(path)); if (_stat(path, &st) == -1) { - // oops, "My Documents/My Games/dhewm3" doesn't exist - does My Games/ at least exist? + /* oops, "My Documents/My Games/dhewm3" doesn't exist - does My Games/ at least exist? */ char myGamesPath[MAX_PATH]; + char* lastslash; memcpy(myGamesPath, path, MAX_PATH); - char* lastslash = strrchr(myGamesPath, '/'); + lastslash = strrchr(myGamesPath, '/'); if (lastslash != NULL) { *lastslash = '\0'; } if (_stat(myGamesPath, &st) == -1) { - // if My Documents/My Games/ doesn't exist, create it + /* if My Documents/My Games/ doesn't exist, create it */ _mkdir(myGamesPath); } - _mkdir(path); // create My Documents/My Games/dhewm3/ + _mkdir(path); /* create My Documents/My Games/dhewm3/ */ } #endif FILE *newfp; -#if 0 // DG: don't do this anymore. +#if 0 /* DG: don't do this anymore. */ DWORD pathlen; pathlen = GetModuleFileName(NULL, path, SDL_arraysize(path)); while ( pathlen > 0 && path[pathlen] != '\\' ) { @@ -250,6 +251,20 @@ static void redirect_output(void) SDL_strlcat( stdoutPath, DIR_SEPERATOR STDOUT_FILE, SDL_arraysize(stdoutPath) ); #endif + { /* DG: rename old stdout log */ +#ifdef _WIN32_WCE + wchar_t stdoutPathBK[MAX_PATH]; + wcsncpy( stdoutPathBK, path, SDL_arraysize(stdoutPath) ); + wcsncat( stdoutPathBK, DIR_SEPERATOR TEXT("dhewm3log-old.txt"), SDL_arraysize(stdoutPath) ); + _wrename( stdoutPath, stdoutpathBK ); +#else + char stdoutPathBK[MAX_PATH]; + SDL_strlcpy( stdoutPathBK, path, SDL_arraysize(stdoutPath) ); + SDL_strlcat( stdoutPathBK, DIR_SEPERATOR TEXT("dhewm3log-old.txt"), SDL_arraysize(stdoutPath) ); + rename( stdoutPath, stdoutPathBK ); +#endif + } /* DG end */ + /* Redirect standard input and standard output */ newfp = freopen(stdoutPath, TEXT("w"), stdout); diff --git a/neo/sys/win32/win_main.cpp b/neo/sys/win32/win_main.cpp index 759c7a60..8b3a1c81 100644 --- a/neo/sys/win32/win_main.cpp +++ b/neo/sys/win32/win_main.cpp @@ -377,7 +377,7 @@ static int WPath2A(char *dst, size_t size, const WCHAR *src) { /* ============== Returns "My Documents"/My Games/dhewm3 directory (or equivalent - "CSIDL_PERSONAL"). -To be used with Sys_DefaultSavePath(), so savegames, screenshots etc will be +To be used with Sys_GetPath(PATH_SAVE), so savegames, screenshots etc will be saved to the users files instead of systemwide. Based on (with kind permission) Yamagi Quake II's Sys_GetHomeDir() @@ -386,7 +386,7 @@ Returns the number of characters written to dst ============== */ extern "C" { // DG: I need this in SDL_win32_main.c - int Sys_GetHomeDir(char *dst, size_t size) + int Win_GetHomeDir(char *dst, size_t size) { int len; WCHAR profile[MAX_OSPATH]; @@ -481,7 +481,7 @@ bool Sys_GetPath(sysPath_t type, idStr &path) { case PATH_CONFIG: case PATH_SAVE: - if (Sys_GetHomeDir(buf, sizeof(buf)) < 1) { + if (Win_GetHomeDir(buf, sizeof(buf)) < 1) { Sys_Error("ERROR: Couldn't get dir to home path"); return false; } @@ -748,6 +748,11 @@ void Sys_Init( void ) { #if 0 cmdSystem->AddCommand( "setAsyncSound", Sys_SetAsyncSound_f, CMD_FL_SYSTEM, "set the async sound option" ); #endif + { + idStr savepath; + Sys_GetPath( PATH_SAVE, savepath ); + common->Printf( "Logging console output to %s/dhewm3log.txt\n", savepath.c_str() ); + } // // Windows version @@ -1002,6 +1007,16 @@ WinMain ================== */ int main(int argc, char *argv[]) { + // SDL_win32_main.c creates the dhewm3log.txt and redirects stdout into it + // so here we can log its (approx.) creation time before anything else is logged: + { + time_t tt = time(NULL); + const struct tm* tms = localtime(&tt); + char timeStr[64] = {}; + strftime(timeStr, sizeof(timeStr), "%F %H:%M:%S", tms); + printf("Opened this log at %s\n", timeStr); + } + const HCURSOR hcurSave = ::SetCursor( LoadCursor( 0, IDC_WAIT ) ); InitializeCriticalSection( &printfCritSect ); From 5f3356627e640b3dfc4ec853cb4b50e378ed11ba Mon Sep 17 00:00:00 2001 From: Daniel Gibson Date: Sun, 16 Jan 2022 17:55:11 +0100 Subject: [PATCH 6/8] CMake: (Theoretically) support Windows on ARM, try to unify ARM CPU names I don't have such hardware, so I can't test; I also didn't actually build dhewm3 (would have to build dependencies for ARM first..), but when configuring ARM or ARM64 targets for Visual C++ in CMake, that's now correctly detected in CMake. Also, now D3_ARCH should always be "arm" for 32bit ARM and "arm64" for 64bit ARM, also when building with GCC or clang. --- neo/CMakeLists.txt | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/neo/CMakeLists.txt b/neo/CMakeLists.txt index f733f5dd..64e36b7a 100644 --- a/neo/CMakeLists.txt +++ b/neo/CMakeLists.txt @@ -79,17 +79,21 @@ if(NOT (CMAKE_SYSTEM_PROCESSOR STREQUAL CMAKE_HOST_SYSTEM_PROCESSOR)) # special case: cross-compiling, here CMAKE_SYSTEM_PROCESSOR should be correct, hopefully # (just leave cpu at ${CMAKE_SYSTEM_PROCESSOR}) elseif(MSVC) - # TODO: use CMAKE_GENERATOR_PLATFORM ? not sure if that works when building with MSVC compiler + ninja - # or only when generating VS solutions (currently CMAKE_GENERATOR_PLATFORM can be "Win32" "x64" "ARM" or "ARM64") - if(cpu MATCHES ".*[aA][rR][mM].*") - # no idea how to detect windows for ARM(64), I don't own such hardware - message(FATAL_ERROR "please fix this code to work for Windows on ARM and send a pull request") - endif() - if(CMAKE_CL_64) - set(cpu "x86_64") - else() + message(STATUS "CMAKE_GENERATOR_PLATFORM: ${CMAKE_GENERATOR_PLATFORM}") + if(CMAKE_GENERATOR_PLATFORM STREQUAL "Win32") set(cpu "x86") + elseif(CMAKE_GENERATOR_PLATFORM STREQUAL "x64") + set(cpu "x64_64") + elseif(CMAKE_GENERATOR_PLATFORM STREQUAL "ARM") + # at least on RPi 32bit, gcc -dumpmachine outputs "arm-linux-gnueabihf", + # so we'll use "arm" there => use the same for 32bit ARM on MSVC + set(cpu "arm") + elseif(CMAKE_GENERATOR_PLATFORM STREQUAL "ARM64") + set(cpu "arm64") + else() + message(FATAL_ERROR "Unknown Target CPU/platform ${CMAKE_GENERATOR_PLATFORM}") endif() + message(STATUS " => CPU architecture extracted from that: \"${cpu}\"") else() # not MSVC and not cross-compiling, assume GCC or clang (-compatible), seems to work for MinGW as well execute_process(COMMAND ${CMAKE_C_COMPILER} "-dumpmachine" RESULT_VARIABLE cc_dumpmachine_res @@ -110,10 +114,21 @@ endif() if(cpu STREQUAL "powerpc") set(cpu "ppc") +elseif(cpu STREQUAL "aarch64") + # "arm64" is more obvious, and some operating systems (like macOS) use it instead of "aarch64" + set(cpu "arm64") elseif(cpu MATCHES "i.86") set(cpu "x86") elseif(cpu MATCHES "[aA][mM][dD]64" OR cpu MATCHES "[xX]64") set(cpu "x86_64") +elseif(cpu MATCHES "[aA][rR][mM].*") # some kind of arm.. + # On 32bit Raspbian gcc -dumpmachine returns sth starting with "arm-", + # while clang -dumpmachine says "arm6k-..." - try to unify that to "arm" + if(CMAKE_SIZEOF_VOID_P EQUAL 8) # sizeof(void*) == 8 => must be arm64 + set(cpu "arm64") + else() # should be 32bit arm then (probably "armv7l" "armv6k" or sth like that) + set(cpu "arm") + endif() endif() add_definitions(-DD3_ARCH="${cpu}" -DD3_SIZEOFPTR=${CMAKE_SIZEOF_VOID_P}) From eff9fd6ac3bf9742fb9a26558817ef86a2d93cf1 Mon Sep 17 00:00:00 2001 From: Daniel Gibson Date: Sun, 16 Jan 2022 19:39:35 +0100 Subject: [PATCH 7/8] GLimp_Init(): Log r_mode and resolution used for creating window --- neo/sys/glimp.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/neo/sys/glimp.cpp b/neo/sys/glimp.cpp index ae392d76..6166796c 100644 --- a/neo/sys/glimp.cpp +++ b/neo/sys/glimp.cpp @@ -241,8 +241,16 @@ bool GLimp_Init(glimpParms_t parms) { SDL_GL_SetAttribute(SDL_GL_MULTISAMPLESAMPLES, parms.multiSamples); #if SDL_VERSION_ATLEAST(2, 0, 0) - int displayIndex = 0; + const char* windowMode = ""; + if(r_fullscreen.GetBool()) { + windowMode = r_fullscreenDesktop.GetBool() ? "desktop-fullscreen-" : "fullscreen-"; + } + + common->Printf("Will create a %swindow with resolution %dx%d (r_mode = %d)\n", + windowMode, parms.width, parms.height, r_mode.GetInteger()); + + int displayIndex = 0; #if SDL_VERSION_ATLEAST(2, 0, 4) // try to put the window on the display the mousecursor currently is on { From d34832e4fc772588747697a869d4bba5d26a1e71 Mon Sep 17 00:00:00 2001 From: Daniel Gibson Date: Mon, 17 Jan 2022 15:31:10 +0100 Subject: [PATCH 8/8] Fix Mac build --- neo/sys/osx/DOOMController.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/neo/sys/osx/DOOMController.mm b/neo/sys/osx/DOOMController.mm index 8360de74..5d363384 100644 --- a/neo/sys/osx/DOOMController.mm +++ b/neo/sys/osx/DOOMController.mm @@ -196,7 +196,7 @@ int SDL_main( int argc, char *argv[] ) { // DG: set exe_path so Posix_InitSignalHandlers() can call Posix_GetExePath() SDL_strlcpy(exe_path, [ [ [ NSBundle mainBundle ] bundlePath ] cString ], sizeof(exe_path)); // same for save_path for Posix_GetSavePath() - snprintf(save_path, sizeof(save_path), "%s/Library/Application Support/dhewm3", [NSHomeDirectory() cString]); + D3_snprintfC99(save_path, sizeof(save_path), "%s/Library/Application Support/dhewm3", [NSHomeDirectory() cString]); // and preinitializing basepath is easy enough so do that as well { char* snap;