[SGen/Tarjan] Handle edge case with node heaviness changing due to deduplication

[filipnavara]

This code path can introduce duplicates into the color->other_colors array:

runtime/src/mono/mono/metadata/sgen-tarjan-bridge.c

Lines 795 to 798 in b658cd2

	// Maybe we should make sure we are not adding duplicates here. It is not really a problem
	// since we will get rid of duplicates before submitting the SCCs to the client in gather_xrefs
	if (color_data)
	add_other_colors (color_data, &other->xrefs);

Then we get to the code that counts xref_count:

runtime/src/mono/mono/metadata/sgen-tarjan-bridge.c

Lines 1101 to 1114 in b658cd2

	// Eliminate non-visible SCCs from the SCC list and redistribute xrefs
	for (cur = root_color_bucket; cur; cur = cur->next) {
	ColorData *cd;
	for (cd = &cur->data [0]; cd < cur->next_data; ++cd) {
	if (!color_visible_to_client (cd))
	continue;
	color_merge_array_empty ();
	gather_xrefs (cd);
	reset_xrefs (cd);
	dyn_array_ptr_set_all (&cd->other_colors, &color_merge_array);
	xref_count += dyn_array_ptr_size (&cd->other_colors);
	}
	}

It uses the color_visible_to_client helper method which in turn calls bridgeless_color_is_heavy:

runtime/src/mono/mono/metadata/sgen-tarjan-bridge.c

Lines 131 to 139 in b658cd2

	static gboolean
	bridgeless_color_is_heavy (ColorData *data) {
	if (disable_non_bridge_scc)
	return FALSE;
	int fanin = data->incoming_colors;
	int fanout = dyn_array_ptr_size (&data->other_colors);
	return fanin > HEAVY_REFS_MIN && fanout > HEAVY_REFS_MIN
	&& fanin*fanout >= HEAVY_COMBINED_REFS_MIN;
	}

The fanout metric is computed from the count of other_colors which contain duplicates. Once the references are gathered with gather_xrefs they get deduplicated. This can change the number of fanout nodes just enough that it no longer satisfies the "heavy" property. We still increase the xref_count but in the second loop that iterates over the same data it will get skipped and result in a mismatch:

runtime/src/mono/mono/metadata/sgen-tarjan-bridge.c

Lines 1126 to 1130 in b658cd2

	for (cur = root_color_bucket; cur; cur = cur->next) {
	ColorData *src;
	for (src = &cur->data [0]; src < cur->next_data; ++src) {
	if (!color_visible_to_client (src))
	continue;

Repro: https://github.com/user-attachments/files/19044193/Archive.zip

Fixes #106410

[filipnavara]

FYI @AaronRobinsonMSFT

[filipnavara]

The fix is incomplete, we would likely report incorrect SCCs now instead of hitting the assert, and previous XREFs would likely be wrong too.

I'm keeping this as draft until I come up with a fix.

[filipnavara]

The fix needs to be done differently. We basically have three options:

1. Get rid of the duplicates early. This may be not be too costly if done right.
2. Lock it the heaviness at certain point and report the nodes anyway even if they are not really heavy in the end.
3. Do the deduplication late, possibly at the beginning of processing_build_callback_data.