From ed25f0bd5a4fb936eddde080b90446e7d55afb2d Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Mon, 4 Dec 2023 14:00:00 -0500 Subject: [PATCH] Make env_clone compaction safe The original order of events is: 1. Allocate new_body. 2. Peform memcpy into new_body. 3. Create new_env using new_body. However, if GC compaction runs during step 3, then new_env would not have yet been created and objects on new_body could move but it would not be reference updated. This commit changes the order of the last two events. --- proc.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/proc.c b/proc.c index 475879f9d5..c770e79880 100644 --- a/proc.c +++ b/proc.c @@ -3419,9 +3419,15 @@ env_clone(const rb_env_t *env, const rb_cref_t *cref) } new_body = ALLOC_N(VALUE, env->env_size); - MEMCPY(new_body, env->env, VALUE, env->env_size); new_ep = &new_body[env->ep - env->env]; new_env = vm_env_new(new_ep, new_body, env->env_size, env->iseq); + + /* The memcpy has to happen after the vm_env_new because it can trigger a + * GC compaction which can move the objects in the env. */ + MEMCPY(new_body, env->env, VALUE, env->env_size); + /* VM_ENV_DATA_INDEX_ENV is set in vm_env_new but will get overwritten + * by the memcpy above. */ + new_ep[VM_ENV_DATA_INDEX_ENV] = (VALUE)new_env; RB_OBJ_WRITE(new_env, &new_ep[VM_ENV_DATA_INDEX_ME_CREF], (VALUE)cref); VM_ASSERT(VM_ENV_ESCAPED_P(new_ep)); return new_env;