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

Use native code slot for default interface methods #25770

Merged
merged 1 commit into from Aug 19, 2019
Merged

Conversation

@kouvel
Copy link
Member

kouvel commented Jul 18, 2019

  • So that MethodDesc::GetNativeCode() can retrieve the current native code entry point (instead of returning null as before), and code versioning can find a matching code version from the code start address
  • Interface methods currently require having a precode, so the "method entry point" can't be used to directly store the native code entry point
  • Reenabled a couple of default interface method tests under GCStress
  • Other small cleanup

Fixes https://github.com/dotnet/coreclr/issues/25690

@kouvel kouvel added this to the Future milestone Jul 18, 2019
@kouvel kouvel requested review from noahfalk, jkotas and MichalStrehovsky Jul 18, 2019
@kouvel kouvel self-assigned this Jul 18, 2019
@jkotas jkotas closed this Jul 18, 2019
@jkotas jkotas reopened this Jul 18, 2019
Copy link
Member

noahfalk left a comment

LGTM, sorry it took me a while to get to this one

@MichalStrehovsky

This comment has been minimized.

Copy link
Member

MichalStrehovsky commented Jul 22, 2019

Thank you!

- So that `MethodDesc::GetNativeCode()` can retrieve the current native code entry point (instead of returning null as before), and code versioning can find a matching code version from the code start address
- Interface methods currently require having a precode, so the "method entry point" can't be used to directly store the native code entry point
- Reenabled a couple of default interface method tests under GCStress
- Other small cleanup

Fixes https://github.com/dotnet/coreclr/issues/25690
@kouvel kouvel force-pushed the kouvel:CvFix branch from 5000d84 to 490cfde Jul 27, 2019
@MichalStrehovsky

This comment has been minimized.

Copy link
Member

MichalStrehovsky commented Aug 19, 2019

@kouvel Can this be merged? There's 3 more issues that are likely the same root cause (it's GCStress crashes in default interface tests, even though we haven't touched default interfaces in many months).

@kouvel

This comment has been minimized.

Copy link
Member Author

kouvel commented Aug 19, 2019

Thanks for rerunning the CI, I'll go ahead and merge

@kouvel kouvel merged commit 98bf56a into dotnet:master Aug 19, 2019
38 checks passed
38 checks passed
WIP Ready for review
Details
coreclr-ci Build #20190808.2 succeeded
Details
coreclr-ci (Build Linux arm checked) Build Linux arm checked succeeded
Details
coreclr-ci (Build Linux arm64 checked) Build Linux arm64 checked succeeded
Details
coreclr-ci (Build Linux arm64 release) Build Linux arm64 release succeeded
Details
coreclr-ci (Build Linux_musl x64 checked) Build Linux_musl x64 checked succeeded
Details
coreclr-ci (Build Linux_musl x64 release) Build Linux_musl x64 release succeeded
Details
coreclr-ci (Build Linux_rhel6 x64 release) Build Linux_rhel6 x64 release succeeded
Details
coreclr-ci (Build Windows_NT arm Checked) Build Windows_NT arm Checked succeeded
Details
coreclr-ci (Build Windows_NT arm64 Checked) Build Windows_NT arm64 Checked succeeded
Details
coreclr-ci (Build Windows_NT x64 Checked) Build Windows_NT x64 Checked succeeded
Details
coreclr-ci (Build Windows_NT x64 Debug) Build Windows_NT x64 Debug succeeded
Details
coreclr-ci (Build Windows_NT x86 Checked) Build Windows_NT x86 Checked succeeded
Details
coreclr-ci (Build Windows_NT x86 Debug) Build Windows_NT x86 Debug succeeded
Details
coreclr-ci (Formatting Linux x64) Formatting Linux x64 succeeded
Details
coreclr-ci (Test Pri0 CoreFX Linux x64 checked) Test Pri0 CoreFX Linux x64 checked succeeded
Details
coreclr-ci (Test Pri0 CoreFX Windows_NT x64 checked) Test Pri0 CoreFX Windows_NT x64 checked succeeded
Details
coreclr-ci (Test Pri0 Linux x64 checked) Test Pri0 Linux x64 checked succeeded
Details
coreclr-ci (Test Pri0 Linux_musl x64 release) Test Pri0 Linux_musl x64 release succeeded
Details
coreclr-ci (Test Pri0 OSX x64 checked) Test Pri0 OSX x64 checked succeeded
Details
coreclr-ci (Test Pri0 R2R Linux x64 checked) Test Pri0 R2R Linux x64 checked succeeded
Details
coreclr-ci (Test Pri0 R2R OSX x64 checked) Test Pri0 R2R OSX x64 checked succeeded
Details
coreclr-ci (Test Pri0 R2R Windows_NT x64 checked) Test Pri0 R2R Windows_NT x64 checked succeeded
Details
coreclr-ci (Test Pri0 R2R Windows_NT x86 checked) Test Pri0 R2R Windows_NT x86 checked succeeded
Details
coreclr-ci (Test Pri0 Windows_NT arm checked) Test Pri0 Windows_NT arm checked succeeded
Details
coreclr-ci (Test Pri0 Windows_NT arm64 checked) Test Pri0 Windows_NT arm64 checked succeeded
Details
coreclr-ci (Test Pri0 Windows_NT x64 checked) Test Pri0 Windows_NT x64 checked succeeded
Details
coreclr-ci (Test Pri0 Windows_NT x86 checked) Test Pri0 Windows_NT x86 checked succeeded
Details
coreclr-ci (Test crossgen-comparison Linux arm checked) Test crossgen-comparison Linux arm checked succeeded
Details
coreclr-ci (Test pri0 Linux arm checked) Test pri0 Linux arm checked succeeded
Details
coreclr-ci (Test pri0 Linux arm64 checked) Test pri0 Linux arm64 checked succeeded
Details
coreclr-ci (Test pri0 Linux_musl x64 checked) Test pri0 Linux_musl x64 checked succeeded
Details
coreclr-ci (build Linux x64 Checked) build Linux x64 Checked succeeded
Details
coreclr-ci (build OSX x64 Checked) build OSX x64 Checked succeeded
Details
coreclr-ci (build Windows_NT arm Release) build Windows_NT arm Release succeeded
Details
coreclr-ci (build Windows_NT arm64 Release) build Windows_NT arm64 Release succeeded
Details
coreclr-ci (build Windows_NT x64 Release) build Windows_NT x64 Release succeeded
Details
license/cla All CLA requirements met.
Details
@kouvel kouvel deleted the kouvel:CvFix branch Aug 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.