-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Add DOTNET_DbgCreateDumpToolPath to override createdump directory (NativeAOT only) #122989
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: steveisok <[email protected]>
Co-authored-by: steveisok <[email protected]>
Co-authored-by: steveisok <[email protected]>
| if (dladdr((void*)&PalCreateDumpInitialize, &info) == 0) | ||
| char* program = nullptr; | ||
|
|
||
| // Check if user provided a custom path to createdump tool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be supported in regular CoreCLR copy of this logic as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think adding the env var on CoreCLR might just create a can of worms we'd prefer not to open. Coreclr dump collection needs DAC to enumerate the right memory and the DAC<->runtime are tightly coupled. The env var makes it much easier for folks to redirect dump collection to an incompatible collector. For example in the CI scenario anyone redirecting create dump for NativeAOT apps might inadvertently be retargetting all their CoreCLR apps too without meaning to do that.
Do we have an existing naming convention to specify that an env var only applies to NativeAOT? I also think it would be OK if we left the name alone and just deliberately ignored the env var on CoreCLR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not aware of any nativeaot specific naming convention. If there isn't, I'd rather not start one and just document this does not apply to coreclr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot please adjust reading the DbgCreateDumpToolPath to only extract the path. The behavior to detect if the file exists changed in another commit. Additionally, DbgCreateDumpToolPath is the path to where the createdump tool lives, not the full path to an executable named something arbitrary. Please make sure to read the path from the DbgCreateDumpToolPath environment variable and use that to concat in the way we do it by default. Finally, please make sure to update the documentation to indicate DbgCreateDumpToolPath works with NativeAOT only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have an existing naming convention to specify that an env var only applies to NativeAOT
We explicitly do not have a naming convention like that. We want NativeAOT to be as similar as possible to regular CoreCLR.
NativeAOT is not the only form-factor with this problem. Regular single file apps have the same problem (there is no createdump and DAC library for single file apps either). We have "solved" the problem for single file apps by supporting full dumps only. If you set DOTNET_DbgEnableMiniDump=1 for single file apps and your app crashes, you will see "Only full dumps are supported by single file apps. Change the dump type to full (DOTNET_DbgMiniDumpType=4)". It would be nice to be consistent across all single-file form factors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot are you able to create a follow up github issue that tracks making DOTNET_DbgEnableMiniDump and DOTNET_DbgMiniDumpType consistent across all single-file form factors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have the capability to create GitHub issues directly. However, here's a suggested issue description you can use:
Title: Make DOTNET_DbgEnableMiniDump and DOTNET_DbgMiniDumpType consistent across all single-file form factors
Description:
Currently, different single-file form factors handle dump generation inconsistently:
- Regular single-file apps only support full dumps and show an error message if other dump types are requested
- NativeAOT apps now support custom createdump paths via DOTNET_DbgCreateDumpToolPath
We should standardize the behavior across all single-file form factors to provide a consistent developer experience.
Area: Diagnostics/Dump Generation
Related PR: #[this PR number]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to be consistent across all single-file form factors.
We could make NativeAOT less functional to increase the parity - require minidump type full and don't add this env var at all. Getting to higher levels of parity probably requires making the specialized dump collection logic an optional linkable component for NativeAOT and SingleFile which sounds out of scope for this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, we can leave this for a follow up
|
Tagging subscribers to this area: @steveisok, @dotnet/dotnet-diag |
…dbgcreatdumptoolpath
… validation - Changed DbgCreateDumpToolPath to accept a directory path instead of full executable path - The runtime now concatenates the directory with "createdump" like the default behavior - Removed stat() validation as it was removed in commit c507305 - Updated documentation to clarify this is NativeAOT only and the variable accepts a directory path - Updated usage example to reflect the directory path usage Co-authored-by: steveisok <[email protected]>
- Initialize buffer with '\0' before using strncat - Use proper buffer size calculations in strncat calls to prevent buffer overruns - Ensures safe string concatenation following C string best practices Co-authored-by: steveisok <[email protected]>
- Calculate exact buffer size based on whether slash is needed - Follow same pattern as default code path using strncpy and strncat - More readable and maintainable code structure Co-authored-by: steveisok <[email protected]>
Changed "ignored in CoreCLR" to "ignored otherwise" since NativeAOT uses a stripped-down version of CoreCLR, making the original wording ambiguous. Co-authored-by: steveisok <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds a new environment variable DOTNET_DbgCreateDumpToolPath to allow NativeAOT applications to specify a custom directory where the createdump tool is located. This addresses scenarios where createdump is not shipped with the NativeAOT runtime, enabling users to provide their own dump generation tool.
Changes:
- Modified
PalCreateDumpInitialize()to check forDOTNET_DbgCreateDumpToolPathenvironment variable and use it to locate createdump - Added if/else logic to support both custom path and default library-based path discovery
- Updated documentation to describe the new environment variable and its NativeAOT-only support
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/coreclr/nativeaot/Runtime/unix/PalCreateDump.cpp | Adds environment variable check and custom path handling for createdump tool location with proper memory management and path concatenation logic |
| docs/design/coreclr/botr/xplat-minidump-generation.md | Documents the new DOTNET_DbgCreateDumpToolPath environment variable with usage examples and NativeAOT-only clarification |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
| - `DOTNET_DbgEnableMiniDump`: if set to "1", enables this core dump generation. The default is NOT to generate a dump. | ||
| - `DOTNET_DbgMiniDumpType`: See below. Default is "2" MiniDumpWithPrivateReadWriteMemory. | ||
| - `DOTNET_DbgMiniDumpName`: if set, use as the template to create the dump path and file name. See "Dump name formatting" for how the dump name can be formatted. The default is _/tmp/coredump.%p_. | ||
| - `DOTNET_DbgCreateDumpToolPath`: **(NativeAOT only)** if set, specifies the directory path where the createdump tool is located. The runtime will look for the createdump binary in this directory. This is useful in scenarios where createdump is not shipped with the runtime and you need to "bring your own" dump generation tool. This environment variable is only supported in NativeAOT applications and ignored otherwise. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be documented int the official docs: t https://learn.microsoft.com/en-us/dotnet/core/diagnostics/collect-dumps-crash
(We may want to do delete the env variable documentation here and just link to the official docs.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The official docs should mention that this works in .NET 11+ only
Description
Adds
DOTNET_DbgCreateDumpToolPathenvironment variable to allow specifying a directory path where the createdump tool is located. In scenarios where createdump is not shipped with the NativeAOT runtime, users can now provide their own dump generation tool.Note: This environment variable is only supported in NativeAOT applications and is ignored otherwise (not in other runtime configurations).
Implementation
PalCreateDumpInitialize()to checkDOTNET_DbgCreateDumpToolPathfor a directory pathRhConfig::Environment::TryGetStringValue()Usage
The runtime will look for
/path/to/directory/createdumpCustomer Impact
Customers deploying NativeAOT in environments without createdump (custom distributions, minimal containers, embedded systems) currently cannot generate crash dumps. This prevents critical debugging in production scenarios.
Regression
No. This is a new opt-in feature with no impact on existing behavior. Only affects NativeAOT applications.
Testing
Risk
Low. New environment variable only used when explicitly set. Code path follows existing directory concatenation pattern used for default createdump lookup. No changes to existing dump generation logic. NativeAOT only, so no impact on other runtime configurations.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.