Windows: Fix crash handling stackwalking code rarely crashing

Based on the CPU registers provided in the crash log, the crash was happening near the end of copying from 'ctx' to 'copy' local variable

Given that the user was running on Windows 95/98, it's quite possible that the 'ctx' CONTEXT was smaller in older operating systems - and so trying to copy a full sized CONTEXT into 'copy' would mean that memory past the smaller sized CONTEXT was wrongly read, which could thus rarely cause an access violation

I can't confirm whether this is actually the case or not though, since I couldn't easily find SDK headers for Windows 95/98.

But since we don't care about StackWalk modifying the CONTEXT provided as an argument anyways (given that the process will be terminated right after this), the simplest solution is to just pass the original 'ctx' to StackWalk instead of passing a copy of the CONTEXT
This commit is contained in:
UnknownShadow200 2023-06-23 20:42:12 +10:00
parent 9809e34e78
commit c4858bb8d0
4 changed files with 28 additions and 13 deletions

View File

@ -1,4 +1,4 @@
ClassiCube is a custom Minecraft Classic compatible client written in C that works on Windows, macOS, Linux, iOS, Android, FreeBSD, NetBSD, OpenBSD, Solaris, Haiku, IRIX, 3DS (unfinished), PSP (unfinished), and in a web browser.<br> ClassiCube is a custom Minecraft Classic compatible client written in C that works on Windows, macOS, Linux, iOS, Android, FreeBSD, NetBSD, OpenBSD, Solaris, Haiku, IRIX, SerenityOS, 3DS (unfinished), PSP (unfinished), GameCube (unfinished), Wii (unfinished), and in a web browser.<br>
**It is not affiliated with (or supported by) Mojang AB, Minecraft, or Microsoft in any way.** **It is not affiliated with (or supported by) Mojang AB, Minecraft, or Microsoft in any way.**
![screenshot_n](http://i.imgur.com/FCiwl27.png) ![screenshot_n](http://i.imgur.com/FCiwl27.png)
@ -199,10 +199,26 @@ The generated javascript file has some issues. [See here for how to fix](doc/com
cd into `src` directory, then run `make psp`. You'll need [pspsdk](https://github.com/pspdev/pspsdk) cd into `src` directory, then run `make psp`. You'll need [pspsdk](https://github.com/pspdev/pspsdk)
The PSP port needs assistance from someone experienced with PSP homebrew development - if you're interested, please get in contact with me. (`unknownshadow200` on Discord)
#### 3DS #### 3DS
cd into `src` directory, then run `make 3ds`. You'll need [libctru](https://github.com/devkitPro/libctru) cd into `src` directory, then run `make 3ds`. You'll need [libctru](https://github.com/devkitPro/libctru)
The 3DS port needs assistance from someone experienced with 3DS homebrew development - if you're interested, please get in contact with me. (`unknownshadow200` on Discord)
#### GameCube
Use a slighly modified standard GameCube makefile. You'll need [libogc](https://github.com/devkitPro/libogc)
The GC port needs assistance from someone experienced with GC homebrew development - if you're interested, please get in contact with me. (`unknownshadow200` on Discord)
#### Wii
Use a slighly modified standard Wii makefile. You'll need [libogc](https://github.com/devkitPro/libogc)
The Wii port needs assistance from someone experienced with Wii homebrew development - if you're interested, please get in contact with me. (`unknownshadow200` on Discord)
##### Other ##### Other
You'll have to write the necessary code. You should read portability.md in doc folder. You'll have to write the necessary code. You should read portability.md in doc folder.

View File

@ -27,7 +27,6 @@ typedef void (*LScreen_Func)(struct LScreen* s);
struct LWidget* selectedWidget; /* Widget mouse last clicked on. */ \ struct LWidget* selectedWidget; /* Widget mouse last clicked on. */ \
int numWidgets; /* Number of widgets actually used. */ \ int numWidgets; /* Number of widgets actually used. */ \
struct LWidget** widgets; /* Array of pointers to all widgets in the screen. */ \ struct LWidget** widgets; /* Array of pointers to all widgets in the screen. */ \
cc_bool hidesTitlebar; /* Whether titlebar in window is hidden. */ \
const char* title; /* Titlebar text */ const char* title; /* Titlebar text */
struct LScreen { LScreen_Layout }; struct LScreen { LScreen_Layout };

View File

@ -275,7 +275,6 @@ static int GetFrames(CONTEXT* ctx, cc_uintptr* addrs, int max) {
STACKFRAME frame = { 0 }; STACKFRAME frame = { 0 };
HANDLE process, thread; HANDLE process, thread;
int count, type; int count, type;
CONTEXT copy;
frame.AddrPC.Mode = AddrModeFlat; frame.AddrPC.Mode = AddrModeFlat;
frame.AddrFrame.Mode = AddrModeFlat; frame.AddrFrame.Mode = AddrModeFlat;
@ -297,10 +296,10 @@ static int GetFrames(CONTEXT* ctx, cc_uintptr* addrs, int max) {
#endif #endif
process = GetCurrentProcess(); process = GetCurrentProcess();
thread = GetCurrentThread(); thread = GetCurrentThread();
copy = *ctx;
for (count = 0; count < max; count++) { for (count = 0; count < max; count++)
if (!StackWalk(type, process, thread, &frame, &copy, NULL, SymFunctionTableAccess, SymGetModuleBase, NULL)) break; {
if (!StackWalk(type, process, thread, &frame, ctx, NULL, SymFunctionTableAccess, SymGetModuleBase, NULL)) break;
if (!frame.AddrFrame.Offset) break; if (!frame.AddrFrame.Offset) break;
addrs[count] = frame.AddrPC.Offset; addrs[count] = frame.AddrPC.Offset;
} }
@ -313,7 +312,7 @@ void Logger_Backtrace(cc_string* trace, void* ctx) {
HANDLE process; HANDLE process;
process = GetCurrentProcess(); process = GetCurrentProcess();
SymInitialize(process, NULL, TRUE); SymInitialize(process, NULL, TRUE); /* TODO only in MSVC.. */
frames = GetFrames((CONTEXT*)ctx, addrs, MAX_BACKTRACE_FRAMES); frames = GetFrames((CONTEXT*)ctx, addrs, MAX_BACKTRACE_FRAMES);
for (i = 0; i < frames; i++) { for (i = 0; i < frames; i++) {
@ -849,7 +848,7 @@ static void DumpRegisters(void* ctx) {
*------------------------------------------------Module/Memory map handling-----------------------------------------------* *------------------------------------------------Module/Memory map handling-----------------------------------------------*
*#########################################################################################################################*/ *#########################################################################################################################*/
#if defined CC_BUILD_WIN #if defined CC_BUILD_WIN
static BOOL CALLBACK DumpModule(const char* name, ULONG_PTR base, ULONG size, void* ctx) { static BOOL CALLBACK DumpModule(const char* name, ULONG_PTR base, ULONG size, void* userCtx) {
cc_string str; char strBuffer[256]; cc_string str; char strBuffer[256];
cc_uintptr beg, end; cc_uintptr beg, end;
@ -862,7 +861,7 @@ static BOOL CALLBACK DumpModule(const char* name, ULONG_PTR base, ULONG size, vo
} }
static BOOL (WINAPI *_EnumerateLoadedModules)(HANDLE process, PENUMLOADED_MODULES_CALLBACK callback, PVOID userContext); static BOOL (WINAPI *_EnumerateLoadedModules)(HANDLE process, PENUMLOADED_MODULES_CALLBACK callback, PVOID userContext);
static void DumpMisc(void* ctx) { static void DumpMisc(void) {
static const cc_string modules = String_FromConst("-- modules --\r\n"); static const cc_string modules = String_FromConst("-- modules --\r\n");
HANDLE process = GetCurrentProcess(); HANDLE process = GetCurrentProcess();
@ -921,7 +920,7 @@ static int SkipRange(const cc_string* str) {
} }
#endif #endif
static void DumpMisc(void* ctx) { static void DumpMisc(void) {
static const cc_string memMap = String_FromConst("-- memory map --\n"); static const cc_string memMap = String_FromConst("-- memory map --\n");
cc_string str; char strBuffer[320]; cc_string str; char strBuffer[320];
int n, fd; int n, fd;
@ -942,7 +941,7 @@ static void DumpMisc(void* ctx) {
#elif defined CC_BUILD_DARWIN #elif defined CC_BUILD_DARWIN
#include <mach-o/dyld.h> #include <mach-o/dyld.h>
static void DumpMisc(void* ctx) { static void DumpMisc(void) {
static const cc_string modules = String_FromConst("-- modules --\n"); static const cc_string modules = String_FromConst("-- modules --\n");
static const cc_string newLine = String_FromConst(_NL); static const cc_string newLine = String_FromConst(_NL);
cc_uint32 i, count; cc_uint32 i, count;
@ -964,7 +963,7 @@ static void DumpMisc(void* ctx) {
} }
} }
#else #else
static void DumpMisc(void* ctx) { } static void DumpMisc(void) { }
#endif #endif
@ -1222,7 +1221,7 @@ static void AbortCommon(cc_result result, const char* raw_msg, void* ctx) {
Logger_Log(&backtrace); Logger_Log(&backtrace);
Logger_Backtrace(&msg, ctx); Logger_Backtrace(&msg, ctx);
DumpMisc(ctx); DumpMisc();
CloseLogFile(); CloseLogFile();
msg.buffer[msg.length] = '\0'; msg.buffer[msg.length] = '\0';

View File

@ -43,6 +43,7 @@ void Logger_SysWarn2(cc_result res, const char* action, const cc_string* path);
/* This is used to attempt to log some information about a crash due to invalid memory read, etc. */ /* This is used to attempt to log some information about a crash due to invalid memory read, etc. */
void Logger_Hook(void); void Logger_Hook(void);
/* Generates a backtrace based on the platform-specific CPU context. */ /* Generates a backtrace based on the platform-specific CPU context. */
/* NOTE: The provided CPU context may be modified (e.g on Windows) */
void Logger_Backtrace(cc_string* trace, void* ctx); void Logger_Backtrace(cc_string* trace, void* ctx);
/* Logs a message to client.log on disc. */ /* Logs a message to client.log on disc. */
/* NOTE: The message is written raw, it is NOT converted to unicode (unlike Stream_WriteLine) */ /* NOTE: The message is written raw, it is NOT converted to unicode (unlike Stream_WriteLine) */