From 18154a6bae476302fc7f6db8fdefa85c0119c7c6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 11 Jan 2026 15:48:27 +0000 Subject: [PATCH 01/13] Initial plan From e3ce225c1becf51d00e38a8e65ce3600bb6cd63b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 11 Jan 2026 15:55:07 +0000 Subject: [PATCH 02/13] Refactor g_argvCreateDump from vector to fixed-size array Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com> --- src/coreclr/pal/src/thread/process.cpp | 108 +++++++++++++++---------- 1 file changed, 64 insertions(+), 44 deletions(-) diff --git a/src/coreclr/pal/src/thread/process.cpp b/src/coreclr/pal/src/thread/process.cpp index 129c883d522d5d..896167baac9d95 100644 --- a/src/coreclr/pal/src/thread/process.cpp +++ b/src/coreclr/pal/src/thread/process.cpp @@ -60,7 +60,6 @@ SET_DEFAULT_DEBUG_CHANNEL(PROCESS); // some headers have code with asserts, so d #include #include #include -#include #ifdef __linux__ #include @@ -197,7 +196,8 @@ Volatile g_shutdownCallback = nullptr; Volatile g_createdumpCallback = nullptr; // Crash dump generating program arguments. Initialized in PROCAbortInitialize(). -std::vector g_argvCreateDump; +#define MAX_ARGV_ENTRIES 32 +const char* g_argvCreateDump[MAX_ARGV_ENTRIES] = { nullptr }; // // Key used for associating CPalThread's with the underlying pthread @@ -2282,7 +2282,7 @@ Return --*/ BOOL PROCBuildCreateDumpCommandLine( - std::vector& argv, + const char* argv[], char** pprogram, char** ppidarg, const char* dumpName, @@ -2323,27 +2323,29 @@ PROCBuildCreateDumpCommandLine( { return FALSE; } - argv.push_back(program); + + int argc = 0; + argv[argc++] = program; if (dumpName != nullptr) { - argv.push_back("--name"); - argv.push_back(dumpName); + argv[argc++] = "--name"; + argv[argc++] = dumpName; } switch (dumpType) { case DumpTypeNormal: - argv.push_back("--normal"); + argv[argc++] = "--normal"; break; case DumpTypeWithHeap: - argv.push_back("--withheap"); + argv[argc++] = "--withheap"; break; case DumpTypeTriage: - argv.push_back("--triage"); + argv[argc++] = "--triage"; break; case DumpTypeFull: - argv.push_back("--full"); + argv[argc++] = "--full"; break; default: break; @@ -2351,37 +2353,42 @@ PROCBuildCreateDumpCommandLine( if (flags & GenerateDumpFlagsLoggingEnabled) { - argv.push_back("--diag"); + argv[argc++] = "--diag"; } if (flags & GenerateDumpFlagsVerboseLoggingEnabled) { - argv.push_back("--verbose"); + argv[argc++] = "--verbose"; } if (flags & GenerateDumpFlagsCrashReportEnabled) { - argv.push_back("--crashreport"); + argv[argc++] = "--crashreport"; } if (flags & GenerateDumpFlagsCrashReportOnlyEnabled) { - argv.push_back("--crashreportonly"); + argv[argc++] = "--crashreportonly"; } if (g_running_in_exe) { - argv.push_back("--singlefile"); + argv[argc++] = "--singlefile"; } if (logFileName != nullptr) { - argv.push_back("--logtofile"); - argv.push_back(logFileName); + argv[argc++] = "--logtofile"; + argv[argc++] = logFileName; } - argv.push_back(*ppidarg); - argv.push_back(nullptr); + argv[argc++] = *ppidarg; + argv[argc++] = nullptr; + + if (argc >= MAX_ARGV_ENTRIES) + { + return FALSE; + } return TRUE; } @@ -2399,7 +2406,7 @@ PROCBuildCreateDumpCommandLine( --*/ BOOL PROCCreateCrashDump( - std::vector& argv, + const char* argv[], LPSTR errorMessageBuffer, INT cbErrorMessageBuffer, bool serialize) @@ -2407,7 +2414,7 @@ PROCCreateCrashDump( #if defined(TARGET_IOS) || defined(TARGET_TVOS) return FALSE; #else - _ASSERTE(argv.size() > 0); + _ASSERTE(argv[0] != nullptr); _ASSERTE(errorMessageBuffer == nullptr || cbErrorMessageBuffer > 0); if (serialize) @@ -2494,7 +2501,12 @@ PROCCreateCrashDump( SEHCleanupSignals(true /* isChildProcess */); // Call the statically linked createdump code - callbackResult = g_createdumpCallback(argv.size(), argv.data()); + int argc = 0; + while (argv[argc] != nullptr && argc < MAX_ARGV_ENTRIES) + { + argc++; + } + callbackResult = g_createdumpCallback(argc, argv); // Set the shutdown callback to nullptr and exit // If we don't exit, the child's execution will continue into the diagnostic server behavior // which causes all sorts of problems. @@ -2504,7 +2516,7 @@ PROCCreateCrashDump( else { // Execute the createdump program - if (execve(argv[0], (char**)argv.data(), palEnvironment) == -1) + if (execve(argv[0], (char**)argv, palEnvironment) == -1) { fprintf(stderr, "Problem launching createdump (may not have execute permissions): execve(%s) FAILED %s (%d)\n", argv[0], strerror(errno), errno); exit(-1); @@ -2682,7 +2694,7 @@ PAL_GenerateCoreDump( LPSTR errorMessageBuffer, INT cbErrorMessageBuffer) { - std::vector argvCreateDump; + const char* argvCreateDump[MAX_ARGV_ENTRIES] = { nullptr }; if (dumpType <= DumpTypeUnknown || dumpType > DumpTypeMax) { @@ -2748,59 +2760,67 @@ PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo, void* context, bool DoNotOptimize(&context); // If enabled, launch the create minidump utility and wait until it completes - if (!g_argvCreateDump.empty()) + if (g_argvCreateDump[0] != nullptr) { - std::vector argv(g_argvCreateDump); + const char* argv[MAX_ARGV_ENTRIES]; char* signalArg = nullptr; char* crashThreadArg = nullptr; char* signalCodeArg = nullptr; char* signalErrnoArg = nullptr; char* signalAddressArg = nullptr; - if (signal != 0) + // Copy the createdump argv + int argc = 0; + for (; argc < MAX_ARGV_ENTRIES && g_argvCreateDump[argc] != nullptr; argc++) { - // Remove the terminating nullptr - argv.pop_back(); + argv[argc] = g_argvCreateDump[argc]; + } + if (signal != 0 && argc < MAX_ARGV_ENTRIES) + { // Add the signal number to the command line signalArg = PROCFormatInt(signal); if (signalArg != nullptr) { - argv.push_back("--signal"); - argv.push_back(signalArg); + argv[argc++] = "--signal"; + argv[argc++] = signalArg; } // Add the current thread id to the command line. This function is always called on the crashing thread. crashThreadArg = PROCFormatInt(THREADSilentGetCurrentThreadId()); - if (crashThreadArg != nullptr) + if (crashThreadArg != nullptr && argc < MAX_ARGV_ENTRIES) { - argv.push_back("--crashthread"); - argv.push_back(crashThreadArg); + argv[argc++] = "--crashthread"; + argv[argc++] = crashThreadArg; } - if (siginfo != nullptr) + if (siginfo != nullptr && argc < MAX_ARGV_ENTRIES) { signalCodeArg = PROCFormatInt(siginfo->si_code); if (signalCodeArg != nullptr) { - argv.push_back("--code"); - argv.push_back(signalCodeArg); + argv[argc++] = "--code"; + argv[argc++] = signalCodeArg; } signalErrnoArg = PROCFormatInt(siginfo->si_errno); - if (signalErrnoArg != nullptr) + if (signalErrnoArg != nullptr && argc < MAX_ARGV_ENTRIES) { - argv.push_back("--errno"); - argv.push_back(signalErrnoArg); + argv[argc++] = "--errno"; + argv[argc++] = signalErrnoArg; } signalAddressArg = PROCFormatInt64((ULONG64)siginfo->si_addr); - if (signalAddressArg != nullptr) + if (signalAddressArg != nullptr && argc < MAX_ARGV_ENTRIES) { - argv.push_back("--address"); - argv.push_back(signalAddressArg); + argv[argc++] = "--address"; + argv[argc++] = signalAddressArg; } } - argv.push_back(nullptr); + argv[argc++] = nullptr; + } + else + { + argv[argc] = nullptr; } PROCCreateCrashDump(argv, nullptr, 0, serialize); From 85d4b3981dabb9146b51c492238627dae4444378 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 11 Jan 2026 16:11:55 +0000 Subject: [PATCH 03/13] Add comprehensive bounds checking to prevent buffer overflow Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com> --- src/coreclr/pal/src/thread/process.cpp | 42 +++++++++++++++++--------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/src/coreclr/pal/src/thread/process.cpp b/src/coreclr/pal/src/thread/process.cpp index 896167baac9d95..0ab869074152df 100644 --- a/src/coreclr/pal/src/thread/process.cpp +++ b/src/coreclr/pal/src/thread/process.cpp @@ -2325,10 +2325,12 @@ PROCBuildCreateDumpCommandLine( } int argc = 0; + if (argc >= MAX_ARGV_ENTRIES) return FALSE; argv[argc++] = program; if (dumpName != nullptr) { + if (argc + 1 >= MAX_ARGV_ENTRIES) return FALSE; argv[argc++] = "--name"; argv[argc++] = dumpName; } @@ -2336,15 +2338,19 @@ PROCBuildCreateDumpCommandLine( switch (dumpType) { case DumpTypeNormal: + if (argc >= MAX_ARGV_ENTRIES) return FALSE; argv[argc++] = "--normal"; break; case DumpTypeWithHeap: + if (argc >= MAX_ARGV_ENTRIES) return FALSE; argv[argc++] = "--withheap"; break; case DumpTypeTriage: + if (argc >= MAX_ARGV_ENTRIES) return FALSE; argv[argc++] = "--triage"; break; case DumpTypeFull: + if (argc >= MAX_ARGV_ENTRIES) return FALSE; argv[argc++] = "--full"; break; default: @@ -2353,43 +2359,45 @@ PROCBuildCreateDumpCommandLine( if (flags & GenerateDumpFlagsLoggingEnabled) { + if (argc >= MAX_ARGV_ENTRIES) return FALSE; argv[argc++] = "--diag"; } if (flags & GenerateDumpFlagsVerboseLoggingEnabled) { + if (argc >= MAX_ARGV_ENTRIES) return FALSE; argv[argc++] = "--verbose"; } if (flags & GenerateDumpFlagsCrashReportEnabled) { + if (argc >= MAX_ARGV_ENTRIES) return FALSE; argv[argc++] = "--crashreport"; } if (flags & GenerateDumpFlagsCrashReportOnlyEnabled) { + if (argc >= MAX_ARGV_ENTRIES) return FALSE; argv[argc++] = "--crashreportonly"; } if (g_running_in_exe) { + if (argc >= MAX_ARGV_ENTRIES) return FALSE; argv[argc++] = "--singlefile"; } if (logFileName != nullptr) { + if (argc + 1 >= MAX_ARGV_ENTRIES) return FALSE; argv[argc++] = "--logtofile"; argv[argc++] = logFileName; } + if (argc + 1 >= MAX_ARGV_ENTRIES) return FALSE; argv[argc++] = *ppidarg; argv[argc++] = nullptr; - if (argc >= MAX_ARGV_ENTRIES) - { - return FALSE; - } - return TRUE; } @@ -2776,11 +2784,11 @@ PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo, void* context, bool argv[argc] = g_argvCreateDump[argc]; } - if (signal != 0 && argc < MAX_ARGV_ENTRIES) + if (signal != 0 && argc + 1 < MAX_ARGV_ENTRIES) { // Add the signal number to the command line signalArg = PROCFormatInt(signal); - if (signalArg != nullptr) + if (signalArg != nullptr && argc + 1 < MAX_ARGV_ENTRIES) { argv[argc++] = "--signal"; argv[argc++] = signalArg; @@ -2788,39 +2796,45 @@ PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo, void* context, bool // Add the current thread id to the command line. This function is always called on the crashing thread. crashThreadArg = PROCFormatInt(THREADSilentGetCurrentThreadId()); - if (crashThreadArg != nullptr && argc < MAX_ARGV_ENTRIES) + if (crashThreadArg != nullptr && argc + 1 < MAX_ARGV_ENTRIES) { argv[argc++] = "--crashthread"; argv[argc++] = crashThreadArg; } - if (siginfo != nullptr && argc < MAX_ARGV_ENTRIES) + if (siginfo != nullptr) { signalCodeArg = PROCFormatInt(siginfo->si_code); - if (signalCodeArg != nullptr) + if (signalCodeArg != nullptr && argc + 1 < MAX_ARGV_ENTRIES) { argv[argc++] = "--code"; argv[argc++] = signalCodeArg; } signalErrnoArg = PROCFormatInt(siginfo->si_errno); - if (signalErrnoArg != nullptr && argc < MAX_ARGV_ENTRIES) + if (signalErrnoArg != nullptr && argc + 1 < MAX_ARGV_ENTRIES) { argv[argc++] = "--errno"; argv[argc++] = signalErrnoArg; } signalAddressArg = PROCFormatInt64((ULONG64)siginfo->si_addr); - if (signalAddressArg != nullptr && argc < MAX_ARGV_ENTRIES) + if (signalAddressArg != nullptr && argc + 1 < MAX_ARGV_ENTRIES) { argv[argc++] = "--address"; argv[argc++] = signalAddressArg; } } - argv[argc++] = nullptr; + if (argc < MAX_ARGV_ENTRIES) + { + argv[argc++] = nullptr; + } } else { - argv[argc] = nullptr; + if (argc < MAX_ARGV_ENTRIES) + { + argv[argc] = nullptr; + } } PROCCreateCrashDump(argv, nullptr, 0, serialize); From d1570fa6ce600c6440cafb5c9a40f5fb5b26296e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 11 Jan 2026 16:14:12 +0000 Subject: [PATCH 04/13] Improve bounds checking to account for null terminator Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com> --- src/coreclr/pal/src/thread/process.cpp | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/coreclr/pal/src/thread/process.cpp b/src/coreclr/pal/src/thread/process.cpp index 0ab869074152df..8e7b2b4b5a929b 100644 --- a/src/coreclr/pal/src/thread/process.cpp +++ b/src/coreclr/pal/src/thread/process.cpp @@ -2325,12 +2325,12 @@ PROCBuildCreateDumpCommandLine( } int argc = 0; - if (argc >= MAX_ARGV_ENTRIES) return FALSE; + if (argc + 1 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for program and null terminator argv[argc++] = program; if (dumpName != nullptr) { - if (argc + 1 >= MAX_ARGV_ENTRIES) return FALSE; + if (argc + 3 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --name, dumpName, and null terminator argv[argc++] = "--name"; argv[argc++] = dumpName; } @@ -2338,19 +2338,19 @@ PROCBuildCreateDumpCommandLine( switch (dumpType) { case DumpTypeNormal: - if (argc >= MAX_ARGV_ENTRIES) return FALSE; + if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --normal and null terminator argv[argc++] = "--normal"; break; case DumpTypeWithHeap: - if (argc >= MAX_ARGV_ENTRIES) return FALSE; + if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --withheap and null terminator argv[argc++] = "--withheap"; break; case DumpTypeTriage: - if (argc >= MAX_ARGV_ENTRIES) return FALSE; + if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --triage and null terminator argv[argc++] = "--triage"; break; case DumpTypeFull: - if (argc >= MAX_ARGV_ENTRIES) return FALSE; + if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --full and null terminator argv[argc++] = "--full"; break; default: @@ -2359,42 +2359,42 @@ PROCBuildCreateDumpCommandLine( if (flags & GenerateDumpFlagsLoggingEnabled) { - if (argc >= MAX_ARGV_ENTRIES) return FALSE; + if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --diag and null terminator argv[argc++] = "--diag"; } if (flags & GenerateDumpFlagsVerboseLoggingEnabled) { - if (argc >= MAX_ARGV_ENTRIES) return FALSE; + if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --verbose and null terminator argv[argc++] = "--verbose"; } if (flags & GenerateDumpFlagsCrashReportEnabled) { - if (argc >= MAX_ARGV_ENTRIES) return FALSE; + if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --crashreport and null terminator argv[argc++] = "--crashreport"; } if (flags & GenerateDumpFlagsCrashReportOnlyEnabled) { - if (argc >= MAX_ARGV_ENTRIES) return FALSE; + if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --crashreportonly and null terminator argv[argc++] = "--crashreportonly"; } if (g_running_in_exe) { - if (argc >= MAX_ARGV_ENTRIES) return FALSE; + if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --singlefile and null terminator argv[argc++] = "--singlefile"; } if (logFileName != nullptr) { - if (argc + 1 >= MAX_ARGV_ENTRIES) return FALSE; + if (argc + 3 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --logtofile, logFileName, and null terminator argv[argc++] = "--logtofile"; argv[argc++] = logFileName; } - if (argc + 1 >= MAX_ARGV_ENTRIES) return FALSE; + if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for pidarg and null terminator argv[argc++] = *ppidarg; argv[argc++] = nullptr; From 5eeff5f45b0d58343e79a23d8d9dd24bf485b196 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 11 Jan 2026 16:16:21 +0000 Subject: [PATCH 05/13] Fix bounds checking in PROCCreateCrashDumpIfEnabled for two-argument additions Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com> --- src/coreclr/pal/src/thread/process.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/coreclr/pal/src/thread/process.cpp b/src/coreclr/pal/src/thread/process.cpp index 8e7b2b4b5a929b..b10afd0df470c9 100644 --- a/src/coreclr/pal/src/thread/process.cpp +++ b/src/coreclr/pal/src/thread/process.cpp @@ -2784,11 +2784,11 @@ PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo, void* context, bool argv[argc] = g_argvCreateDump[argc]; } - if (signal != 0 && argc + 1 < MAX_ARGV_ENTRIES) + if (signal != 0 && argc + 3 < MAX_ARGV_ENTRIES) // Need room for at least --signal, signalArg, and null terminator { // Add the signal number to the command line signalArg = PROCFormatInt(signal); - if (signalArg != nullptr && argc + 1 < MAX_ARGV_ENTRIES) + if (signalArg != nullptr && argc + 3 < MAX_ARGV_ENTRIES) // Need room for --signal, signalArg, and null terminator { argv[argc++] = "--signal"; argv[argc++] = signalArg; @@ -2796,7 +2796,7 @@ PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo, void* context, bool // Add the current thread id to the command line. This function is always called on the crashing thread. crashThreadArg = PROCFormatInt(THREADSilentGetCurrentThreadId()); - if (crashThreadArg != nullptr && argc + 1 < MAX_ARGV_ENTRIES) + if (crashThreadArg != nullptr && argc + 3 < MAX_ARGV_ENTRIES) // Need room for --crashthread, crashThreadArg, and null terminator { argv[argc++] = "--crashthread"; argv[argc++] = crashThreadArg; @@ -2805,19 +2805,19 @@ PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo, void* context, bool if (siginfo != nullptr) { signalCodeArg = PROCFormatInt(siginfo->si_code); - if (signalCodeArg != nullptr && argc + 1 < MAX_ARGV_ENTRIES) + if (signalCodeArg != nullptr && argc + 3 < MAX_ARGV_ENTRIES) // Need room for --code, signalCodeArg, and null terminator { argv[argc++] = "--code"; argv[argc++] = signalCodeArg; } signalErrnoArg = PROCFormatInt(siginfo->si_errno); - if (signalErrnoArg != nullptr && argc + 1 < MAX_ARGV_ENTRIES) + if (signalErrnoArg != nullptr && argc + 3 < MAX_ARGV_ENTRIES) // Need room for --errno, signalErrnoArg, and null terminator { argv[argc++] = "--errno"; argv[argc++] = signalErrnoArg; } signalAddressArg = PROCFormatInt64((ULONG64)siginfo->si_addr); - if (signalAddressArg != nullptr && argc + 1 < MAX_ARGV_ENTRIES) + if (signalAddressArg != nullptr && argc + 3 < MAX_ARGV_ENTRIES) // Need room for --address, signalAddressArg, and null terminator { argv[argc++] = "--address"; argv[argc++] = signalAddressArg; From 250256d379e1e3f185ff70c7e5215bc32bee5df2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 11 Jan 2026 16:33:27 +0000 Subject: [PATCH 06/13] Replace bounds checks with asserts per code review feedback Changed runtime bounds checks to _ASSERTE calls since MAX_ARGV_ENTRIES (32) is large enough that these conditions should never be true in practice. This matches the pattern used in NativeAOT's PalCreateDump.cpp. Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com> --- src/coreclr/pal/src/thread/process.cpp | 38 ++++++++------------------ 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/src/coreclr/pal/src/thread/process.cpp b/src/coreclr/pal/src/thread/process.cpp index b10afd0df470c9..676b8c0be7152f 100644 --- a/src/coreclr/pal/src/thread/process.cpp +++ b/src/coreclr/pal/src/thread/process.cpp @@ -2325,12 +2325,10 @@ PROCBuildCreateDumpCommandLine( } int argc = 0; - if (argc + 1 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for program and null terminator argv[argc++] = program; if (dumpName != nullptr) { - if (argc + 3 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --name, dumpName, and null terminator argv[argc++] = "--name"; argv[argc++] = dumpName; } @@ -2338,19 +2336,15 @@ PROCBuildCreateDumpCommandLine( switch (dumpType) { case DumpTypeNormal: - if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --normal and null terminator argv[argc++] = "--normal"; break; case DumpTypeWithHeap: - if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --withheap and null terminator argv[argc++] = "--withheap"; break; case DumpTypeTriage: - if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --triage and null terminator argv[argc++] = "--triage"; break; case DumpTypeFull: - if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --full and null terminator argv[argc++] = "--full"; break; default: @@ -2359,45 +2353,40 @@ PROCBuildCreateDumpCommandLine( if (flags & GenerateDumpFlagsLoggingEnabled) { - if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --diag and null terminator argv[argc++] = "--diag"; } if (flags & GenerateDumpFlagsVerboseLoggingEnabled) { - if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --verbose and null terminator argv[argc++] = "--verbose"; } if (flags & GenerateDumpFlagsCrashReportEnabled) { - if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --crashreport and null terminator argv[argc++] = "--crashreport"; } if (flags & GenerateDumpFlagsCrashReportOnlyEnabled) { - if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --crashreportonly and null terminator argv[argc++] = "--crashreportonly"; } if (g_running_in_exe) { - if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --singlefile and null terminator argv[argc++] = "--singlefile"; } if (logFileName != nullptr) { - if (argc + 3 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for --logtofile, logFileName, and null terminator argv[argc++] = "--logtofile"; argv[argc++] = logFileName; } - if (argc + 2 >= MAX_ARGV_ENTRIES) return FALSE; // Need room for pidarg and null terminator argv[argc++] = *ppidarg; argv[argc++] = nullptr; + _ASSERTE(argc < MAX_ARGV_ENTRIES); + return TRUE; } @@ -2784,11 +2773,11 @@ PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo, void* context, bool argv[argc] = g_argvCreateDump[argc]; } - if (signal != 0 && argc + 3 < MAX_ARGV_ENTRIES) // Need room for at least --signal, signalArg, and null terminator + if (signal != 0 && argc < MAX_ARGV_ENTRIES) { // Add the signal number to the command line signalArg = PROCFormatInt(signal); - if (signalArg != nullptr && argc + 3 < MAX_ARGV_ENTRIES) // Need room for --signal, signalArg, and null terminator + if (signalArg != nullptr) { argv[argc++] = "--signal"; argv[argc++] = signalArg; @@ -2796,7 +2785,7 @@ PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo, void* context, bool // Add the current thread id to the command line. This function is always called on the crashing thread. crashThreadArg = PROCFormatInt(THREADSilentGetCurrentThreadId()); - if (crashThreadArg != nullptr && argc + 3 < MAX_ARGV_ENTRIES) // Need room for --crashthread, crashThreadArg, and null terminator + if (crashThreadArg != nullptr) { argv[argc++] = "--crashthread"; argv[argc++] = crashThreadArg; @@ -2805,36 +2794,31 @@ PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo, void* context, bool if (siginfo != nullptr) { signalCodeArg = PROCFormatInt(siginfo->si_code); - if (signalCodeArg != nullptr && argc + 3 < MAX_ARGV_ENTRIES) // Need room for --code, signalCodeArg, and null terminator + if (signalCodeArg != nullptr) { argv[argc++] = "--code"; argv[argc++] = signalCodeArg; } signalErrnoArg = PROCFormatInt(siginfo->si_errno); - if (signalErrnoArg != nullptr && argc + 3 < MAX_ARGV_ENTRIES) // Need room for --errno, signalErrnoArg, and null terminator + if (signalErrnoArg != nullptr) { argv[argc++] = "--errno"; argv[argc++] = signalErrnoArg; } signalAddressArg = PROCFormatInt64((ULONG64)siginfo->si_addr); - if (signalAddressArg != nullptr && argc + 3 < MAX_ARGV_ENTRIES) // Need room for --address, signalAddressArg, and null terminator + if (signalAddressArg != nullptr) { argv[argc++] = "--address"; argv[argc++] = signalAddressArg; } } - if (argc < MAX_ARGV_ENTRIES) - { - argv[argc++] = nullptr; - } + argv[argc++] = nullptr; + _ASSERTE(argc < MAX_ARGV_ENTRIES); } else { - if (argc < MAX_ARGV_ENTRIES) - { - argv[argc] = nullptr; - } + argv[argc] = nullptr; } PROCCreateCrashDump(argv, nullptr, 0, serialize); From 6a232223dc446e1cdfea8c9ab1a6167e4b61667f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 12 Jan 2026 17:14:56 +0000 Subject: [PATCH 07/13] Deduplicate null terminator assignment per code review feedback Moved the `argv[argc] = nullptr;` assignment outside the if/else block to eliminate duplication, as suggested by @jkotas. Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com> --- src/coreclr/pal/src/thread/process.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/coreclr/pal/src/thread/process.cpp b/src/coreclr/pal/src/thread/process.cpp index 676b8c0be7152f..a202a43c10a498 100644 --- a/src/coreclr/pal/src/thread/process.cpp +++ b/src/coreclr/pal/src/thread/process.cpp @@ -2812,15 +2812,11 @@ PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo, void* context, bool argv[argc++] = signalAddressArg; } } - - argv[argc++] = nullptr; - _ASSERTE(argc < MAX_ARGV_ENTRIES); - } - else - { - argv[argc] = nullptr; } + argv[argc] = nullptr; + _ASSERTE(argc < MAX_ARGV_ENTRIES); + PROCCreateCrashDump(argv, nullptr, 0, serialize); free(signalArg); From 46c40ec5af6107a1dc2ffafa13af241158b4315b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 14 Jan 2026 03:12:00 +0000 Subject: [PATCH 08/13] Address code review feedback: Add bounds check for siginfo and align NativeAOT with assert pattern - Added bounds check `&& argc < MAX_ARGV_ENTRIES` before processing siginfo in process.cpp - Changed NativeAOT PalCreateDump.cpp to use assert instead of runtime check, consistent with the pattern established in earlier review feedback Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com> --- src/coreclr/nativeaot/Runtime/unix/PalCreateDump.cpp | 5 +---- src/coreclr/pal/src/thread/process.cpp | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/unix/PalCreateDump.cpp b/src/coreclr/nativeaot/Runtime/unix/PalCreateDump.cpp index 2e44a1c82ff4e8..ae025bba1ca0b4 100644 --- a/src/coreclr/nativeaot/Runtime/unix/PalCreateDump.cpp +++ b/src/coreclr/nativeaot/Runtime/unix/PalCreateDump.cpp @@ -191,10 +191,7 @@ BuildCreateDumpCommandLine( argv[argc++] = g_ppidarg; argv[argc++] = nullptr; - if (argc >= MAX_ARGV_ENTRIES) - { - return false; - } + assert(argc < MAX_ARGV_ENTRIES); return true; } diff --git a/src/coreclr/pal/src/thread/process.cpp b/src/coreclr/pal/src/thread/process.cpp index a202a43c10a498..73d4aeeb6059c1 100644 --- a/src/coreclr/pal/src/thread/process.cpp +++ b/src/coreclr/pal/src/thread/process.cpp @@ -2791,7 +2791,7 @@ PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo, void* context, bool argv[argc++] = crashThreadArg; } - if (siginfo != nullptr) + if (siginfo != nullptr && argc < MAX_ARGV_ENTRIES) { signalCodeArg = PROCFormatInt(siginfo->si_code); if (signalCodeArg != nullptr) From 04e424e0cf2324f6c26c46daf76a710c3d0cff29 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 13 Jan 2026 19:20:04 -0800 Subject: [PATCH 09/13] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/coreclr/pal/src/thread/process.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/coreclr/pal/src/thread/process.cpp b/src/coreclr/pal/src/thread/process.cpp index 73d4aeeb6059c1..0be6b16171f527 100644 --- a/src/coreclr/pal/src/thread/process.cpp +++ b/src/coreclr/pal/src/thread/process.cpp @@ -2385,6 +2385,10 @@ PROCBuildCreateDumpCommandLine( argv[argc++] = *ppidarg; argv[argc++] = nullptr; + if (argc >= MAX_ARGV_ENTRIES) + { + return FALSE; + } _ASSERTE(argc < MAX_ARGV_ENTRIES); return TRUE; @@ -2773,7 +2777,7 @@ PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo, void* context, bool argv[argc] = g_argvCreateDump[argc]; } - if (signal != 0 && argc < MAX_ARGV_ENTRIES) + if (signal != 0) { // Add the signal number to the command line signalArg = PROCFormatInt(signal); From 26ae775eae25429ba5ce948d3ae18538891351d5 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 13 Jan 2026 19:22:42 -0800 Subject: [PATCH 10/13] Apply suggestions from code review --- src/coreclr/pal/src/thread/process.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/pal/src/thread/process.cpp b/src/coreclr/pal/src/thread/process.cpp index 0be6b16171f527..2690b3f3b5ee03 100644 --- a/src/coreclr/pal/src/thread/process.cpp +++ b/src/coreclr/pal/src/thread/process.cpp @@ -2795,7 +2795,7 @@ PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo, void* context, bool argv[argc++] = crashThreadArg; } - if (siginfo != nullptr && argc < MAX_ARGV_ENTRIES) + if (siginfo != nullptr) { signalCodeArg = PROCFormatInt(siginfo->si_code); if (signalCodeArg != nullptr) From b929d807450370c42794405bd74f3e214a940053 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 13 Jan 2026 19:23:35 -0800 Subject: [PATCH 11/13] Update src/coreclr/pal/src/thread/process.cpp --- src/coreclr/pal/src/thread/process.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/coreclr/pal/src/thread/process.cpp b/src/coreclr/pal/src/thread/process.cpp index 2690b3f3b5ee03..077ad4e78985ae 100644 --- a/src/coreclr/pal/src/thread/process.cpp +++ b/src/coreclr/pal/src/thread/process.cpp @@ -2385,10 +2385,6 @@ PROCBuildCreateDumpCommandLine( argv[argc++] = *ppidarg; argv[argc++] = nullptr; - if (argc >= MAX_ARGV_ENTRIES) - { - return FALSE; - } _ASSERTE(argc < MAX_ARGV_ENTRIES); return TRUE; From f989c88d28a671eff8b8e03947d958cc2318e066 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 13 Jan 2026 19:24:05 -0800 Subject: [PATCH 12/13] Update src/coreclr/pal/src/thread/process.cpp --- src/coreclr/pal/src/thread/process.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/pal/src/thread/process.cpp b/src/coreclr/pal/src/thread/process.cpp index 077ad4e78985ae..76a32cd347db56 100644 --- a/src/coreclr/pal/src/thread/process.cpp +++ b/src/coreclr/pal/src/thread/process.cpp @@ -2383,7 +2383,7 @@ PROCBuildCreateDumpCommandLine( } argv[argc++] = *ppidarg; - argv[argc++] = nullptr; + argv[argc] = nullptr; _ASSERTE(argc < MAX_ARGV_ENTRIES); From 5c20a379b9408418efc116077f73685e928cfa97 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 13 Jan 2026 19:24:41 -0800 Subject: [PATCH 13/13] Update src/coreclr/pal/src/thread/process.cpp --- src/coreclr/pal/src/thread/process.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/pal/src/thread/process.cpp b/src/coreclr/pal/src/thread/process.cpp index 76a32cd347db56..42897270a9456f 100644 --- a/src/coreclr/pal/src/thread/process.cpp +++ b/src/coreclr/pal/src/thread/process.cpp @@ -2383,8 +2383,8 @@ PROCBuildCreateDumpCommandLine( } argv[argc++] = *ppidarg; - argv[argc] = nullptr; + argv[argc] = nullptr; _ASSERTE(argc < MAX_ARGV_ENTRIES); return TRUE;