Skip to content

Conversation

@hoyosjs
Copy link
Member

@hoyosjs hoyosjs commented Jan 27, 2026

No description provided.

jkotas and others added 11 commits December 22, 2025 06:00
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 `&lt;`, `&gt;`, and `&amp;` 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>
@hoyosjs hoyosjs requested a review from a team as a code owner January 27, 2026 12:53
@hoyosjs hoyosjs enabled auto-merge January 27, 2026 12:53
@hoyosjs hoyosjs disabled auto-merge January 27, 2026 12:53
@hoyosjs hoyosjs enabled auto-merge January 27, 2026 12:54
@hoyosjs hoyosjs closed this 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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants