erts: Fix use after free for ErtsTracerNif*

When doing `lookup_tracer_nif` the code assumes that
the ErtsTracerNif struct will not be deallocated after
the lookup. However, that is not true for unmanaged threads
as thread progress may pass after the lookup and before
the call to the tracer nif.

So, we make sure to delay thread progress before we do the
lookup to the ErtsTracerNif and undelay when done. As of the
writing of this commit there are 3 different trace flags that
can be triggered on a dirty scheduler:

 - garbage_collect
 - running
 - send

6 files changed, 61 insertions, 12 deletions

diff --git a/erts/emulator/beam/erl_nif.c b/erts/emulator/beam/erl_nif.c
index ebef485b04..25b3ce22b3 100644
--- a/erts/emulator/beam/erl_nif.c
+++ b/erts/emulator/beam/erl_nif.c
@@ -850,6 +850,8 @@ int enif_send(ErlNifEnv* env, const ErlNifPid* to_pid,
     from = c_p ? c_p->common.id : am_undefined;
 
     if (!env || !env->tracee) {
+        /* This clause is taken when enif_send is called in a nif
+           that is not a erl_tracer nif. */
 
         if (c_p && IS_TRACED_FL(c_p, F_TRACE_SEND)) {
 	    full_flush_env(env);
diff --git a/erts/emulator/beam/erl_process.c b/erts/emulator/beam/erl_process.c
index 1f464e2e5a..f6e30843fa 100644
--- a/erts/emulator/beam/erl_process.c
+++ b/erts/emulator/beam/erl_process.c
@@ -9970,9 +9970,12 @@ Process *erts_schedule(ErtsSchedulerData *esdp, Process *p, int calls)
 static void
 trace_schedule_in(Process *p, erts_aint32_t state)
 {
+    ErtsThrPrgrDelayHandle dhndl;
     ASSERT(IS_TRACED(p));
     ERTS_LC_ASSERT(erts_proc_lc_my_proc_locks(p) == ERTS_PROC_LOCK_MAIN);
 
+    dhndl = erts_thr_progress_unmanaged_delay();
+
     /* Clear tracer if it has been removed */
     if (erts_is_tracer_proc_enabled(p, ERTS_PROC_LOCK_MAIN, &p->common)) {
 
@@ -9988,14 +9991,17 @@ trace_schedule_in(Process *p, erts_aint32_t state)
         if (IS_TRACED_FL(p, F_TRACE_CALLS))
             erts_schedule_time_break(p, ERTS_BP_CALL_TIME_SCHEDULE_IN);
     }
-
+    erts_thr_progress_unmanaged_continue(dhndl);
 }
 
 static void
 trace_schedule_out(Process *p, erts_aint32_t state)
 {
+    ErtsThrPrgrDelayHandle dhndl;
     ASSERT(IS_TRACED(p));
     ERTS_LC_ASSERT(erts_proc_lc_my_proc_locks(p) == ERTS_PROC_LOCK_MAIN);
+
+    dhndl = erts_thr_progress_unmanaged_delay();
     
     if (IS_TRACED_FL(p, F_TRACE_CALLS) && !(state & ERTS_PSFLG_FREE))
         erts_schedule_time_break(p, ERTS_BP_CALL_TIME_SCHEDULE_OUT);
@@ -10012,6 +10018,7 @@ trace_schedule_out(Process *p, erts_aint32_t state)
             ARE_TRACE_FLAGS_ON(p, F_TRACE_SCHED_PROCS))
             trace_sched(p, ERTS_PROC_LOCK_MAIN, am_out);
     }
+    erts_thr_progress_unmanaged_continue(dhndl);
 }
 
 static int
diff --git a/erts/emulator/beam/erl_trace.c b/erts/emulator/beam/erl_trace.c
index ae7084b7f4..f787d627b0 100644
--- a/erts/emulator/beam/erl_trace.c
+++ b/erts/emulator/beam/erl_trace.c
@@ -1342,6 +1342,7 @@ trace_gc(Process *p, Eterm what, Uint size, Eterm msg)
     Eterm* hp;
     Uint sz = 0;
     Eterm tup;
+    ErtsThrPrgrDelayHandle dhndl = erts_thr_progress_unmanaged_delay();
 
     if (is_tracer_enabled(p, ERTS_PROC_LOCK_MAIN, &p->common, &tnif,
                           TRACE_FUN_E_GC, what)) {
@@ -1361,6 +1362,7 @@ trace_gc(Process *p, Eterm what, Uint size, Eterm msg)
         if (o_hp)
             erts_free(ERTS_ALC_T_TMP, o_hp);
     }
+    erts_thr_progress_unmanaged_continue(dhndl);
 }
 
 void 
@@ -2570,6 +2572,7 @@ lookup_tracer_nif(const ErtsTracer tracer)
     ErtsTracerNif tnif_tmpl;
     ErtsTracerNif *tnif;
     tnif_tmpl.module = ERTS_TRACER_MODULE(tracer);
