From aa086af7156490e22d139ab2938544ad07714169 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Tue, 9 Oct 2018 12:28:12 +0200 Subject: erts: Fix bug in ets:select_replace for bound key which may cause following calls to ets:next or ets:prev to fail. --- erts/emulator/beam/erl_db_tree.c | 12 ++---------- lib/stdlib/test/ets_SUITE.erl | 22 +++++++++++++++++++++- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/erts/emulator/beam/erl_db_tree.c b/erts/emulator/beam/erl_db_tree.c index d7deadacf0..f0c01e6e78 100644 --- a/erts/emulator/beam/erl_db_tree.c +++ b/erts/emulator/beam/erl_db_tree.c @@ -1887,22 +1887,14 @@ static int db_select_replace_tree(Process *p, DbTable *tbl, Eterm tid, sc.mp = mpi.mp; sc.all_objects = mpi.all_objects; - stack = get_static_stack(tb); if (!mpi.got_partial && mpi.some_limitation && CMP_EQ(mpi.least,mpi.most)) { - TreeDbTerm* term = *(mpi.save_term); doit_select_replace(tb,mpi.save_term,&sc,0 /* dummy */); - if (stack != NULL) { - if (TOP_NODE(stack) == term) - // throw away potentially invalid reference - REPLACE_TOP_NODE(stack, *(mpi.save_term)); - release_stack(tb, stack); - } + reset_static_stack(tb); /* may refer replaced term */ RET_TO_BIF(erts_make_integer(sc.replaced,p),DB_ERROR_NONE); } - if (stack == NULL) - stack = get_any_stack(tb); + stack = get_any_stack(tb); if (mpi.some_limitation) { if ((this = find_next_from_pb_key(tb, stack, mpi.most)) != NULL) { diff --git a/lib/stdlib/test/ets_SUITE.erl b/lib/stdlib/test/ets_SUITE.erl index 05451a83fb..257a718404 100644 --- a/lib/stdlib/test/ets_SUITE.erl +++ b/lib/stdlib/test/ets_SUITE.erl @@ -41,7 +41,7 @@ -export([t_delete_object/1, t_init_table/1, t_whitebox/1, select_bound_chunk/1, t_delete_all_objects/1, t_insert_list/1, t_test_ms/1, - t_select_delete/1,t_select_replace/1,t_ets_dets/1]). + t_select_delete/1,t_select_replace/1,t_select_replace_next_bug/1,t_ets_dets/1]). -export([ordered/1, ordered_match/1, interface_equality/1, fixtable_next/1, fixtable_insert/1, rename/1, rename_unnamed/1, evil_rename/1, @@ -122,6 +122,7 @@ all() -> select_bound_chunk, t_init_table, t_whitebox, t_delete_all_objects, t_insert_list, t_test_ms, t_select_delete, t_select_replace, + t_select_replace_next_bug, t_ets_dets, memory, t_select_reverse, t_bucket_disappears, select_fail, t_insert_new, t_repair_continuation, otp_5340, otp_6338, otp_6842_select_1000, otp_7665, @@ -1373,6 +1374,25 @@ t_select_replace(Config) when is_list(Config) -> verify_etsmem(EtsMem). +%% OTP-15346: Bug caused select_replace of bound key to corrupt static stack +%% used by ets:next and ets:prev. +t_select_replace_next_bug(Config) when is_list(Config) -> + T = ets:new(k, [ordered_set]), + [ets:insert(T, {I, value}) || I <- lists:seq(1,10)], + 1 = ets:first(T), + + %% Make sure select_replace does not leave pointer + %% to deallocated {2,value} in static stack. + MS = [{{2,value}, [], [{{2,"new_value"}}]}], + 1 = ets:select_replace(T, MS), + + %% This would crash or give wrong result at least on DEBUG emulator + %% where deallocated memory is overwritten. + 2 = ets:next(T, 1), + + ets:delete(T). + + %% Test that partly bound keys gives faster matches. partly_bound(Config) when is_list(Config) -> case os:type() of -- cgit v1.2.3