From 2193618f3524d6115693c8e710fffbdf9a68b533 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?John=20H=C3=B6gberg?= <john@erlang.org>
Date: Mon, 12 Aug 2019 15:09:17 +0200
Subject: erts: Fix crash in instrument:allocations/0-1

The current carrier list was read when the allocator wasn't locked,
crashing the emulator if a block scan raced with a carrier
allocation.
---
 erts/emulator/beam/erl_alloc_util.c | 69 ++++++++++++++++++++++---------------
 1 file changed, 42 insertions(+), 27 deletions(-)

diff --git a/erts/emulator/beam/erl_alloc_util.c b/erts/emulator/beam/erl_alloc_util.c
index 0be4562785..fa8d04f88c 100644
--- a/erts/emulator/beam/erl_alloc_util.c
+++ b/erts/emulator/beam/erl_alloc_util.c
@@ -6760,12 +6760,21 @@ static int blockscan_cpool_yielding(blockscan_t *state)
     return 0;
 }
 
-static int blockscan_yield_helper(blockscan_t *state,
-                                  int (*yielding_op)(blockscan_t*))
+/* */
+
+static int blockscan_finish(blockscan_t *state)
 {
-    /* Note that we don't check whether to abort here; only yielding_op knows
-     * whether the carrier is still in the list/pool. */
+    if (ERTS_PROC_IS_EXITING(state->process)) {
+        state->abort(state->user_data);
+        return 0;
+    }
 
+    state->current_op = blockscan_finish;
+
+    return state->finish(state->user_data);
+}
+
+static void blockscan_lock_helper(blockscan_t *state) {
     if ((state->allocator)->thread_safe) {
         /* Locked scans have to be as short as possible. */
         state->reductions = 1;
@@ -6774,34 +6783,18 @@ static int blockscan_yield_helper(blockscan_t *state,
     } else {
         state->reductions = BLOCKSCAN_REDUCTIONS;
     }
+}
 
-    if (yielding_op(state)) {
-        state->next_op = state->current_op;
-    }
-
+static void blockscan_unlock_helper(blockscan_t *state) {
     if ((state->allocator)->thread_safe) {
         erts_mtx_unlock(&(state->allocator)->mutex);
     }
-
-    return 1;
-}
-
-/* */
-
-static int blockscan_finish(blockscan_t *state)
-{
-    if (ERTS_PROC_IS_EXITING(state->process)) {
-        state->abort(state->user_data);
-        return 0;
-    }
-
-    state->current_op = blockscan_finish;
-
-    return state->finish(state->user_data);
 }
 
 static int blockscan_sweep_sbcs(blockscan_t *state)
 {
+    blockscan_lock_helper(state);
+
     if (state->current_op != blockscan_sweep_sbcs) {
         SET_CARRIER_HDR(&state->dummy_carrier, 0, SCH_SBC, state->allocator);
         state->current_clist = &(state->allocator)->sbc_list;
@@ -6811,11 +6804,19 @@ static int blockscan_sweep_sbcs(blockscan_t *state)
     state->current_op = blockscan_sweep_sbcs;
     state->next_op = blockscan_finish;
 
-    return blockscan_yield_helper(state, blockscan_clist_yielding);
+    if (blockscan_clist_yielding(state)) {
+        state->next_op = state->current_op;
+    }
+    
+    blockscan_unlock_helper(state);
+
+    return 1;
 }
 
 static int blockscan_sweep_mbcs(blockscan_t *state)
 {
+    blockscan_lock_helper(state);
+
     if (state->current_op != blockscan_sweep_mbcs) {
         SET_CARRIER_HDR(&state->dummy_carrier, 0, SCH_MBC, state->allocator);
         state->current_clist = &(state->allocator)->mbc_list;
@@ -6825,11 +6826,19 @@ static int blockscan_sweep_mbcs(blockscan_t *state)
     state->current_op = blockscan_sweep_mbcs;
     state->next_op = blockscan_sweep_sbcs;
 
-    return blockscan_yield_helper(state, blockscan_clist_yielding);
+    if (blockscan_clist_yielding(state)) {
+        state->next_op = state->current_op;
+    }
+
+    blockscan_unlock_helper(state);
+
+    return 1;
 }
 
 static int blockscan_sweep_cpool(blockscan_t *state)
 {
+    blockscan_lock_helper(state);
+
     if (state->current_op != blockscan_sweep_cpool) {
         ErtsAlcCPoolData_t *sentinel;
 
@@ -6841,7 +6850,13 @@ static int blockscan_sweep_cpool(blockscan_t *state)
     state->current_op = blockscan_sweep_cpool;
     state->next_op = blockscan_sweep_mbcs;
 
-    return blockscan_yield_helper(state, blockscan_cpool_yielding);
+    if (blockscan_cpool_yielding(state)) {
+        state->next_op = state->current_op;
+    }
+
+    blockscan_unlock_helper(state);
+
+    return 1;
 }
 
 static int blockscan_get_specific_allocator(int allocator_num,
-- 
cgit v1.2.3