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.

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

Delete StructLayoutAttribute.Pack adjustment #64965

Merged
merged 1 commit into from
Feb 8, 2022
Merged

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Feb 8, 2022

This adjustment was trying to minic adjustment performed by type loader, but the logic has been out-of-sync with the type loader evolution for years.
It does not make sense for custom attribute reading via reflection to perform this adjustment.

Fixes #12480

This adjustment was trying to minic adjustment performed by type loader, but the logic has been out-of-sync with the type loader evolution for years.
It does not make sense for custom attribute reading via reflection to perform this adjustment.

Fixes dotnet#12480
@ghost
Copy link

ghost commented Feb 8, 2022

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Issue Details

This adjustment was trying to minic adjustment performed by type loader, but the logic has been out-of-sync with the type loader evolution for years.
It does not make sense for custom attribute reading via reflection to perform this adjustment.

Fixes #12480

Author: jkotas
Assignees: -
Labels:

area-System.Reflection

Milestone: -

@jkotas jkotas requested a review from buyaa-n February 8, 2022 05:25
@am11
Copy link
Member

am11 commented Feb 8, 2022

The nativeaot failure (seems unrelated but) is interesting one:

C:\h\w\A86E08E3\w\B33F0964\e>System.Runtime.Tests.exe -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing
Process terminated. Access Violation: Attempted to read or write protected memory. This is often an indication that other memory is corrupt. The application will be terminated since this platform does not support throwing an AccessViolationException.

should it run category=OuterLoop test with standard PR run?

@jkotas
Copy link
Member Author

jkotas commented Feb 8, 2022

The nativeaot failure (seems unrelated but) is interesting one:

This looks similar to #64060. We need to get crash dumps to root cause it. I believe @MichalStrehovsky has been working towards it.

@MichalStrehovsky
Copy link
Member

should it run category=OuterLoop test with standard PR run?

That's fine, the test runner in use for single file and NativeAOT testing ignores all command line arguments :).

This looks similar to #64060. We need to get crash dumps to root cause it. I believe @MichalStrehovsky has been working towards it.

I thought running this test as a Helix workitem would already collect the dump. I'll need to find where...

@ghost ghost locked as resolved and limited conversation to collaborators Mar 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PseudoCustomAttribute.GetStructLayoutCustomAttribute returns incorrect data
4 participants