Changes for doubly linked freelists in the 64-bit GC

[PeterSolMS]

The 64-bit implementation of GC now uses one more bit in the methodtable pointer, so SOS needs to mask out one more bit as well.

[noahfalk]

@PeterSolMS thanks! The change looked fine except there is a little extra work to warn customers using old versions of SOS that this is a breaking change. Our breaking change mechanism will give them some warning text that they need to upgrade. You would need to increment the breaking change number in the runtime and the corresponding value in SOS
Did the corresponding runtime changes already go in or is this still to come?

@mikem8361 - you want to take a look? It looked fine to me

[mikem8361]

This breaks backward compatibility with older runtimes. This code has to runtime aginst 2.1, 3.1 and 5.0 runtimes too. You can check the runtime version with the IsRuntimeVersionAtLeast() function.

[PeterSolMS]

The runtime change went in today.

I believe the new sos.dll will run just fine against older runtimes. Using the old sos.dll against the new runtime will work fine almost always as the new bit is only used rarely and for a short time during GC plan_phase.

I will look at the breaking change number mechanism...

[mikem8361]

It don't know the GC details so if this works with older runtimes then we are good. Bumping the breaking change number in the DAC would be good though.

[Maoni0]

I'm kind of puzzled by the mechanism to indicate whether GC and sos are compatible in general. on the GC side we have this:

gcDacVars->major_version_number = 1;

and presumably we should bump up this number but I don't see this getting checked anywhere in the diagnostics repo...

[mikem8361]

I guess it slipped through the cracks when SOS moved to out of band in 3.0. If the GC data structures have changed in an incompatible way that isn't hidden by the DAC, then maybe this version field should be incremented and SOS be updated to check the version. But the more general DAC "breaking change" version update may be enough. SOS already checks it and displays an update message. I assume that on the runtime side it has been incremented.

[Maoni0]

can you please tell me where in sos it checks the version number of the runtime? or does it use a different mechanism?

[mikem8361]

We check the DAC breaking change version here:

diagnostics/src/SOS/Strike/util.cpp

Line 3530 in 925c979

if (g_sos != nullptr)

In the runtime, the DAC returns this version:
https://github.com/dotnet/runtime/blob/d2fc747f6d74dc0c4e4dd7b7749ff9a5fa5e11f4/src/coreclr/src/inc/sospriv.idl#L416

[Maoni0]

thanks @mikem8361!

@PeterSolMS we'll need to change the corresponding SOS_BREAKING_CHANGE_VERSION in the runtime repo as well. also after talking to Mike I realized that the gcDacVars->major_version_number/gcDacVars->major_version_number we set are not used 😞 however, it's not really worth to remove them either because that would change the offset of the other fields in this structure... so I would suggest that we add a comment above those lines that says "if you change the GC in a way that makes it no longer compatible with sos please change SOS_BREAKING_CHANGE_VERSION instead of these.

[PeterSolMS]

@Maoni0 thanks - yes, I'm aware of the need to change SOS_BREAKING_CHANGE_VERSION in the runtime repo - see PR dotnet/runtime#44800 - @noahfalk pointed this out in his comment above.

@mikem8361 could you have a look at this corresponding PR in the runtime repo as well? Thanks!

[Maoni0]

oops I didn't realized the runtime repo change was already there.. I'll take a look.