From 08081a577f2cc5752026357be219b286e8418918 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Wed, 25 Jan 2023 21:48:15 +0100 Subject: [PATCH 1/2] Fix GC interfaces versioning The change that introduced GC interfaces versioning had a bug preventing it from using .NET 8 GC being used with .NET 7 runtime. The GC_VersionInfo return value should have been the minimum supported version, not the current one. But more importantly there was also some confusion on what interface the GC_INTERFACE_MAJOR_VERSION represents. While the change considered it to be a version of the IGCToCLR interface, it really means the version of the IGCHeap interface. This change creates a separate version, EE_INTERFACE_MAJOR_VERSION for versioning o the IGCToCLR interface to rectify that. @Maoni0 also didn't like the way of creating a new version of IGCToCLR interface for each major version change, so I am changing it to single IGCToCLR interface. --- src/coreclr/gc/gccommon.cpp | 2 +- src/coreclr/gc/gcenv.ee.standalone.inl | 4 ++-- src/coreclr/gc/gcinterface.ee.h | 5 +---- src/coreclr/gc/gcinterface.h | 12 ++++++------ src/coreclr/gc/gcload.cpp | 2 +- src/coreclr/vm/gcheaputilities.cpp | 4 ++-- 6 files changed, 13 insertions(+), 16 deletions(-) diff --git a/src/coreclr/gc/gccommon.cpp b/src/coreclr/gc/gccommon.cpp index 2dca1e19281eab..413075246fd5c4 100644 --- a/src/coreclr/gc/gccommon.cpp +++ b/src/coreclr/gc/gccommon.cpp @@ -17,7 +17,7 @@ IGCHeapInternal* g_theGCHeap; IGCHandleManager* g_theGCHandleManager; #ifdef BUILD_AS_STANDALONE -IGCToCLR2* g_theGCToCLR; +IGCToCLR* g_theGCToCLR; VersionInfo g_runtimeSupportedVersion; #endif // BUILD_AS_STANDALONE diff --git a/src/coreclr/gc/gcenv.ee.standalone.inl b/src/coreclr/gc/gcenv.ee.standalone.inl index 78bf2ee7daff00..8729d5173c2956 100644 --- a/src/coreclr/gc/gcenv.ee.standalone.inl +++ b/src/coreclr/gc/gcenv.ee.standalone.inl @@ -9,7 +9,7 @@ // The singular interface instance. All calls in GCToEEInterface // will be forwarded to this interface instance. -extern IGCToCLR2* g_theGCToCLR; +extern IGCToCLR* g_theGCToCLR; // GC version that the current runtime supports extern VersionInfo g_runtimeSupportedVersion; @@ -316,7 +316,7 @@ inline void GCToEEInterface::DiagAddNewRegion(int generation, uint8_t* rangeStar inline void GCToEEInterface::LogErrorToHost(const char *message) { - if (g_runtimeSupportedVersion.MajorVersion >= GC_INTERFACE2_MAJOR_VERSION) + if (g_runtimeSupportedVersion.MajorVersion >= EE_INTERFACE_MAJOR_VERSION) { g_theGCToCLR->LogErrorToHost(message); } diff --git a/src/coreclr/gc/gcinterface.ee.h b/src/coreclr/gc/gcinterface.ee.h index e1a53bd244d240..5a67bf4e0dd7e7 100644 --- a/src/coreclr/gc/gcinterface.ee.h +++ b/src/coreclr/gc/gcinterface.ee.h @@ -446,11 +446,8 @@ class IGCToCLR { virtual void DiagAddNewRegion(int generation, uint8_t* rangeStart, uint8_t* rangeEnd, uint8_t* rangeEndReserved) = 0; -}; - -class IGCToCLR2 : public IGCToCLR { -public: + // The following method is available only with EE_INTERFACE_MAJOR_VERSION >= 1 virtual void LogErrorToHost(const char *message) = 0; }; diff --git a/src/coreclr/gc/gcinterface.h b/src/coreclr/gc/gcinterface.h index cb706134679f02..112cf3a93acac1 100644 --- a/src/coreclr/gc/gcinterface.h +++ b/src/coreclr/gc/gcinterface.h @@ -4,18 +4,18 @@ #ifndef _GC_INTERFACE_H_ #define _GC_INTERFACE_H_ -// The major version of the GC/EE interface. Breaking changes to this interface +// The major version of the IGCHeap interface. Breaking changes to this interface // require bumps in the major version number. -#define GC_INTERFACE_MAJOR_VERSION 6 +#define GC_INTERFACE_MAJOR_VERSION 5 -// The minor version of the GC/EE interface. Non-breaking changes are required +// The minor version of the IGCHeap interface. Non-breaking changes are required // to bump the minor version number. GCs and EEs with minor version number -// mismatches can still interopate correctly, with some care. +// mismatches can still interoperate correctly, with some care. #define GC_INTERFACE_MINOR_VERSION 1 -// The major version of the GC/EE interface. Breaking changes to this interface +// The major version of the IGCToCLR interface. Breaking changes to this interface // require bumps in the major version number. -#define GC_INTERFACE2_MAJOR_VERSION 6 +#define EE_INTERFACE_MAJOR_VERSION 1 struct ScanContext; struct gc_alloc_context; diff --git a/src/coreclr/gc/gcload.cpp b/src/coreclr/gc/gcload.cpp index 20c469f2baf844..cd3a0b43a6d82d 100644 --- a/src/coreclr/gc/gcload.cpp +++ b/src/coreclr/gc/gcload.cpp @@ -75,7 +75,7 @@ GC_Initialize( #ifdef BUILD_AS_STANDALONE assert(clrToGC != nullptr); - g_theGCToCLR = (IGCToCLR2*)clrToGC; + g_theGCToCLR = clrToGC; #else UNREFERENCED_PARAMETER(clrToGC); assert(clrToGC == nullptr); diff --git a/src/coreclr/vm/gcheaputilities.cpp b/src/coreclr/vm/gcheaputilities.cpp index c7c9ff4f716d29..ab93ae5c5129fd 100644 --- a/src/coreclr/vm/gcheaputilities.cpp +++ b/src/coreclr/vm/gcheaputilities.cpp @@ -218,8 +218,8 @@ HRESULT LoadAndInitializeGC(LPCWSTR standaloneGcLocation) } g_gc_load_status = GC_LOAD_STATUS_GET_VERSIONINFO; - g_gc_version_info.MajorVersion = GC_INTERFACE_MAJOR_VERSION; - g_gc_version_info.MinorVersion = GC_INTERFACE_MINOR_VERSION; + g_gc_version_info.MajorVersion = EE_INTERFACE_MAJOR_VERSION; + g_gc_version_info.MinorVersion = 0; g_gc_version_info.BuildVersion = 0; versionInfo(&g_gc_version_info); g_gc_load_status = GC_LOAD_STATUS_CALL_VERSIONINFO; From 9f2fe091573c7b78ee78f3eeb1bdc983e5f29aad Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Wed, 25 Jan 2023 22:47:21 +0100 Subject: [PATCH 2/2] Fix version check at the call site of the LogErrorToHost --- src/coreclr/gc/gcenv.ee.standalone.inl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/gc/gcenv.ee.standalone.inl b/src/coreclr/gc/gcenv.ee.standalone.inl index 8729d5173c2956..a1478a116e7b72 100644 --- a/src/coreclr/gc/gcenv.ee.standalone.inl +++ b/src/coreclr/gc/gcenv.ee.standalone.inl @@ -316,7 +316,7 @@ inline void GCToEEInterface::DiagAddNewRegion(int generation, uint8_t* rangeStar inline void GCToEEInterface::LogErrorToHost(const char *message) { - if (g_runtimeSupportedVersion.MajorVersion >= EE_INTERFACE_MAJOR_VERSION) + if (g_runtimeSupportedVersion.MajorVersion >= 1) { g_theGCToCLR->LogErrorToHost(message); }