From 636321c57803ab19f93947e3423a22d3c8ad9501 Mon Sep 17 00:00:00 2001 From: Lukas Larsson Date: Fri, 12 Feb 2021 17:03:14 +0100 Subject: erts: Fix seq_trace set_on_spawn lock check bug --- erts/emulator/beam/erl_process.c | 64 ++++++++++++------------- lib/kernel/test/seq_trace_SUITE.erl | 93 +++++++++++++++++++++++++++++++++---- 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 -- cgit v1.2.1