Merge branch 'jv/18/emulator-send-to-error_logger/OTP-12735'

* jv/18/emulator-send-to-error_logger/OTP-12735:
  erts: Fix erts_send_error_term_to_logger memory leak
  Send format and args on process exit to error_logger
  Add erts_send_error_term_to_logger

4 files changed, 165 insertions, 78 deletions

diff --git a/erts/emulator/beam/beam_emu.c b/erts/emulator/beam/beam_emu.c
index 9fe02c3724..a21622f424 100644
--- a/erts/emulator/beam/beam_emu.c
+++ b/erts/emulator/beam/beam_emu.c
@@ -5577,18 +5577,35 @@ next_catch(Process* c_p, Eterm *reg) {
 static void
 terminate_proc(Process* c_p, Eterm Value)
 {
+    Eterm *hp;
+    Eterm Args = NIL;
+
     /* Add a stacktrace if this is an error. */
     if (GET_EXC_CLASS(c_p->freason) == EXTAG_ERROR) {
         Value = add_stacktrace(c_p, Value, c_p->ftrace);
     }
     /* EXF_LOG is a primary exception flag */
     if (c_p->freason & EXF_LOG) {
+	int alive = erts_is_alive;
 	erts_dsprintf_buf_t *dsbufp = erts_create_logger_dsbuf();
-	erts_dsprintf(dsbufp, "Error in process %T ", c_p->common.id);
-	if (erts_is_alive)
-	    erts_dsprintf(dsbufp, "on node %T ", erts_this_node->sysname);
-	erts_dsprintf(dsbufp,"with exit value: %0.*T\n", display_items, Value);
-	erts_send_error_to_logger(c_p->group_leader, dsbufp);
+
+        /* Build the format message */
+	erts_dsprintf(dsbufp, "Error in process ~p ");
+	if (alive)
+	    erts_dsprintf(dsbufp, "on node ~p ");
+	erts_dsprintf(dsbufp, "with exit value:~n~p~n");
+
+        /* Build the args in reverse order */
+	hp = HAlloc(c_p, 2);
+	Args = CONS(hp, Value, Args);
+	if (alive) {
+	    hp = HAlloc(c_p, 2);
+	    Args = CONS(hp, erts_this_node->sysname, Args);
+	}
+	hp = HAlloc(c_p, 2);
+	Args = CONS(hp, c_p->common.id, Args);
+
+	erts_send_error_term_to_logger(c_p->group_leader, dsbufp, Args);
     }
     /*
      * If we use a shared heap, the process will be garbage-collected.
diff --git a/erts/emulator/beam/sys.h b/erts/emulator/beam/sys.h
index 54059ee9a5..cd53069872 100644
--- a/erts/emulator/beam/sys.h
+++ b/erts/emulator/beam/sys.h
@@ -657,6 +657,7 @@ erts_dsprintf_buf_t *erts_create_logger_dsbuf(void);
 int erts_send_info_to_logger(Eterm, erts_dsprintf_buf_t *);
 int erts_send_warning_to_logger(Eterm, erts_dsprintf_buf_t *);
 int erts_send_error_to_logger(Eterm, erts_dsprintf_buf_t *);
+int erts_send_error_term_to_logger(Eterm, erts_dsprintf_buf_t *, Eterm);
 int erts_send_info_to_logger_str(Eterm, char *); 
 int erts_send_warning_to_logger_str(Eterm, char *);
 int erts_send_error_to_logger_str(Eterm, char *);
diff --git a/erts/emulator/beam/utils.c b/erts/emulator/beam/utils.c
index e21e916de5..965de748c9 100644
--- a/erts/emulator/beam/utils.c
+++ b/erts/emulator/beam/utils.c
@@ -2222,97 +2222,157 @@ tail_recur:
 #undef MAKE_HASH_CDR_POST_OP
 }
 
-static int do_send_to_logger(Eterm tag, Eterm gleader, char *buf, int len)
+static Eterm
+do_allocate_logger_message(Eterm gleader, Eterm **hp, ErlOffHeap **ohp,
+			   ErlHeapFragment **bp, Process **p, Uint sz)
 {
-    /* error_logger ! 
-       {notify,{info_msg,gleader,{emulator,"~s~n",[<message as list>]}}} |
-       {notify,{error,gleader,{emulator,"~s~n",[<message as list>]}}} |
-       {notify,{warning_msg,gleader,{emulator,"~s~n",[<message as list>}]}} */
-    Eterm* hp;
-    Uint sz;
     Uint gl_sz;
-    Eterm gl;
-    Eterm list,plist,format,tuple1,tuple2,tuple3;
-    ErlOffHeap *ohp;
-    ErlHeapFragment *bp = NULL;
-#if !defined(ERTS_SMP)
-    Process *p;
-#endif
-
-    ASSERT(is_atom(tag));
-
-    if (len <= 0) {
-	return -1;
-    }
+    gl_sz = IS_CONST(gleader) ? 0 : size_object(gleader);
+    sz = sz + gl_sz;
 
 #ifndef ERTS_SMP
 #ifdef USE_THREADS
-    p = NULL;
     if (erts_get_scheduler_data()) /* Must be scheduler thread */
 #endif
     {
-	p = erts_whereis_process(NULL, 0, am_error_logger, 0, 0);
-	if (p) {
-	    erts_aint32_t state = erts_smp_atomic32_read_acqb(&p->state);
+	*p = erts_whereis_process(NULL, 0, am_error_logger, 0, 0);
+	if (*p) {
+	    erts_aint32_t state = erts_smp_atomic32_read_acqb(&(*p)->state);
 	    if (state & (ERTS_PSFLG_RUNNING|ERTS_PSFLG_RUNNING_SYS))
-		p = NULL;
+		*p = NULL;
 	}
     }
 
-    if (!p) {
-	/* buf *always* points to a null terminated string */
-	erts_fprintf(stderr, "(no error logger present) %T: \"%s\"\n",
-		     tag, buf);
-	return 0;
+    if (!*p) {
+	return NIL;
     }
-    /* So we have an error logger, lets build the message */
-#endif
-    gl_sz = IS_CONST(gleader) ? 0 : size_object(gleader);
-    sz = len * 2 /* message list */+ 2 /* cons surrounding message list */
-	+ gl_sz + 
-	3 /*outer 2-tuple*/ + 4 /* middle 3-tuple */ + 4 /*inner 3-tuple */ +
-	8 /* "~s~n" */;
 
-#ifndef ERTS_SMP
-    if (sz <= HeapWordsLeft(p)) {
-	ohp = &MSO(p);
-	hp = HEAP_TOP(p);
-	HEAP_TOP(p) += sz;
+    /* So we have an error logger, lets build the message */
+    if (sz <= HeapWordsLeft(*p)) {
+	*ohp = &MSO(*p);
+	*hp = HEAP_TOP(*p);
+	HEAP_TOP(*p) += sz;
     } else {
 #endif
-	bp = new_message_buffer(sz);
-	ohp = &bp->off_heap;
-	hp = bp->mem;
+	*bp = new_message_buffer(sz);
+	*ohp = &(*bp)->off_heap;
+	*hp = (*bp)->mem;
 #ifndef ERTS_SMP
     }
 #endif
-    gl = (is_nil(gleader)
+
+    return (is_nil(gleader)
 	  ? am_noproc
 	  : (IS_CONST(gleader)
 	     ? gleader
-	     : copy_struct(gleader,gl_sz,&hp,ohp)));
-    list = buf_to_intlist(&hp, buf, len, NIL);
-    plist = CONS(hp,list,NIL);
-    hp += 2;
-    format = buf_to_intlist(&hp, "~s~n", 4, NIL);
-    tuple1 = TUPLE3(hp, am_emulator, format, plist);
-    hp += 4;
-    tuple2 = TUPLE3(hp, tag, gl, tuple1);
-    hp += 4;
-    tuple3 = TUPLE2(hp, am_notify, tuple2);
+	     : copy_struct(gleader,gl_sz,hp,*ohp)));
+}
+
+static void do_send_logger_message(Eterm *hp, ErlOffHeap *ohp, ErlHeapFragment *bp,
+				   Process *p, Eterm message)
+{
 #ifdef HARDDEBUG
-    erts_fprintf(stderr, "%T\n", tuple3);
+    erts_fprintf(stderr, "%T\n", message);
 #endif
 #ifdef ERTS_SMP
     {
 	Eterm from = erts_get_current_pid();
 	if (is_not_internal_pid(from))
 	    from = NIL;
-	erts_queue_error_logger_message(from, tuple3, bp);
+	erts_queue_error_logger_message(from, message, bp);
     }
 #else
-    erts_queue_message(p, NULL /* only used for smp build */, bp, tuple3, NIL);
+    erts_queue_message(p, NULL /* only used for smp build */, bp, message, NIL);
 #endif
+}
+
+/* error_logger !
+   {notify,{info_msg,gleader,{emulator,format,[args]}}} |
+   {notify,{error,gleader,{emulator,format,[args]}}} |
+   {notify,{warning_msg,gleader,{emulator,format,[args}]}} */
+static int do_send_to_logger(Eterm tag, Eterm gleader, char *buf, int len)
+{
+    Uint sz;
+    Eterm gl;
+    Eterm list,args,format,tuple1,tuple2,tuple3;
+
+    Eterm *hp = NULL;
+    ErlOffHeap *ohp = NULL;
+    ErlHeapFragment *bp = NULL;
+    Process *p = NULL;
+
+    ASSERT(is_atom(tag));
+
+    if (len <= 0) {
+	return -1;
+    }
+
+    sz = len * 2 /* message list */ + 2 /* cons surrounding message list */
+	+ 3 /*outer 2-tuple*/ + 4 /* middle 3-tuple */ + 4 /*inner 3-tuple */
+	+ 8 /* "~s~n" */;
+
+    /* gleader size is accounted and allocated next */
+    gl = do_allocate_logger_message(gleader, &hp, &ohp, &bp, &p, sz);
+
+    if(is_nil(gl)) {
+       /* buf *always* points to a null terminated string */
+       erts_fprintf(stderr, "(no error logger present) %T: \"%s\"\n",
+                    tag, buf);
+       return 0;
+    }
+
+    list = buf_to_intlist(&hp, buf, len, NIL);
+    args = CONS(hp,list,NIL);
+    hp += 2;
+    format = buf_to_intlist(&hp, "~s~n", 4, NIL);
+    tuple1 = TUPLE3(hp, am_emulator, format, args);
+    hp += 4;
+    tuple2 = TUPLE3(hp, tag, gl, tuple1);
+    hp += 4;
+    tuple3 = TUPLE2(hp, am_notify, tuple2);
+
+    do_send_logger_message(hp, ohp, bp, p, tuple3);
+    return 0;
+}
+
+static int do_send_term_to_logger(Eterm tag, Eterm gleader,
+				  char *buf, int len, Eterm args)
+{
+    Uint sz;
+    Eterm gl;
+    Uint args_sz;
+    Eterm format,tuple1,tuple2,tuple3;
+
+    Eterm *hp = NULL;
+    ErlOffHeap *ohp = NULL;
+    ErlHeapFragment *bp = NULL;
+    Process *p = NULL;
+
+    ASSERT(is_atom(tag));
+
+    args_sz = size_object(args);
+    sz = len * 2 /* format */ + args_sz
+	+ 3 /*outer 2-tuple*/ + 4 /* middle 3-tuple */ + 4 /*inner 3-tuple */;
+
+    /* gleader size is accounted and allocated next */
+    gl = do_allocate_logger_message(gleader, &hp, &ohp, &bp, &p, sz);
+
+    if(is_nil(gl)) {
+       /* buf *always* points to a null terminated string */
+       erts_fprintf(stderr, "(no error logger present) %T: \"%s\" %T\n",
+                    tag, buf, args);
+       return 0;
+    }
+
+    format = buf_to_intlist(&hp, buf, len, NIL);
+    args = copy_struct(args, args_sz, &hp, ohp);
+    tuple1 = TUPLE3(hp, am_emulator, format, args);
+    hp += 4;
+    tuple2 = TUPLE3(hp, tag, gl, tuple1);
+    hp += 4;
+    tuple3 = TUPLE2(hp, am_notify, tuple2);
+
+    do_send_logger_message(hp, ohp, bp, p, tuple3);
     return 0;
 }
 
@@ -2340,6 +2400,12 @@ send_error_to_logger(Eterm gleader, char *buf, int len)
     return do_send_to_logger(am_error, gleader, buf, len);
 }
 
+static ERTS_INLINE int
+send_error_term_to_logger(Eterm gleader, char *buf, int len, Eterm args)
+{
+    return do_send_term_to_logger(am_error, gleader, buf, len, args);
+}
+
 #define LOGGER_DSBUF_INC_SZ 256
 
 static erts_dsprintf_buf_t *
@@ -2415,6 +2481,15 @@ erts_send_error_to_logger(Eterm gleader, erts_dsprintf_buf_t *dsbufp)
 }
 
 int
+erts_send_error_term_to_logger(Eterm gleader, erts_dsprintf_buf_t *dsbufp, Eterm args)
+{
+    int res;
+    res = send_error_term_to_logger(gleader, dsbufp->str, dsbufp->str_len, args);
+    destroy_logger_dsbuf(dsbufp);
+    return res;
+}
+
+int
 erts_send_info_to_logger_str(Eterm gleader, char *str)
 {
     return send_info_to_logger(gleader, str, sys_strlen(str));
diff --git a/lib/kernel/test/error_logger_SUITE.erl b/lib/kernel/test/error_logger_SUITE.erl
index 05bf5aae18..1c2e56f083 100644
--- a/lib/kernel/test/error_logger_SUITE.erl
+++ b/lib/kernel/test/error_logger_SUITE.erl
@@ -32,7 +32,7 @@
 	 error_report/1, info_report/1, error/1, info/1,
 	 emulator/1, tty/1, logfile/1, add/1, delete/1]).
 
