[mono][sgen] Make color visible to client permanent

A color (SCC) that isn't containing any bridge objects is made visible to client if xrefs_in * xrefs_out is greater than 60. Later on in bridge processing, we need to build the callback to pass for .net android. During this stage, we reduce from the full set of SCCs to SCCs that should be visible to client (containing bridge objects or satisfying the above condition). If an SCC has an xref to a color that is not visible to client, we need to do a recursive traversal to find all neighbors that are visible to client. The problem is that this process can end up making an SCC no longer visible to client, leading to inconsistencies in the computation. Consider a color(C1) that has a neighbor that is not visible to client(C2). In this final stage, we compute the neighbors of C1 by traversing recursively through the neighbors of C2. If C2 ends up pointing to colors that were already neighbors of C1, then, following this computation, C1 would end up with fewer xrefs_out, making the color no longer visible to client. This make future checks incorrect, resulting in building incorrect graph for client.

This scenario seems rare in practice, we should have gotten way more reports otherwise. We fix this by pinning the visible_to_client property for a color once it first satisfies it, so it will no longer matter how many actual xrefs the color has.

Author: BrzVlad

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

@@ -96,6 +96,11 @@ typedef struct {
 	// Count of colors that list this color in their other_colors
 	unsigned incoming_colors : INCOMING_COLORS_BITS;
 	unsigned visited : 1;
+	// color_visible_to_client for a ColorData* can change over the course of bridge processing which
+	// is problematic. We fix this by setting this flag when a color is detected as visible to client.
+	// Once the flag is set, the color is pinned to being visible to client, even though it could lose
+	// some xrefs, making it not satisfy the bridgeless_color_is_heavy condition.
+	unsigned visible_to_client : 1;
 } ColorData;
 
 // Represents one managed object. Equivalent of new/old bridge "HashEntry"
@@ -140,8 +145,17 @@ bridgeless_color_is_heavy (ColorData *data) {
 
 // Should color be made visible to client?
 static gboolean
-color_visible_to_client (ColorData *data) {
-	return dyn_array_ptr_size (&data->bridges) || bridgeless_color_is_heavy (data);
+color_visible_to_client (ColorData *data)
+{
+	if (data->visible_to_client)
+		return TRUE;
+
+	if (dyn_array_ptr_size (&data->bridges) || bridgeless_color_is_heavy (data)) {
+		data->visible_to_client = TRUE;
+		return TRUE;
+	} else {
+		return FALSE;
+	}
 }
 
 // Stacks of ScanData objects used for tarjan algorithm.