Skip to content

[release/7.0] Fix FailFast logging message #75718

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

Merged
merged 1 commit into from
Sep 19, 2022

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Sep 15, 2022

Backport of #75685 to release/7.0

/cc @eduardo-vp

Customer Impact

When the customer's app crashes due to an exception like access violation in an external native code, the customer doesn't get the crash address and error code in the event log entry (there is an x instead due to a bug). This change is a regression from .NET 6.

Testing

A function in an external native code file with a memory access crash was used by a managed app. In the Event Viewer app it could be found the logs with the correct information.

Risk

Very low, this change only involves formatting character for the FormatMessage Windows API from a wrong one that doesn't format the values to a correct one and adds 0x prefix.

@ghost
Copy link

ghost commented Sep 15, 2022

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@eduardo-vp eduardo-vp added the Servicing-consider Issue for next servicing release review label Sep 15, 2022
@carlossanlop
Copy link
Contributor

@eduardo-vp what branch are you trying to target? Do you want this change backported to RC2 or to GA?

  • If GA, then this branch is fine.
  • But if you want the change to go to RC2, you need to close this and resubmit the /backport command but targeting the release/7.0-rc2 branch.

@janvorli
Copy link
Member

@carlossanlop I think that GA is ok for this.

@carlossanlop
Copy link
Contributor

Thanks @janvorli for the confirmation. Since this is GA, can one of you please fill the template, then send an email to Tactics for approval?

@janvorli
Copy link
Member

Yes, @eduardo-vp will do that.

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

approved. we will take for consideration in RC2. Can you take a look at the ci failures and get a code review?

@carlossanlop carlossanlop added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Sep 16, 2022
@carlossanlop
Copy link
Contributor

Approved via email by Tactics.

@carlossanlop
Copy link
Contributor

CI green. Approved and signed off. Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit 5d79822 into release/7.0 Sep 19, 2022
@carlossanlop carlossanlop deleted the backport/pr-75685-to-release/7.0 branch September 19, 2022 19:05
@ghost ghost locked as resolved and limited conversation to collaborators Oct 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-VM-coreclr Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants