Merge branch 'lukas/erts/fix-seq-trace-token-gc/OTP-17209' into maint

* lukas/erts/fix-seq-trace-token-gc/OTP-17209:
  erts: Fix seq_trace set_on_spawn lock check bug
  erts: Fix seq_trace token gc

3 files changed, 141 insertions, 43 deletions

diff --git a/erts/emulator/beam/erl_bif_trace.c b/erts/emulator/beam/erl_bif_trace.c
index 7708e0755c..36cad53ce4 100644
--- a/erts/emulator/beam/erl_bif_trace.c
+++ b/erts/emulator/beam/erl_bif_trace.c
@@ -1841,10 +1841,13 @@ new_seq_trace_token(Process* p, int ensure_new_heap)
 				    make_small(p->seq_trace_lastcnt));
     }
     else if (ensure_new_heap) {
+        Eterm *mature = p->abandoned_heap ? p->abandoned_heap : p->heap;
+        Uint mature_size = p->high_water - mature;
         Eterm* tpl = tuple_val(SEQ_TRACE_TOKEN(p));
         ASSERT(arityval(tpl[0]) == 5);
         if (ErtsInArea(tpl, OLD_HEAP(p),
-                       (OLD_HEND(p) - OLD_HEAP(p))*sizeof(Eterm))) {
+                       (OLD_HEND(p) - OLD_HEAP(p))*sizeof(Eterm)) ||
+            ErtsInArea(tpl, mature, mature_size*sizeof(Eterm))) {
             hp = HAlloc(p, 6);
             sys_memcpy(hp, tpl, 6*sizeof(Eterm));
             SEQ_TRACE_TOKEN(p) = make_tuple(hp);
diff --git a/erts/emulator/beam/erl_process.c b/erts/emulator/beam/erl_process.c
index 3c8b1631b4..3b9ee7b4fb 100644
--- a/erts/emulator/beam/erl_process.c
+++ b/erts/emulator/beam/erl_process.c
@@ -11939,6 +11939,28 @@ erl_create_process(Process* parent, /* Parent of process (default group leader).
     p->fp_exception = 0;
 #endif
 
+    if (parent && IS_TRACED(parent)) {
+	if (ERTS_TRACE_FLAGS(parent) & F_TRACE_SOS) {
+	    ERTS_TRACE_FLAGS(p) |= (ERTS_TRACE_FLAGS(parent) & TRACEE_FLAGS);
+            erts_tracer_replace(&p->common, ERTS_TRACER(parent));
+	}
+        if (ERTS_TRACE_FLAGS(parent) & F_TRACE_SOS1) {
+	    /* Overrides TRACE_CHILDREN */
+	    ERTS_TRACE_FLAGS(p) |= (ERTS_TRACE_FLAGS(parent) & TRACEE_FLAGS);
+            erts_tracer_replace(&p->common, ERTS_TRACER(parent));
+	    ERTS_TRACE_FLAGS(p) &= ~(F_TRACE_SOS1 | F_TRACE_SOS);
+	    ERTS_TRACE_FLAGS(parent) &= ~(F_TRACE_SOS1 | F_TRACE_SOS);
+	}
+        if (so->flags & SPO_LINK && ERTS_TRACE_FLAGS(parent) & (F_TRACE_SOL|F_TRACE_SOL1)) {
+            ERTS_TRACE_FLAGS(p) |= (ERTS_TRACE_FLAGS(parent)&TRACEE_FLAGS);
+            erts_tracer_replace(&p->common, ERTS_TRACER(parent));
+            if (ERTS_TRACE_FLAGS(parent) & F_TRACE_SOL1) {/*maybe override*/
+                ERTS_TRACE_FLAGS(p) &= ~(F_TRACE_SOL1 | F_TRACE_SOL);
+                ERTS_TRACE_FLAGS(parent) &= ~(F_TRACE_SOL1 | F_TRACE_SOL);
+            }
+        }
+    }
+
     /* seq_trace is handled before regular tracing as the latter may touch the
      * trace token. */
     if (!have_seqtrace(token)) {
@@ -12039,39 +12061,17 @@ erl_create_process(Process* parent, /* Parent of process (default group leader).
         }
     }
 
-    if (parent && IS_TRACED(parent)) {
-	if (ERTS_TRACE_FLAGS(parent) & F_TRACE_SOS) {
-	    ERTS_TRACE_FLAGS(p) |= (ERTS_TRACE_FLAGS(parent) & TRACEE_FLAGS);
-            erts_tracer_replace(&p->common, ERTS_TRACER(parent));
-	}
-        if (ERTS_TRACE_FLAGS(parent) & F_TRACE_SOS1) {
-	    /* Overrides TRACE_CHILDREN */
-	    ERTS_TRACE_FLAGS(p) |= (ERTS_TRACE_FLAGS(parent) & TRACEE_FLAGS);
-            erts_tracer_replace(&p->common, ERTS_TRACER(parent));
-	    ERTS_TRACE_FLAGS(p) &= ~(F_TRACE_SOS1 | F_TRACE_SOS);
-	    ERTS_TRACE_FLAGS(parent) &= ~(F_TRACE_SOS1 | F_TRACE_SOS);
-	}
-        if (so->flags & SPO_LINK && ERTS_TRACE_FLAGS(parent) & (F_TRACE_SOL|F_TRACE_SOL1)) {
-		ERTS_TRACE_FLAGS(p) |= (ERTS_TRACE_FLAGS(parent)&TRACEE_FLAGS);
-                erts_tracer_replace(&p->common, ERTS_TRACER(parent));
-		if (ERTS_TRACE_FLAGS(parent) & F_TRACE_SOL1) {/*maybe override*/
-		    ERTS_TRACE_FLAGS(p) &= ~(F_TRACE_SOL1 | F_TRACE_SOL);
-		    ERTS_TRACE_FLAGS(parent) &= ~(F_TRACE_SOL1 | F_TRACE_SOL);
-		}
-        }
-        if (ARE_TRACE_FLAGS_ON(parent, F_TRACE_PROCS)) {
-            /* The locks may already be released if seq_trace is enabled as
-             * well. */
-            if ((locks & (ERTS_PROC_LOCK_STATUS|ERTS_PROC_LOCK_TRACE))
-                  == (ERTS_PROC_LOCK_STATUS|ERTS_PROC_LOCK_TRACE)) {
-                locks &= ~(ERTS_PROC_LOCK_STATUS|ERTS_PROC_LOCK_TRACE);
-                erts_proc_unlock(p, ERTS_PROC_LOCK_STATUS|ERTS_PROC_LOCK_TRACE);
-                erts_proc_unlock(parent, ERTS_PROC_LOCK_STATUS|ERTS_PROC_LOCK_TRACE);
-            }
-            trace_proc_spawn(parent, am_spawn, p->common.id, mod, func, args);
-            if (so->flags & SPO_LINK)
-                trace_proc(parent, locks, parent, am_link, p->common.id);
+    if (parent && IS_TRACED_FL(parent, F_TRACE_PROCS)) {
+        /* The locks may already be released if seq_trace is enabled as well. */
+        if ((locks & (ERTS_PROC_LOCK_STATUS|ERTS_PROC_LOCK_TRACE))
+            == (ERTS_PROC_LOCK_STATUS|ERTS_PROC_LOCK_TRACE)) {
+            locks &= ~(ERTS_PROC_LOCK_STATUS|ERTS_PROC_LOCK_TRACE);
+            erts_proc_unlock(p, ERTS_PROC_LOCK_STATUS|ERTS_PROC_LOCK_TRACE);
+            erts_proc_unlock(parent, ERTS_PROC_LOCK_STATUS|ERTS_PROC_LOCK_TRACE);
         }
+        trace_proc_spawn(parent, am_spawn, p->common.id, mod, func, args);
+        if (so->flags & SPO_LINK)
+            trace_proc(parent, locks, parent, am_link, p->common.id);
     }
 
     if (IS_TRACED_FL(p, F_TRACE_PROCS)) {
diff --git a/lib/kernel/test/seq_trace_SUITE.erl b/lib/kernel/test/seq_trace_SUITE.erl
index f8efd1ffea..4a00b8d3d0 100644
--- a/lib/kernel/test/seq_trace_SUITE.erl
+++ b/lib/kernel/test/seq_trace_SUITE.erl
@@ -26,7 +26,7 @@
 	 init_per_group/2,end_per_group/2,
 	 init_per_testcase/2,end_per_testcase/2]).
 -export([token_set_get/1, tracer_set_get/1, print/1,
-         old_heap_token/1,
+         old_heap_token/1,mature_heap_token/1,
 	 send/1, distributed_send/1, recv/1, distributed_recv/1,
 	 trace_exit/1, distributed_exit/1, call/1, port/1,
          port_clean_token/1,
@@ -54,7 +54,7 @@ suite() ->
 all() -> 
     [token_set_get, tracer_set_get, print, send, send_literal,
      distributed_send, recv, distributed_recv, trace_exit,
-     old_heap_token,
+     old_heap_token, mature_heap_token,
      distributed_exit, call, port, match_set_seq_token,
      port_clean_token,
      gc_seq_token, label_capability_mismatch,
@@ -538,18 +538,24 @@ call(Config) when is_list(Config) ->
 
 %% The token should follow spawn, just like it follows messages.
 inherit_on_spawn(Config) when is_list(Config) ->
-    lists:foreach(fun (Test) ->
-                          inherit_on_spawn_test(Test)
-                  end,
-                  [spawn, spawn_link, spawn_monitor,
-                   spawn_opt, spawn_request]),
+    lists:foreach(
+      fun (Test) ->
+              lists:foreach(
+                fun (TraceFlags) ->
+                        inherit_on_spawn_test(Test, TraceFlags)
+                end,
+                combinations(spawn_trace_flags()))
+      end,
+      [spawn, spawn_link, spawn_monitor,
+       spawn_opt, spawn_request]),
     ok.
 
-inherit_on_spawn_test(Spawn) ->
-    io:format("Testing ~p()~n", [Spawn]),
+inherit_on_spawn_test(Spawn, TraceFlags) ->
+    io:format("Testing ~p() with ~p trace flags~n", [Spawn, TraceFlags]),
 
     seq_trace:reset_trace(),
     start_tracer(),
+    start_spawn_tracer(TraceFlags),
 
     Ref = make_ref(),
     seq_trace:set_token(label,Ref),
@@ -580,6 +586,7 @@ inherit_on_spawn_test(Spawn) ->
 
     receive {gurka,Ref} -> ok end,
     seq_trace:reset_trace(),
+    erlang:trace(self(),false,[procs|TraceFlags]),
 
     Sequence = lists:keysort(3, stop_tracer(6)),
     io:format("Sequence: ~p~n", [Sequence]),
@@ -644,9 +651,35 @@ inherit_on_spawn_test(Spawn) ->
        GurkaMsg},
       _} = RGurkaMsg,
 
+
+    Links = not(spawn =:= spawn orelse Spawn =:= spawn_monitor),
+    SoL = lists:member(set_on_link,TraceFlags) orelse
+        lists:member(set_on_first_link,TraceFlags),
+    SoS = lists:member(set_on_spawn,TraceFlags) orelse
+        lists:member(set_on_first_sapwn,TraceFlags),
+
+    NoTraceMessages =
+        if
+            SoS andalso Links ->
+                4;
+            SoS andalso not Links ->
+                2;
+            SoL andalso Links ->
+                4;
+            SoL andalso not Links->
+                1;
+            Links andalso not SoL andalso not SoS ->
+                2;
+            not Links andalso not SoL andalso not SoS ->
+                1
+        end,
+
+    TraceMessages = stop_spawn_tracer(NoTraceMessages),
+
     unlink(Other),
     exit(Other, kill),
 
+
     ok.
 
 inherit_on_dist_spawn(Config) when is_list(Config) ->
@@ -971,6 +1004,24 @@ old_heap_token(Config) when is_list(Config) ->
     {label,NewLabel} = seq_trace:get_token(label),
     ok.
 
+%% Verify changing label on existing token when it resides on mature heap.
+%% Bug caused faulty ref from old to new heap.
+mature_heap_token(Config) when is_list(Config) ->
+
+    seq_trace:set_token(label, 1),
+    erlang:garbage_collect(self(), [{type, minor}]),
+    %% Now token should be on mature heap
+    %% Set a new non-literal label which should reside on new-heap.
+    NewLabel = {self(), "new label"},
+    seq_trace:set_token(label, NewLabel),
+
+    %% If bug, we now have a ref from mature to new heap. If we now GC
+    %% twice the token will refer to deallocated memory.
+    erlang:garbage_collect(self(), [{type, minor}]),
+    erlang:garbage_collect(self(), [{type, minor}]),
+    {label,NewLabel} = seq_trace:get_token(label),
+    ok.
+
 
 match_set_seq_token(doc) ->
     ["Tests that match spec function set_seq_token does not "
@@ -1366,7 +1417,6 @@ start_tracer(Node) ->
             unlink(Pid),
             Pid
     end.
-           
 
 set_token_flags([]) ->
     ok;
@@ -1379,6 +1429,51 @@ set_token_flags([Flag|Flags]) ->
     seq_trace:set_token(Flag, true),
     set_token_flags(Flags).
 
+start_spawn_tracer(TraceFlags) ->
+
+    %% Disable old trace flags
+    erlang:trace(self(), false, spawn_trace_flags()),
+
+    Me = self(),
+    Ref = make_ref(),
+    Pid = spawn_link(
+            fun () ->
+                    register(spawn_tracer, self()),
+                    Me ! Ref,
+                    (fun F(Data) ->
+                             receive
+                                 {get, N, StopRef, Pid} when N =< length(Data) ->
+                                     Pid ! {lists:reverse(Data), StopRef};
+                                 M when element(1,M) =:= trace ->
+                                     F([M|Data])
+                             end
+                    end)([])
+            end),
+    receive
+        Ref ->
+            erlang:trace(self(),true,[{tracer,Pid}, procs | TraceFlags])
+    end.
+
+stop_spawn_tracer(N) ->
+    Ref = make_ref(),
+    spawn_tracer ! {get, N, Ref, self()},
+    receive
+        {Data, Ref} ->
+            Data
+    end.
+
+spawn_trace_flags() ->
+    [set_on_spawn, set_on_link, set_on_spawn,
+     set_on_first_link, set_on_first_spawn].
+
+combinations(Flags) ->
+    %% Do a bit of sofs magic to create a list of lists with
+    %% all the combinations of all the flags above
+    Set = sofs:from_term(Flags),
+    Product = sofs:product(list_to_tuple(lists:duplicate(length(Flags),Set))),
+    Combinations = [lists:usort(tuple_to_list(T)) || T <- sofs:to_external(Product)],
+    [[] | lists:usort(Combinations)].
+
 check_ts(no_timestamp, Ts) ->
     try
 	no_timestamp = Ts