-
Notifications
You must be signed in to change notification settings - Fork 384
snap main branch for release #5695
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
Closed
Closed
+463
−429
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The makes !gcinfo work correctly against current main
Adds the missing Process `SetEnvironmentVariable` command section (0x0403) to `documentation/design-docs/ipc-protocol.md`. Fixes #5664.
…nabled (#5594) - [x] Create a DML escape function for WString in util.h/util.cpp - [x] Apply DML escaping to method names in PrintThread function in strike.cpp - [x] Apply DML escaping to frame names in PrintThread function in strike.cpp - [x] Add test case for async Main with angle brackets in method name - [x] Verify the fix handles <> characters correctly - [x] Rebase changes on current main branch and resolve all merge conflicts - [x] Move AsyncMain debuggee to correct directory location - [x] Enable DML in AsyncMain test with /d flag - [x] Fix DML escaping order (ampersand first) - [x] Revert test verification to check for rendered output - [x] Remove unused variable ## Summary Fixed the issue where `!clrstack` incorrectly trimmed method names containing angle brackets (e.g., `Program.<Main>` became `Program.`). The root cause was that when DML (Debugger Markup Language) was enabled, the `<>` characters were interpreted as DML tags instead of literal text. **Solution:** - Added `DmlEscape()` function to properly escape `<`, `>`, and `&` characters as `<`, `>`, and `&` respectively - Applied DML escaping to both method names and frame names in the `PrintThread` function when DML is enabled - Added test case with async Main method to verify the fix **Latest Updates:** - Fixed escaping order: ampersand (`&`) is now escaped FIRST to prevent double-escaping issues - Test verification checks for literal angle brackets `<Main>` as the debugger renders DML back to unescaped text before verification - Confirmed behavior through local testing with CDB and WinDBG - Removed unused `estimatedSize` variable from DmlEscape function **Files Changed:** - `src/SOS/Strike/util.h` - Added DmlEscape declaration - `src/SOS/Strike/util.cpp` - Implemented DmlEscape function with correct escaping order - `src/SOS/Strike/strike.cpp` - Applied escaping in PrintThread - `src/tests/SOS.UnitTests/Debuggees/AsyncMain/` - AsyncMain debuggee (in correct location) - `src/tests/SOS.UnitTests/Scripts/AsyncMain.script` - Test script verifying rendered DML output - `src/tests/SOS.UnitTests/SOS.cs` - Test registration <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>!clrstack reports incorrect method names when <> is encountered</issue_title> > <issue_description>When !clrstack encounters `Program.<Main>(System.String[])`, it trims the method name to: `Program.(System.String[])`. Both ClrMD and WinDbg (ICorDebug + IMetaDataReader) correctly return `Program.<Main>(System.String[])`. > > I verified this with the main branch. I think the issue is that `ClrStackImpl::PrintThread` does not DML escape the `MethodNameFromIP` string we pass to `TableOutput`. It's not clear whether we should be specifically escaping in PrintThread or if TableOutput should do that. > > To repro, create a .Net 8 app (preview), and add this code, and debug the resulting crash dump (or any async Main): > > ``` C# > DivideData data = new() > { > Numerator = 16, > Denominator = 0, > }; > > Console.WriteLine($"{data.Numerator}/{data.Denominator} = {await DivideAsync(data)}"); > > static async Task<int> DivideAsync(DivideData divData) > { > await Task.Delay(1000); > > return divData.Numerator / divData.Denominator; > } > > class DivideData > { > public int Numerator { get; set; } > public int Denominator { get; set; } > } > ``` > > (I'm working on other stuff at the moment, can't take this issue in the next two weeks. I was going to submit a quick fix, but there's not a globally defined DmlEscape.)</issue_description> > > <agent_instructions>In strike.cpp, the PrintThread function calls into MethodNameFromIP in order to supply what gets written out. Properly format the string it returns so that <> is not trimmed.</agent_instructions> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> Fixes #4314 <!-- START COPILOT CODING AGENT TIPS --> --- ✨ Let Copilot coding agent [set things up for you](https://github.com/dotnet/diagnostics/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: steveisok <471438+steveisok@users.noreply.github.com> Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com> Co-authored-by: noahfalk <6243776+noahfalk@users.noreply.github.com>
…ror messages (#5680) SOS commands like `!dumpheap -stat` throw identical error messages for two distinct failure modes: memory read failures vs. successfully reading a zero/invalid MethodTable. These require different diagnostic approaches. NOTE: LLDB reports missing memory as if it were zeroed memory so SOS is still going to claim corrupt method tables in place of missing memory there. Co-authored-by: noahfalk <6243776+noahfalk@users.noreply.github.com>
## Replace BaseString in util.h with std::string and std::basic_string This PR replaces the custom `BaseString` template class with standard STL string types as suggested in the PR review comment from PR #5594. ### Plan: - [x] Explore repository structure and understand BaseString usage - [x] Replace BaseString class template definition with std::string/std::basic_string - [x] Update typedef declarations (String -> std::string, WString -> std::basic_string<WCHAR>) - [x] Update function signatures using WString to use std::basic_string<WCHAR> - [x] Update function implementations to use std::string methods (c_str(), length(), etc.) - [x] Update string concatenation and operations to work with std::string - [x] Update the Format class conversion operators to work with std::string/std::basic_string<WCHAR> - [x] Build the code to verify compilation - [x] Address code review comments - [x] Run security checks - [x] Request code review ### Changes made: - Removed the custom `BaseString` template class from util.h (150+ lines of code) - Changed `String` typedef from `BaseString<char, strlen, strcpy_s>` to `std::string` - Changed `WString` typedef from `BaseString<WCHAR, _wcslen, wcscpy_s>` to `std::basic_string<WCHAR>` - Note: WCHAR is char16_t on Linux/PAL platforms, so WString is std::basic_string<char16_t> on Linux - Updated `GetLength()` calls to `length()` (std::string standard method) - Updated implicit conversions to use explicit `.c_str()` calls where needed - Updated string concatenation with Format objects to use explicit `WString()` constructor wrapping - **Required**: Unlike BaseString which had custom operator+ overloads, std::basic_string's operator+ doesn't trigger implicit conversions for user-defined types. The Format class has `operator WString()`, but it's not invoked when used with std::string's operator+. Therefore, explicit wrapping with `WString()` is necessary. - Updated TableOutput::WriteColumn methods to call `.c_str()` on string parameters - Used implicit conversion from ArrayHolder to const WCHAR* instead of explicit cast - When passing String objects to output functions (`ExtOut`, `DMLOut`, etc.), explicitly call `.c_str()` to convert to `PCSTR` - Applied consistently across all platform-specific sections (AMD64, X86, ARM, ARM64, RISCV64, LOONGARCH64) - Updated comments in Format class conversion operators to reflect the requirement for explicit String()/WString() wrapping when concatenating with Format objects ### Testing: - Clean rebuild successful on Linux x64 Debug configuration - No security vulnerabilities detected by CodeQL - All code compiles without warnings or errors <!-- START COPILOT CODING AGENT SUFFIX --> <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > Based on this comment: #5594 (comment), can you open a PR to replace BaseString in util.h with usages of stl::string </details> <!-- START COPILOT CODING AGENT TIPS --> --- 💡 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](https://gh.io/copilot-coding-agent-tips) in the docs. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
This pull request updates the following dependencies [marker]: <> (Begin:39a983c9-68a4-46ae-bbba-6795aab4810d) ## From https://github.com/dotnet/arcade - **Subscription**: [39a983c9-68a4-46ae-bbba-6795aab4810d](https://maestro.dot.net/subscriptions?search=39a983c9-68a4-46ae-bbba-6795aab4810d) - **Build**: [20260112.3](https://dev.azure.com/dnceng/internal/_build/results?buildId=2877128) ([296898](https://maestro.dot.net/channel/8394/github:dotnet:arcade/build/296898)) - **Date Produced**: January 12, 2026 1:42:08 PM UTC - **Commit**: [9f518f2be968c4c0102c2e3f8c793c5b7f28b731](dotnet/arcade@9f518f2) - **Branch**: [release/10.0](https://github.com/dotnet/arcade/tree/release/10.0) [DependencyUpdate]: <> (Begin) - **Dependency Updates**: - From [10.0.0-beta.25603.103 to 10.0.0-beta.26062.3][1] - Microsoft.DotNet.Arcade.Sdk - Microsoft.DotNet.CodeAnalysis [1]: dotnet/arcade@5ddd0dd...9f518f2 [DependencyUpdate]: <> (End) [marker]: <> (End:39a983c9-68a4-46ae-bbba-6795aab4810d) --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Juan Hoyos <19413848+hoyosjs@users.noreply.github.com>
[main] Update dependencies from microsoft/clrmd
This pull request updates the following dependencies [marker]: <> (Begin:39a983c9-68a4-46ae-bbba-6795aab4810d) ## From https://github.com/dotnet/arcade - **Subscription**: [39a983c9-68a4-46ae-bbba-6795aab4810d](https://maestro.dot.net/subscriptions?search=39a983c9-68a4-46ae-bbba-6795aab4810d) - **Build**: [20260116.3](https://dev.azure.com/dnceng/internal/_build/results?buildId=2881172) ([297689](https://maestro.dot.net/channel/8394/github:dotnet:arcade/build/297689)) - **Date Produced**: January 16, 2026 11:19:37 PM UTC - **Commit**: [af17297350d5e5357d2ab3d69369d2a58b8bc4ab](dotnet/arcade@af17297) - **Branch**: [release/10.0](https://github.com/dotnet/arcade/tree/release/10.0) [DependencyUpdate]: <> (Begin) - **Dependency Updates**: - From [10.0.0-beta.26062.3 to 10.0.0-beta.26066.3][1] - Microsoft.DotNet.Arcade.Sdk - Microsoft.DotNet.CodeAnalysis [1]: dotnet/arcade@9f518f2...af17297 [DependencyUpdate]: <> (End) [marker]: <> (End:39a983c9-68a4-46ae-bbba-6795aab4810d) Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
steveisok
approved these changes
Jan 27, 2026
auto-merge was automatically disabled
January 27, 2026 20:30
Pull request was closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.