File 1161-Don-t-keep-a-stacktrace-longer-than-necess... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 1161-Don-t-keep-a-stacktrace-longer-than-necessary.patch of Package erlang (Revision c698cd69bc2bea1d808e4837af05cc32)

Currently displaying revision c698cd69bc2bea1d808e4837af05cc32 , Show latest

From a65d0d571c13d632eed9b0a6b61052c1b4b758bb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Wed, 30 Jun 2021 12:32:04 +0200
Subject: [PATCH] Don't keep a stacktrace longer than necessary

After an exception had been caught, the stacktrace (`p->ftrace`) would
be retained in the process until another exception occurred.

While at it, also clear the exception term (`p->fvalue`) in the common
error handling code instead of in the code for `try_case` in each module.
---
 erts/emulator/beam/beam_common.c            | 10 +++++++---
 erts/emulator/beam/emu/instrs.tab           |  4 ++--
 erts/emulator/beam/jit/x86/instr_common.cpp | 20 ++++++++++++++++----
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/erts/emulator/beam/beam_common.c b/erts/emulator/beam/beam_common.c
index bc784f7cae..5eeb9a9807 100644
--- a/erts/emulator/beam/beam_common.c
+++ b/erts/emulator/beam/beam_common.c
@@ -506,11 +506,14 @@ handle_error(Process* c_p, ErtsCodePtr pc, Eterm* reg,
     /* Get the fully expanded error term */
     Value = expand_error_value(c_p, c_p->freason, Value);
 
-    /* Save final error term and stabilize the exception flags so no
-       further expansion is done. */
-    c_p->fvalue = Value;
+    /* Stabilize the exception flags so no further expansion is
+       done. */
     c_p->freason = PRIMARY_EXCEPTION(c_p->freason);
 
+    /* Clear out error term from process structure to avoid keeping
+       garbage. */
+    c_p->fvalue = NIL;
+
     /* Find a handler or die */
     if ((c_p->catches > 0 || IS_TRACED_FL(c_p, F_EXCEPTION_TRACE))
 	&& !(c_p->freason & EXF_PANIC)) {
@@ -546,6 +549,7 @@ handle_error(Process* c_p, ErtsCodePtr pc, Eterm* reg,
 	    /* No longer safe to use this position */
             erts_msgq_recv_marker_clear(c_p, erts_old_recv_marker_id);
 #endif
+            c_p->ftrace = NIL;
 	    return new_pc;
 	}
 	if (c_p->catches > 0) erts_exit(ERTS_ERROR_EXIT, "Catch not found");
diff --git a/erts/emulator/beam/emu/instrs.tab b/erts/emulator/beam/emu/instrs.tab
index b05533c830..caa52927aa 100644
--- a/erts/emulator/beam/emu/instrs.tab
+++ b/erts/emulator/beam/emu/instrs.tab
@@ -1124,8 +1124,8 @@ try_end(Y) {
 try_case(Y) {
     $try_end($Y);
     ASSERT(is_non_value(r(0)));
-    c_p->fvalue = NIL;
-    c_p->ftrace = NIL;
+    ASSERT(c_p->fvalue == NIL);
+    ASSERT(c_p->ftrace == NIL);
     r(0) = x(1);
     x(1) = x(2);
     x(2) = x(3);
diff --git a/erts/emulator/beam/jit/x86/instr_common.cpp b/erts/emulator/beam/jit/x86/instr_common.cpp
index 4ca7a10629..a8c368a4af 100644
--- a/erts/emulator/beam/jit/x86/instr_common.cpp
+++ b/erts/emulator/beam/jit/x86/instr_common.cpp
@@ -1644,7 +1644,6 @@ void BeamGlobalAssembler::emit_catch_end_shared() {
 
     /* Load thrown value / reason into ARG2 for add_stacktrace */
     a.mov(ARG2, getXRef(2));
-    a.mov(x86::qword_ptr(c_p, offsetof(Process, fvalue)), imm(NIL));
 
     a.cmp(getXRef(1), imm(am_throw));
     a.short_().jne(not_throw);
@@ -1721,13 +1720,26 @@ void BeamModuleAssembler::emit_try_end(const ArgVal &Y) {
 
 void BeamModuleAssembler::emit_try_case(const ArgVal &Y) {
     a.dec(x86::qword_ptr(c_p, offsetof(Process, catches)));
-    mov_imm(RET, NIL);
-    mov_arg(Y, RET);
-    a.mov(x86::qword_ptr(c_p, offsetof(Process, fvalue)), RET);
+    mov_arg(Y, NIL);
     a.movups(x86::xmm0, x86::xmmword_ptr(registers, 1 * sizeof(Eterm)));
     a.mov(RET, getXRef(3));
     a.movups(x86::xmmword_ptr(registers, 0 * sizeof(Eterm)), x86::xmm0);
     a.mov(getXRef(2), RET);
+
+#ifdef DEBUG
+    Label fvalue_ok = a.newLabel(), assertion_failed = a.newLabel();
+    comment("Start of assertion code");
+    a.cmp(x86::qword_ptr(c_p, offsetof(Process, fvalue)), NIL);
+    a.short_().je(fvalue_ok);
+
+    a.bind(assertion_failed);
+    comment("Assertion c_p->fvalue == NIL && c_p->ftrace == NIL failed");
+    a.ud2();
+
+    a.bind(fvalue_ok);
+    a.cmp(x86::qword_ptr(c_p, offsetof(Process, fvalue)), NIL);
+    a.short_().jne(assertion_failed);
+#endif
 }
 
 void BeamModuleAssembler::emit_try_case_end(const ArgVal &Src) {
-- 
2.26.2

Places

File 1161-Don-t-keep-a-stacktrace-longer-than-necessary.patch of Package erlang (Revision c698cd69bc2bea1d808e4837af05cc32)

Places