From 5959fb00182048d50f55d04684723d383d1aeee8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Thu, 23 May 2019 07:22:41 +0200
Subject: Fix sticky class in exception

When catching an exception re-throwing with a changed
class, the class could be changed to the original class
if the exception got caught and rethrown in (for example)
an after block:

    sticky_class() ->
        try
            try
                throw(reason)
            catch
                throw:Reason:Stack ->
                    erlang:raise(error, Reason, Stack)
            end
        after
            ok
        end.
---
 erts/emulator/beam/beam_emu.c          | 20 ++++++++++++++++++++
 erts/emulator/beam/instrs.tab          | 17 ++++++++++++++---
 erts/emulator/test/exception_SUITE.erl | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/erts/emulator/beam/beam_emu.c b/erts/emulator/beam/beam_emu.c
index 3af0838794..ec4f9b4339 100644
--- a/erts/emulator/beam/beam_emu.c
+++ b/erts/emulator/beam/beam_emu.c
@@ -422,6 +422,7 @@ static Eterm add_stacktrace(Process* c_p, Eterm Value, Eterm exc);
 static void save_stacktrace(Process* c_p, BeamInstr* pc, Eterm* reg,
 			    ErtsCodeMFA *bif_mfa, Eterm args);
 static struct StackTrace * get_trace_from_exc(Eterm exc);
+static Eterm *get_freason_ptr_from_exc(Eterm exc);
 static Eterm make_arglist(Process* c_p, Eterm* reg, int a);
 
 void
@@ -1904,6 +1905,25 @@ static int is_raised_exc(Eterm exc) {
     }
 }
 
+static Eterm *get_freason_ptr_from_exc(Eterm exc) {
+    static Eterm dummy_freason;
+    struct StackTrace* s;
+
+    if (exc == NIL) {
+        /*
+         * Is is not exactly clear when exc can be NIL. Probably only
+         * when the exception has been generated from native code.
+         * Return a pointer to an Eterm that can be safely written and
+         * ignored.
+         */
+	return &dummy_freason;
+    } else {
+	ASSERT(is_list(exc));
+        s = (struct StackTrace *) big_val(CDR(list_val(exc)));
+        return &s->freason;
+    }
+}
+
 /*
  * Creating a list with the argument registers
  */
diff --git a/erts/emulator/beam/instrs.tab b/erts/emulator/beam/instrs.tab
index 42c1168f85..999e9337ff 100644
--- a/erts/emulator/beam/instrs.tab
+++ b/erts/emulator/beam/instrs.tab
@@ -951,19 +951,30 @@ raw_raise() {
     Eterm class = x(0);
     Eterm value = x(1);
     Eterm stacktrace = x(2);
+    Eterm* freason_ptr;
+
+    /*
+     * Note that the i_raise instruction will override c_p->freason
+     * with the freason field stored inside the StackTrace struct in
+     * ftrace. Therefore, we must take care to store the class both
+     * inside the StackTrace struct and in c_p->freason (important if
+     * the class is different from the class of the original
+     * exception).
+     */
+    freason_ptr = get_freason_ptr_from_exc(stacktrace);
 
     if (class == am_error) {
-        c_p->freason = EXC_ERROR & ~EXF_SAVETRACE;
+        *freason_ptr = c_p->freason = EXC_ERROR & ~EXF_SAVETRACE;
         c_p->fvalue = value;
         c_p->ftrace = stacktrace;
         goto find_func_info;
     } else if (class == am_exit) {
-        c_p->freason = EXC_EXIT & ~EXF_SAVETRACE;
+        *freason_ptr = c_p->freason = EXC_EXIT & ~EXF_SAVETRACE;
         c_p->fvalue = value;
         c_p->ftrace = stacktrace;
         goto find_func_info;
     } else if (class == am_throw) {
-        c_p->freason = EXC_THROWN & ~EXF_SAVETRACE;
+        *freason_ptr = c_p->freason = EXC_THROWN & ~EXF_SAVETRACE;
         c_p->fvalue = value;
         c_p->ftrace = stacktrace;
         goto find_func_info;
diff --git a/erts/emulator/test/exception_SUITE.erl b/erts/emulator/test/exception_SUITE.erl
index aec66cb9a3..6e6f7d78ab 100644
--- a/erts/emulator/test/exception_SUITE.erl
+++ b/erts/emulator/test/exception_SUITE.erl
@@ -23,6 +23,7 @@
 -export([all/0, suite/0,
          badmatch/1, pending_errors/1, nil_arith/1, top_of_stacktrace/1,
          stacktrace/1, nested_stacktrace/1, raise/1, gunilla/1, per/1,
+         change_exception_class/1,
          exception_with_heap_frag/1, backtrace_depth/1,
          line_numbers/1]).
 
@@ -43,6 +44,7 @@ suite() ->
 all() -> 
     [badmatch, pending_errors, nil_arith, top_of_stacktrace,
      stacktrace, nested_stacktrace, raise, gunilla, per,
+     change_exception_class,
      exception_with_heap_frag, backtrace_depth, line_numbers].
 
 -define(try_match(E),
@@ -507,6 +509,38 @@ t1(_,X,_) ->
 t2(_,X,_) ->
     (X bsl 1) + 1.
 
+change_exception_class(_Config) ->
+    try
+        change_exception_class_1(fun() -> throw(arne) end)
+    catch
+        error:arne ->
+            ok;
+        Class:arne ->
+            ct:fail({wrong_exception_class,Class})
+    end.
+
+change_exception_class_1(F) ->
+    try
+        change_exception_class_2(F)
+    after
+        %% The exception would be caught and rethrown using
+        %% an i_raise instruction. Before the correction
+        %% of the raw_raise instruction, the change of class
+        %% would not stick.
+        io:put_chars("Exception automatically rethrown here\n")
+    end.
+
+change_exception_class_2(F) ->
+    try
+        F()
+    catch
+        throw:Reason:Stack ->
+            %% Translated to a raw_raise instruction.
+            %% The change of exception class would not stick
+            %% if the i_raise instruction was later executed.
+            erlang:raise(error, Reason, Stack)
+    end.
+
 %%
 %% Make sure that even if a BIF builds an heap fragment, then causes an exception,
 %% the stacktrace term will still be OK (specifically, that it does not contain
-- 
cgit v1.2.3