From 81f9ccdfa58512cee3dd4b4d4afb44384a22def8 Mon Sep 17 00:00:00 2001
From: Stefan Seifert <nine@detonation.org>
Date: Wed, 2 Oct 2019 12:51:35 +0200
Subject: [PATCH] Fix profiler's call graph node getting missed by the GC

When the NodeWorklist hit its allocated size, add_node would reallocate
the list but then not actually add the node to the resized list. This
made us miss this node during the GC's mark run and led to outdated
object pointers in the allocation list.

Fix by restructuring the code, so we reallocate first and then add the
item. I also removed the unused code for handling start > 0 as was an
optimization for a condition that was never actually implemented. This
code an the added complexity probably led to the oversight in the first
place.
---
 src/profiler/instrument.c | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/src/profiler/instrument.c b/src/profiler/instrument.c
index 18f04a4465..0fa1eab49b 100644
--- a/src/profiler/instrument.c
+++ b/src/profiler/instrument.c
@@ -2,26 +2,20 @@
 
 typedef struct {
     MVMuint32 items;
-    MVMuint32 start;
     MVMuint32 alloc;
 
     MVMProfileCallNode **list;
 } NodeWorklist;
 
 static void add_node(MVMThreadContext *tc, NodeWorklist *list, MVMProfileCallNode *node) {
-    if (list->start + list->items + 1 < list->alloc) {
-        /* Add at the end */
-        list->items++;
-        list->list[list->start + list->items] = node;
-    } else if (list->start > 0) {
-        /* End reached, add to the start now */
-        list->start--;
-        list->list[list->start] = node;
-    } else {
+    if (list->items == list->alloc) {
         /* Filled up the whole list. Make it bigger */
         list->alloc *= 2;
         list->list = MVM_realloc(list->list, list->alloc * sizeof(MVMProfileCallNode *));
     }
+    /* Add at the end */
+    list->list[list->items] = node;
+    list->items++;
 }
 
 static MVMProfileCallNode *take_node(MVMThreadContext *tc, NodeWorklist *list) {
@@ -29,13 +23,8 @@ static MVMProfileCallNode *take_node(MVMThreadContext *tc, NodeWorklist *list) {
     if (list->items == 0) {
         MVM_panic(1, "profiler: tried to take a node from an empty node worklist");
     }
-    if (list->start > 0) {
-        result = list->list[list->start];
-        list->start++;
-    } else {
-        result = list->list[list->start + list->items];
-        list->items--;
-    }
+    list->items--;
+    result = list->list[list->items];
     return result;
 }
 
@@ -846,7 +835,6 @@ void MVM_profile_instrumented_mark_data(MVMThreadContext *tc, MVMGCWorklist *wor
         /* Allocate our worklist on the stack. */
         NodeWorklist nodelist;
         nodelist.items = 0;
-        nodelist.start = 0;
         nodelist.alloc = 256;
         nodelist.list = MVM_malloc(nodelist.alloc * sizeof(MVMProfileCallNode *));