Skip to content

Comments

Update a few stresslogs for the frame identifiers#112555

Merged
jkotas merged 2 commits intodotnet:mainfrom
jkotas:frame-identifier
Feb 19, 2025
Merged

Update a few stresslogs for the frame identifiers#112555
jkotas merged 2 commits intodotnet:mainfrom
jkotas:frame-identifier

Conversation

@jkotas
Copy link
Member

@jkotas jkotas commented Feb 14, 2025

No description provided.

Copilot AI review requested due to automatic review settings February 14, 2025 06:00
Copilot AI reviewed Feb 14, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.

Files not reviewed (3)
  • src/coreclr/vm/gcenv.ee.common.cpp: Language not supported
  • src/coreclr/vm/i386/excepx86.cpp: Language not supported
  • src/coreclr/vm/threadsuspend.cpp: Language not supported

@ghost ghost added the area-VM-coreclr label Feb 14, 2025
@jkotas jkotas requested a review from davidwrighton February 14, 2025 06:00
@jkotas jkotas requested a review from max-charlamb February 14, 2025 06:00
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Feb 14, 2025

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

@max-charlamb
Copy link
Member

max-charlamb commented Feb 14, 2025

Is it possible to use the same list of strings for the stresslog and DAC FrameNames?

Currently the runtime GetFrameTypeName returns a char* string as opposed to the DAC WCHAR string, but this could be converted before returning to the DAC.

jkotas
jkotas commented Feb 18, 2025
View reviewed changes
src/coreclr/debug/daccess/request.cpp
unsigned truncatedLength = min(len, count - 1);
for (unsigned i = 0; i < truncatedLength; i++)
{
frameName[i] = pszName[i];
Copy link
Member Author

@jkotas jkotas Feb 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not able to find a reasonable conversion method that is available in the DAC and that has the right truncate semantics, so I have just open-coded it.

@jkotas
Copy link
Member Author

jkotas commented Feb 18, 2025

Is it possible to use the same list of strings for the stresslog and DAC FrameNames?

Fixed

@build-analysis build-analysis bot mentioned this pull request Feb 19, 2025
Closed
max-charlamb
max-charlamb approved these changes Feb 19, 2025
View reviewed changes
Copy link
Member

@max-charlamb max-charlamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I verified that the DAC strings work correctly.

lgtm

@jkotas jkotas merged commit 5587e2f into dotnet:main Feb 19, 2025
92 of 94 checks passed
@jkotas jkotas deleted the frame-identifier branch February 19, 2025 17:17
grendello pushed a commit to grendello/runtime that referenced this pull request Feb 19, 2025
@jkotas @grendello
Update a few stresslogs for the frame identifiers (dotnet#112555)
863b3d1
@github-actions github-actions bot locked and limited conversation to collaborators Mar 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Copilot left review comments

@max-charlamb max-charlamb max-charlamb approved these changes

@davidwrighton davidwrighton Awaiting requested review from davidwrighton

Assignees

@jkotas jkotas

Labels

area-VM-coreclr

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jkotas @max-charlamb