--export([generate_error/0]).
+-export([generate_error/2]).
 
 -export([init/1,
 	 handle_event/2, handle_call/2, handle_info/2,
@@ -210,13 +210,16 @@ emulator(suite) -> [];
 emulator(doc) -> [];
 emulator(Config) when is_list(Config) ->
     ?line error_logger:add_report_handler(?MODULE, self()),
-    spawn(?MODULE, generate_error, []),
-    reported(emulator),
+    Msg = "Error in process ~p on node ~p with exit value:~n~p~n",
+    Error = {badmatch,4},
+    Stack = [{module, function, 2, []}],
+    Pid = spawn(?MODULE, generate_error, [Error, Stack]),
+    reported(error, Msg, [Pid, node(), {Error, Stack}]),
     ?line my_yes = error_logger:delete_report_handler(?MODULE),
     ok.
 
-generate_error() ->
-    erlang:error({badmatch,4}).
+generate_error(Error, Stack) ->
+    erlang:raise(error, Error, Stack).
 
 %%-----------------------------------------------------------------
 %% We don't enables or disables tty error logging here. We do not
@@ -283,15 +286,6 @@ reported(Tag, Type, Report) ->
 	    test_server:fail(no_report_received)
     end.
 
-reported(emulator) ->
-    receive
-	{error, "~s~n", String} when is_list(String) ->
-	    test_server:messages_get(),
-	    ok
-    after 1000 ->
-	    test_server:fail(no_report_received)
-    end.
-
 %%-----------------------------------------------------------------
 %% The error_logger handler (gen_event behaviour).
 %% Sends a notification to the Tester process about the events