Skip to content

[release/7.0] Enable logging managed stack trace for AV to event log - #76428

Merged
carlossanlop merged 1 commit into
release/7.0from
backport/pr-75721-to-release/7.0
Sep 30, 2022
Merged

[release/7.0] Enable logging managed stack trace for AV to event log#76428
carlossanlop merged 1 commit into
release/7.0from
backport/pr-75721-to-release/7.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Sep 30, 2022

Copy link
Copy Markdown
Contributor

Backport of #75721 to release/7.0

/cc @janvorli

Customer Impact

.NET Framework was logging managed stack trace of access violations that happened in external native code in the event log. .NET core only logs the address and error code of the exception, which makes it difficult for developers to figure out which part of their managed code has called the failing native code on their customer's machines.

Testing

Local directed test, coreclr and libraries tests.

Risk

Very low, the change affects only the final part of fatal error process exit where we are enabling logging of the stack trace for one more case.

.NET Framework was logging managed stack trace of access violations that
happened in external native code in the event log. .NET core only logs
the address and error code of the exception, which makes it difficult
for developers to figure out which part of their managed code has called
the failing native code.
The reason why .NET core doesn't print the stack trace is that the
access violation is now handled as fail fast instead of regular
unhandled exception. And while we report managed stack traces in the
EEPolicy::FatalError for fail fasts called from our runtime and managed
code in both runtime and user code, we don't report it when we come to
that method due to the access violation.
This change enables printing the stack trace for that case too.
@ghostghost added the area-VM-coreclr label Sep 30, 2022
@janvorlijanvorli self-assigned this Sep 30, 2022
@janvorlijanvorli added this to the 7.0.0 milestone Sep 30, 2022
@janvorlijanvorli added the Servicing-consider Issue for next servicing release review label Sep 30, 2022

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

approved. this is good for overall diagnostics. we will take for consideration in 7 ga.

@jeffschwMSFTjeffschwMSFT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Sep 30, 2022
@carlossanlop

Copy link
Copy Markdown
Contributor

Approved, signed off, CI is green. Ready to merge. :shipit:

@carlossanlop
carlossanlop merged commit cb09ca7 into release/7.0Sep 30, 2022
@carlossanlop
carlossanlop deleted the backport/pr-75721-to-release/7.0 branch September 30, 2022 21:30
@ghostghost locked as resolved and limited conversation to collaborators Oct 31, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-VM-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@carlossanlop@jkotas@jeffschwMSFT@janvorli