File 1371-erts-Fix-db_match-flatmap-copy-bug.patch of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 1371-erts-Fix-db_match-flatmap-copy-bug.patch of Package erlang

From 2f55c5e5a342f519b448aa9c077db0562c211bbc Mon Sep 17 00:00:00 2001
From: Lukas Larsson <lukas@erlang.org>
Date: Mon, 2 Oct 2023 16:59:06 +0200
Subject: [PATCH 1/2] erts: Fix db_match flatmap copy bug

If the body of a matchspec would return a flatmap with
a variable ('$1', '$_' etc) as one of the keys and the
variable was not an immidiate, the key term would not
be copied to the receiving processes heap. This would
later corrupt the term in the table as the GC could
place move markers in it.

Also fixed a bug in the stack estimation logic when
a flatmap with all constant values, but not constant
keys was encountered.

Closes #7683
---
 erts/emulator/beam/erl_db_util.c | 42 +++++++++++++++++---------------
 lib/stdlib/test/ets_SUITE.erl    | 25 +++++++++++++++++++
 2 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/erts/emulator/beam/erl_db_util.c b/erts/emulator/beam/erl_db_util.c
index 8bace68d0f..98cde1a030 100644
--- a/erts/emulator/beam/erl_db_util.c
+++ b/erts/emulator/beam/erl_db_util.c
@@ -3983,7 +3983,6 @@ dmc_map(DMCContext *context, DMCHeap *heap, DMC_STACK_TYPE(UWord) *text,
         flatmap_t *m = (flatmap_t *)flatmap_val(t);
         Eterm *values = flatmap_get_values(m);
         int textpos = DMC_STACK_NUM(*text);
-        int stackpos = context->stack_used;
 
         nelems = flatmap_get_size(m);
 
@@ -3991,39 +3990,42 @@ dmc_map(DMCContext *context, DMCHeap *heap, DMC_STACK_TYPE(UWord) *text,
             return ret;
         }
 
+        if (constant_values) {
+            /* We may have to convert all values to individual matchPushC
+               instructions, if we do that then more stack will be needed
+               than estimated, so we artificially bump the needed stack here
+               so that dmc_tuple thinks that dmc_array has used the needed stack. */
+            context->stack_used += nelems;
+        }
+
         if ((ret = dmc_tuple(context, heap, text, m->keys, &constant_keys)) != retOk) {
             return ret;
         }
 
+        if (constant_values) {
+            context->stack_used -= nelems;
+        }
+
         if (constant_values && constant_keys) {
             *constant = 1;
             return retOk;
         }
 
-        /* If all values were constants, then nothing was emitted by the
-           first dmc_array, so we reset the pc and emit all values as
-           constants and then re-emit the keys. */
         if (constant_values) {
-            DMC_STACK_NUM(*text) = textpos;
-            context->stack_used = stackpos;
-            ASSERT(!constant_keys);
-            for (int i = nelems; i--;) {
-                do_emit_constant(context, text, values[i]);
-            }
-            dmc_tuple(context, heap, text, m->keys, &constant_keys);
+            /* If all values were constants, then nothing was emitted by the
+               first dmc_array, so we insert the constants at the start of the
+               stack and place the dmc_tuple after. */
+            dmc_rearrange_constants(context, text, textpos, values, nelems);
         } else if (constant_keys) {
-            Eterm *p = tuple_val(m->keys);
-            Uint nelems = arityval(*p);
-            ASSERT(!constant_values);
-            p++;
-            for (int i = nelems; i--;)
-                do_emit_constant(context, text, p[i]);
-            DMC_PUSH2(*text, matchMkTuple, nelems);
-            context->stack_used -= nelems - 1;
+            /* If all keys were constant we just want to emit the key tuple.
+               Since do_emit_constant expects tuples to be wrapped in 1 arity
+               tuples we need give do_emit_constant {keys} */
+            Eterm wrapTuple[2] = {make_arityval(1), m->keys};
+            do_emit_constant(context, text, make_tuple(wrapTuple));
         }
 
         DMC_PUSH2(*text, matchMkFlatMap, nelems);
-        context->stack_used -= nelems;  /* n values + 1 key-tuple => 1 map */
+        context->stack_used -= (nelems + 1) - 1;  /* n values + 1 key-tuple - 1 map ptr => 1 map */
         *constant = 0;
         return retOk;
     } else {
diff --git a/lib/stdlib/test/ets_SUITE.erl b/lib/stdlib/test/ets_SUITE.erl
index 588096301d..1e823e0b1c 100644
--- a/lib/stdlib/test/ets_SUITE.erl
+++ b/lib/stdlib/test/ets_SUITE.erl
@@ -44,6 +44,7 @@
          t_delete_all_objects_trap/1,
 	 t_select_delete/1,t_select_replace/1,t_select_replace_next_bug/1,
          t_select_pam_stack_overflow_bug/1,
+         t_select_flatmap_term_copy_bug/1,
          t_ets_dets/1]).
 -export([t_insert_list/1, t_insert_list_bag/1, t_insert_list_duplicate_bag/1,
          t_insert_list_set/1, t_insert_list_delete_set/1,
@@ -152,6 +153,7 @@ all() ->
      t_test_ms, t_select_delete, t_select_replace,
      t_select_replace_next_bug,
      t_select_pam_stack_overflow_bug,
+     t_select_flatmap_term_copy_bug,
      t_ets_dets, memory, t_select_reverse, t_bucket_disappears,
      t_named_select, select_fixtab_owner_change,
      select_fail, t_insert_new, t_repair_continuation,
@@ -1913,6 +1915,29 @@ t_select_pam_stack_overflow_bug(Config) ->
     ets:delete(T),
     ok.
 
+%% When a variable was used as key in ms body, the matched value would
+%% not be copied to the heap of the calling process.
+t_select_flatmap_term_copy_bug(_Config) ->
+    T = ets:new(a,[]),
+    ets:insert(T, {list_to_binary(lists:duplicate(36,$a))}),
+    V1 = ets:select(T, [{{'$1'},[],[#{ '$1' => a }]}]),
+    erlang:garbage_collect(),
+    V1 = ets:select(T, [{{'$1'},[],[#{ '$1' => a }]}]),
+    erlang:garbage_collect(),
+    V2 = ets:select(T, [{{'$1'},[],[#{ a => '$1' }]}]),
+    erlang:garbage_collect(),
+    V2 = ets:select(T, [{{'$1'},[],[#{ a => '$1' }]}]),
+    erlang:garbage_collect(),
+    V3 = ets:select(T, [{{'$1'},[],[#{ '$1' => '$1' }]}]),
+    erlang:garbage_collect(),
+    V3 = ets:select(T, [{{'$1'},[],[#{ '$1' => '$1' }]}]),
+    erlang:garbage_collect(),
+    V4 = ets:select(T, [{{'$1'},[],[#{ a => a }]}]),
+    erlang:garbage_collect(),
+    V4 = ets:select(T, [{{'$1'},[],[#{ a => a }]}]),
+    erlang:garbage_collect(),
+    ets:delete(T),
+    ok.
 
 %% Test that partly bound keys gives faster matches.
 partly_bound(Config) when is_list(Config) ->
-- 
2.35.3

Places

File 1371-erts-Fix-db_match-flatmap-copy-bug.patch of Package erlang

Places