erts: Fix seq_trace set_on_spawn lock check bug

2 files changed, 117 insertions, 40 deletions

diff --git a/erts/emulator/beam/erl_process.c b/erts/emulator/beam/erl_process.c
index 5d68bd1ae0..edaccbb284 100644
--- a/erts/emulator/beam/erl_process.c
+++ b/erts/emulator/beam/erl_process.c
@@ -11936,6 +11936,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)) {
@@ -12036,39 +12058,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 df1ad00a80..4a00b8d3d0 100644
--- a/lib/kernel/test/seq_trace_SUITE.erl
+++ b/lib/kernel/test/seq_trace_SUITE.erl
@@ -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) ->
@@ -1384,7 +1417,6 @@ start_tracer(Node) ->
             unlink(Pid),
             Pid
     end.
-           
 
 set_token_flags([]) ->
     ok;
@@ -1397,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