Skip to content

Commit 659321f

Browse files
filipnavaraBrzVlad
andauthored
Fix an edge case in the Tarjan GC bridge that leads to losing xref information (#112825)
* Fix an edge case in the Tarjan SCC that lead to losing xref information In the Tarjan SCC bridge processing there's a color graph used to find out connections between SCCs. There was a rare case which only manifested when a cycle in the object graph points to another cycle that points to a bridge object. We only recognized direct bridge pointers but not pointers to other non-bridge SCCs that in turn point to bridges and where we already calculated the xrefs. These xrefs were then lost. * Add test case to sgen-bridge-pathologies and add an assert to catch the original bug * Add review --------- Co-authored-by: Vlad Brezae <brezaevlad@gmail.com>
1 parent 76750df commit 659321f

File tree

2 files changed

+14
-3
lines changed

2 files changed

+14
-3
lines changed

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

+13-2
Original file line numberDiff line numberDiff line change
@@ -736,19 +736,27 @@ create_scc (ScanData *data)
736736
gboolean found = FALSE;
737737
gboolean found_bridge = FALSE;
738738
ColorData *color_data = NULL;
739+
gboolean can_reduce_color = TRUE;
739740

740741
for (i = dyn_array_ptr_size (&loop_stack) - 1; i >= 0; --i) {
741742
ScanData *other = (ScanData *)dyn_array_ptr_get (&loop_stack, i);
742743
found_bridge |= other->is_bridge;
744+
if (dyn_array_ptr_size (&other->xrefs) > 0) {
745+
// This scc will have more xrefs than the ones from the color_merge_array,
746+
// we will need to create a new color to store this information.
747+
can_reduce_color = FALSE;
748+
}
743749
if (found_bridge || other == data)
744750
break;
745751
}
746752

747753
if (found_bridge) {
748754
color_data = new_color (TRUE);
749755
++num_colors_with_bridges;
750-
} else {
756+
} else if (can_reduce_color) {
751757
color_data = reduce_color ();
758+
} else {
759+
color_data = new_color (FALSE);
752760
}
753761
#if DUMP_GRAPH
754762
printf ("|SCC %p rooted in %s (%p) has bridge %d\n", color_data, safe_name_bridge (data->obj), data->obj, found_bridge);
@@ -785,8 +793,11 @@ create_scc (ScanData *data)
785793

786794
// Maybe we should make sure we are not adding duplicates here. It is not really a problem
787795
// since we will get rid of duplicates before submitting the SCCs to the client in gather_xrefs
788-
if (color_data)
796+
if (color_data) {
789797
add_other_colors (color_data, &other->xrefs);
798+
} else {
799+
g_assert (dyn_array_ptr_size (&other->xrefs) == 0);
800+
}
790801
dyn_array_ptr_uninit (&other->xrefs);
791802

792803
if (other == data) {

src/tests/GC/Features/Bridge/Bridge.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,7 @@ public static int Main(string[] args)
385385

386386
RunGraphTest(SetupDeadList);
387387
RunGraphTest(SetupSelfLinks);
388-
// RunGraphTest(NestedCycles); // Fixed by Filip
388+
RunGraphTest(NestedCycles);
389389
// RunGraphTest(Spider); // Crashes
390390
return 100;
391391
}

0 commit comments

Comments
 (0)