+    ERTS_LC_ASSERT(erts_thr_progress_lc_is_delaying() || erts_get_scheduler_id() > 0);
     erts_rwmtx_rlock(&tracer_mtx);
     if ((tnif = hash_get(tracer_hash, &tnif_tmpl)) == NULL) {
         erts_rwmtx_runlock(&tracer_mtx);
diff --git a/erts/emulator/beam/module.c b/erts/emulator/beam/module.c
index 0642a06123..08d4f26d8d 100644
--- a/erts/emulator/beam/module.c
+++ b/erts/emulator/beam/module.c
@@ -133,6 +133,7 @@ erts_get_module(Eterm mod, ErtsCodeIndex code_ix)
     IndexTable* mod_tab;
 
     ASSERT(is_atom(mod));
+    ERTS_LC_ASSERT(erts_get_scheduler_id() > 0 || erts_thr_progress_lc_is_delaying());
 
     mod_tab = &module_tables[code_ix];
 
diff --git a/erts/emulator/test/dirty_nif_SUITE.erl b/erts/emulator/test/dirty_nif_SUITE.erl
index 93d0ac392c..e56ad670cb 100644
--- a/erts/emulator/test/dirty_nif_SUITE.erl
+++ b/erts/emulator/test/dirty_nif_SUITE.erl
@@ -442,10 +442,10 @@ dirty_nif_send_traced(Config) when is_list(Config) ->
 			      {ok, Rcvr} = send_wait_from_dirty_nif(Rcvr),
 			      Parent ! {self(), sent}
 		      end),
-    1 = erlang:trace(Sndr, true, [send]),
+    1 = erlang:trace(Sndr, true, [send, running, exiting]),
     Start = erlang:monotonic_time(),
     Sndr ! {self(), go},
-    receive {trace, Sndr, send, {ok, Rcvr}, Rcvr} -> ok end,
+
     receive {Rcvr, received} -> ok end,
     End1 = erlang:monotonic_time(),
     Time1 = erlang:convert_time_unit(End1-Start, native, 1000),
@@ -456,8 +456,33 @@ dirty_nif_send_traced(Config) when is_list(Config) ->
     Time2 = erlang:convert_time_unit(End2-Start, native, 1000),
     io:format("Time2: ~p milliseconds~n", [Time2]),
     true = Time2 >= 1900,
+
+    %% Make sure that the send trace is
+    %% in between an in and and out trace
+    (fun F() ->
+             %% We got an in trace, look for out or send
+             {trace,Sndr,in,_} = recv_trace_from(Sndr),
+             case recv_trace_from(Sndr) of
+                 {trace,Sndr,out,_} ->
+                     %% We got an out, look for another in
+                     F();
+                 {trace,Sndr,send,_,_} ->
+                     %% We got a send, look for another out
+                     {trace,Sndr,out,_} = recv_trace_from(Sndr),
+                     ok
+             end
+     end)(),
+
     ok.
 
+recv_trace_from(Sndr) ->
+    receive
+        M when element(1, M) =:= trace;
+               element(1, M) =:= trace_ts,
+               element(2, M) =:= Sndr ->
+            M
+    end.
+
 %%
 %% Internal...
 %%
diff --git a/erts/emulator/test/gc_SUITE.erl b/erts/emulator/test/gc_SUITE.erl
index f3942ef416..9d02102d04 100644
--- a/erts/emulator/test/gc_SUITE.erl
+++ b/erts/emulator/test/gc_SUITE.erl
@@ -209,6 +209,13 @@ minor_major_gc_option_self(_Config) ->
             Pid ! {gc, Ref, major}
         end, [gc_major_start, gc_major_end]),
 
+    %% Try as major dirty, the test process will self-trigger GC
+    check_gc_tracing_around(
+        fun(Pid, Ref) ->
+            Pid ! {gc, Ref, major}
+        end, [gc_major_start, gc_major_end],
+      lists:seq(1,128 * 1024)),
+
     %% Try as minor, the test process will self-trigger GC
     check_gc_tracing_around(
         fun(Pid, Ref) ->
@@ -252,17 +259,21 @@ minor_major_gc_option_async(_Config) ->
 %% it results in any unexpected messages or if the expected trace tags are not
 %% received.
 check_gc_tracing_around(Fun, ExpectedTraceTags) ->
+    check_gc_tracing_around(Fun, ExpectedTraceTags, []).
+check_gc_tracing_around(Fun, ExpectedTraceTags, State) ->
     Ref = erlang:make_ref(),
     Pid = spawn(
-        fun Endless() ->
-            receive
-                {gc, Ref, Type} ->
-                    erlang:garbage_collect(self(), [{type, Type}])
-            after 100 ->
-                ok
-            end,
-            Endless()
-        end),
+            fun() ->
+                    (fun Endless(S) ->
+                             receive
+                                 {gc, Ref, Type} ->
+                                     erlang:garbage_collect(self(), [{type, Type}])
+                             after 100 ->
+                                     ok
+                             end,
+                             Endless(S)
+                     end)(State)
+            end),
     erlang:garbage_collect(Pid, []),
     erlang:trace(Pid, true, [garbage_collection]),
     Fun(Pid, Ref),