Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Delete FEATURE_CORRUPTING_EXCEPTIONS #35938

Merged
merged 5 commits into from
May 7, 2020
Merged

Delete FEATURE_CORRUPTING_EXCEPTIONS #35938

merged 5 commits into from
May 7, 2020

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented May 7, 2020

No description provided.

@jkotas jkotas requested a review from janvorli May 7, 2020 04:21
jkotas added 3 commits May 6, 2020 21:21
@jkotas
Delete HCORMODULE
3b924a8
@jkotas
Delete FEATURE_CORRUPTING_EXCEPTIONS
e4348f4 
Ability to catch corrupting exceptions was disabled in .NET Core since early days for security reasons. This is still disabled after this change, but achieved in much simpler way without the full corrupting exceptions machinery.
@jkotas
Delete unused bSecurityCheck argument of GetComIPFromObjectRef
0bf8a16
janvorli
janvorli approved these changes May 7, 2020
View reviewed changes
Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM modulo the nits. Very nice cleanup, I would not have expected so much stuff to go away.

src/coreclr/src/vm/commodule.cpp Outdated Show resolved Hide resolved
src/coreclr/src/vm/excep.cpp
STATIC_CONTRACT_GC_TRIGGERS;
STATIC_CONTRACT_MODE_COOPERATIVE;

LOG((LF_EH, LL_INFO100, "RealCOMPlusThrow throwing %s\n",
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be good to preserve this LOG (move it to the RealCOMPlusThrow)?

@jkotas @janvorli
Update src/coreclr/src/vm/commodule.cpp
0179f41 
Co-authored-by: Jan Vorlicek <janvorli@microsoft.com>
@stephentoub
Copy link
Member

What did all this code do that's being removed? Is HandleProcessCorruptedStateExceptionsAttribute meaningful after this PR?

@jkotas
Copy link
Member Author

jkotas commented May 7, 2020

HandleProcessCorruptedStateExceptionsAttribute was not meaningful even before this change. #31157 tracks making it obsolete.

It was disabled in #7272, without the code being actually removed.

@stephentoub
Copy link
Member

It was disabled in #7272, without the code being actually removed.

Alrighty then :)

@jkotas jkotas merged commit 5178041 into dotnet:master May 7, 2020
@jkotas jkotas deleted the corrupting-exceptions branch May 7, 2020 18:59
@ericwj ericwj mentioned this pull request Jun 20, 2020
Open
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@janvorli janvorli janvorli approved these changes

Assignees
No one assigned
Labels
area-ExceptionHandling-coreclr
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jkotas @stephentoub @janvorli @Dotnet-GitSync-Bot