Merge pull request atomvm#1959 from bettio/error-handling-rework

Error handling rework

This PR fixes 2 main issues and limitations in exception handling.

- re-rase now works (see also atomvm#1951)
- when a function head doesn't match, arguments are now visible in stacktrace
- when a bad argument is given to a NIF/BIF, arguments are visible in
stacktrace too, this has been implemented starting from
`erlang:integer_to_binary`

This is a huge leap forward in terms of developer experience when debugging
errors.

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

Author: bettio

CHANGELOG.md

@@ -90,6 +90,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 instead `badarg`.
 - Resources are now references instead of empty binaries.
 - Badarg error return from calling crypto:crypto_one_time with invalid arguments now matches OTP24+.
+- When function head doesn't match, function arguments are now in stacktrace
+- Function arguments are added to stacktrace also for some NIFs, when one of the arguments is badarg
 
 ### Fixed
 
@@ -106,6 +108,7 @@ instead `badarg`.
 - Fix supervisor crash if a `one_for_one` child fails to restart.
 - Fix collision in references created with `make_ref/0` on 32 bits platforms.
 - Fixed a bug in `OP_BS_CREATE_BIN`
+- Fix re-raise behavior by implementing `erlang:raise/3` 3rd argument support
 
 ## [0.6.7] - Unreleased
 

libs/jit/src/jit.erl

@@ -175,28 +175,37 @@ first_pass(
 % 2
 first_pass(<<?OP_FUNC_INFO, Rest0/binary>>, MMod, MSt0, #state{tail_cache = TC} = State0) ->
     ?ASSERT_ALL_NATIVE_FREE(MSt0),
-    {_ModuleAtom, Rest1} = decode_atom(Rest0),
-    {_FunctionName, Rest2} = decode_atom(Rest1),
-    {_Arity, Rest3} = decode_literal(Rest2),
-    ?TRACE("OP_FUNC_INFO ~p, ~p, ~p\n", [_ModuleAtom, _FunctionName, _Arity]),
-    % Implement function clause at the previous label.
+    {_ModuleAtomIndex, Rest1} = decode_atom(Rest0),
+    {FunctionAtomIndex, Rest2} = decode_atom(Rest1),
+    {Arity, Rest3} = decode_literal(Rest2),
+    ?TRACE("OP_FUNC_INFO ~p, ~p, ~p\n", [_ModuleAtomIndex, FunctionAtomIndex, Arity]),
     Offset = MMod:offset(MSt0),
     {MSt1, OffsetReg} = MMod:move_to_native_register(MSt0, Offset),
-    TailCacheKey = {call_primitive_last, ?PRIM_RAISE_ERROR, [OffsetReg, ?FUNCTION_CLAUSE_ATOM]},
-    State1 =
+    {MSt2, FunctionAtomIndexReg} = MMod:move_to_native_register(MSt1, FunctionAtomIndex),
+    {MSt3, ArityReg} = MMod:move_to_native_register(MSt2, Arity),
+    TailCacheKey =
+        {call_primitive_last, ?PRIM_RAISE_ERROR_MFA, [OffsetReg, FunctionAtomIndexReg, ArityReg]},
+    {MSt4, State1} =
         case lists:keyfind(TailCacheKey, 1, TC) of
             false ->
-                MSt3 = MMod:call_primitive_last(MSt1, ?PRIM_RAISE_ERROR, [
-                    ctx, jit_state, {free, OffsetReg}, ?FUNCTION_CLAUSE_ATOM
+                CacheOffset = MMod:offset(MSt3),
+                MSt4a = MMod:call_primitive_last(MSt3, ?PRIM_RAISE_ERROR_MFA, [
+                    ctx,
+                    jit_state,
+                    {free, OffsetReg},
+                    {free, FunctionAtomIndexReg},
+                    {free, ArityReg}
                 ]),
-                State0#state{tail_cache = [{TailCacheKey, Offset} | TC]};
+                {MSt4a, State0#state{tail_cache = [{TailCacheKey, CacheOffset} | TC]}};
             {TailCacheKey, CacheOffset} ->
-                MSt2 = MMod:jump_to_offset(MSt1, CacheOffset),
-                MSt3 = MMod:free_native_registers(MSt2, [OffsetReg]),
-                State0
+                MSt4a = MMod:jump_to_offset(MSt3, CacheOffset),
+                MSt4b = MMod:free_native_registers(MSt4a, [
+                    OffsetReg, FunctionAtomIndexReg, ArityReg
+                ]),
+                {MSt4b, State0}
         end,
-    ?ASSERT_ALL_NATIVE_FREE(MSt3),
-    first_pass(Rest3, MMod, MSt3, State1);
+    ?ASSERT_ALL_NATIVE_FREE(MSt4),
+    first_pass(Rest3, MMod, MSt4, State1);
 % 3
 first_pass(
     <<?OP_INT_CALL_END>>, MMod, MSt0, #state{labels_count = LabelsCount} = State
@@ -276,7 +285,7 @@ first_pass(<<?OP_CALL_EXT, Rest0/binary>>, MMod, MSt0, State0) ->
     ?TRACE("OP_CALL_EXT ~p, ~p\n", [Arity, Index]),
     MSt1 = MMod:decrement_reductions_and_maybe_schedule_next(MSt0),
     MSt2 = MMod:call_primitive_with_cp(MSt1, ?PRIM_CALL_EXT, [
-        ctx, jit_state, offset, Arity, Index, -1
+        ctx, jit_state, offset, Arity, Index, ?CALL_EXT_NO_DEALLOC_MFA
     ]),
     ?ASSERT_ALL_NATIVE_FREE(MSt2),
     first_pass(Rest2, MMod, MSt2, State0);
@@ -1034,7 +1043,9 @@ first_pass(<<?OP_CALL_EXT_ONLY, Rest0/binary>>, MMod, MSt0, State0) ->
     {Index, Rest2} = decode_literal(Rest1),
     ?TRACE("OP_CALL_EXT_ONLY ~p, ~p\n", [Arity, Index]),
     MSt1 = MMod:decrement_reductions_and_maybe_schedule_next(MSt0),
-    MSt2 = MMod:call_primitive_last(MSt1, ?PRIM_CALL_EXT, [ctx, jit_state, offset, Arity, Index, -1]),
+    MSt2 = MMod:call_primitive_last(MSt1, ?PRIM_CALL_EXT, [
+        ctx, jit_state, offset, Arity, Index, ?CALL_EXT_NO_DEALLOC
+    ]),
     ?ASSERT_ALL_NATIVE_FREE(MSt2),
     first_pass(Rest2, MMod, MSt2, State0);
 % 96
@@ -1154,7 +1165,7 @@ first_pass(<<?OP_RAISE, Rest0/binary>>, MMod, MSt0, State0) ->
     {MSt2, ExcValue, Rest2} = decode_compact_term(Rest1, MMod, MSt1, State0),
     ?TRACE("OP_RAISE ~p, ~p\n", [Stacktrace, ExcValue]),
     MSt3 = MMod:call_primitive_last(MSt2, ?PRIM_RAISE, [
-        ctx, jit_state, offset, Stacktrace, ExcValue
+        ctx, jit_state, Stacktrace, ExcValue
     ]),
     ?ASSERT_ALL_NATIVE_FREE(MSt3),
     first_pass(Rest2, MMod, MSt3, State0);
@@ -2020,13 +2031,13 @@ first_pass(<<?OP_RAW_RAISE, Rest0/binary>>, MMod, MSt0, State0) ->
     ?ASSERT_ALL_NATIVE_FREE(MSt0),
     {MSt1, ExClassReg} = MMod:move_to_native_register(MSt0, {x_reg, 0}),
     MSt2 = MMod:if_block(MSt1, {ExClassReg, '==', ?ERROR_ATOM}, fun(BSt0) ->
-        MMod:call_primitive_last(BSt0, ?PRIM_HANDLE_ERROR, [ctx, jit_state, offset])
+        MMod:call_primitive_last(BSt0, ?PRIM_RAW_RAISE, [ctx, jit_state])
     end),
     MSt3 = MMod:if_block(MSt2, {ExClassReg, '==', ?LOWERCASE_EXIT_ATOM}, fun(BSt0) ->
-        MMod:call_primitive_last(BSt0, ?PRIM_HANDLE_ERROR, [ctx, jit_state, offset])
+        MMod:call_primitive_last(BSt0, ?PRIM_RAW_RAISE, [ctx, jit_state])
     end),
     MSt4 = MMod:if_block(MSt3, {{free, ExClassReg}, '==', ?THROW_ATOM}, fun(BSt0) ->
-        MMod:call_primitive_last(BSt0, ?PRIM_HANDLE_ERROR, [ctx, jit_state, offset])
+        MMod:call_primitive_last(BSt0, ?PRIM_RAW_RAISE, [ctx, jit_state])
     end),
     MSt5 = MMod:move_to_vm_register(MSt4, ?BADARG_ATOM, {x_reg, 0}),
     ?ASSERT_ALL_NATIVE_FREE(MSt5),

libs/jit/src/primitives.hrl

@@ -95,13 +95,20 @@
 -define(PRIM_TERM_REUSE_BINARY, 72).
 -define(PRIM_ALLOC_BIG_INTEGER_FRAGMENT, 73).
 -define(PRIM_BITSTRING_INSERT_FLOAT, 74).
+-define(PRIM_RAW_RAISE, 75).
+-define(PRIM_RAISE_ERROR_MFA, 76).
 
 % Parameters to ?PRIM_MEMORY_ENSURE_FREE_WITH_ROOTS
 % -define(MEMORY_NO_SHRINK, 0).
 -define(MEMORY_CAN_SHRINK, 1).
 % -define(MEMORY_FORCE_SHRINK, 2).
 % -define(MEMORY_NO_GC, 3).
 
+% n_words parameter values for ?PRIM_CALL_EXT
+% n_words >= 0 means CALL_EXT_LAST (deallocate n_words from stack)
+-define(CALL_EXT_NO_DEALLOC, -1).
+-define(CALL_EXT_NO_DEALLOC_MFA, -2).
+
 % term_integer_sign_t sign parameter for PRIM_ALLOC_BIG_INTEGER_FRAGMENT
 -define(TERM_POSITIVE_INTEGER, 0).
 -define(TERM_NEGATIVE_INTEGER, 4).

src/libAtomVM/bif.c

@@ -43,18 +43,18 @@
 #include "bifs_hash.h"
 #pragma GCC diagnostic pop
 
-#define RAISE_ERROR(error_type_atom)   \
-    do {                               \
-        ctx->x[0] = ERROR_ATOM;        \
-        ctx->x[1] = (error_type_atom); \
-        return term_invalid_term();    \
+#define RAISE_ERROR(error_type_atom)                  \
+    do {                                              \
+        context_set_exception_class(ctx, ERROR_ATOM); \
+        ctx->exception_reason = (error_type_atom);    \
+        return term_invalid_term();                   \
     } while (0);
 
-#define RAISE_ERROR_BIF(fail_label, error_type_atom) \
-    if (fail_label == 0) {                           \
-        ctx->x[0] = ERROR_ATOM;                      \
-        ctx->x[1] = (error_type_atom);               \
-    }                                                \
+#define RAISE_ERROR_BIF(fail_label, error_type_atom)  \
+    if (fail_label == 0) {                            \
+        context_set_exception_class(ctx, ERROR_ATOM); \
+        ctx->exception_reason = (error_type_atom);    \
+    }                                                 \
     return term_invalid_term();
 
 #define VALIDATE_VALUE_BIF(fail_label, value, verify_function) \

src/libAtomVM/context.c

@@ -123,6 +123,10 @@ Context *context_new(GlobalContext *glb)
     ctx->bs = term_invalid_term();
     ctx->bs_offset = 0;
 
+    context_set_exception_class(ctx, term_nil());
+    ctx->exception_reason = term_nil();
+    ctx->exception_stacktrace = term_nil();
+
     ctx->exit_reason = NORMAL_ATOM;
 
     globalcontext_init_process(glb, ctx);

src/libAtomVM/context.h

@@ -158,6 +158,10 @@ struct Context
     term group_leader;
 
     term exit_reason;
+
+    uintptr_t exception_class;
+    term exception_reason;
+    term exception_stacktrace;
 };
 
 #ifndef TYPEDEF_CONTEXT
@@ -620,6 +624,32 @@ int context_get_catch_label(Context *ctx, Module **mod);
  */
 void context_dump(Context *ctx);
 
+enum
+{
+    EXCEPTION_USE_LIVE_REGS_FLAG = 1
+};
+
+static inline term context_exception_class(const Context *ctx)
+{
+    return (ctx->exception_class & ~TERM_IMMED2_TAG_MASK) | TERM_IMMED2_ATOM;
+}
+
+static inline void context_set_exception_class(Context *ctx, term exception_class)
+{
+    ctx->exception_class = exception_class & ~TERM_IMMED2_TAG_MASK;
+}
+
+static inline void context_set_exception_class_use_live_flag(Context *ctx, term exception_class)
+{
+    ctx->exception_class
+        = (exception_class & ~TERM_IMMED2_TAG_MASK) | EXCEPTION_USE_LIVE_REGS_FLAG;
+}
+
+static inline bool context_exception_class_has_live_regs_flag(const Context *ctx)
+{
+    return (ctx->exception_class & EXCEPTION_USE_LIVE_REGS_FLAG) != 0;
+}
+
 #ifdef __cplusplus
 }
 #endif