Skip to content

Commit 85dce1e

Browse files
bettiobettio
committed
Instead of using x[0..2] registers, store exception on dedicated ctx fields
Store exception information in context fields: - exception_class (was `x[0]` register) - exception_reason (was `x[1]` register) - exception_stacktrace (was `x[2]` register). This avoids overwriting function arguments when raising an error, allowing better diagnostic, also this should allow the fix of #1951 and proposes an alternative approach to #1804. Signed-off-by: Davide Bettio <[email protected]>
1 parent 012ba89 commit 85dce1e

File tree

8 files changed

+74
-52
lines changed

8 files changed

+74
-52
lines changed

‎src/libAtomVM/bif.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,15 @@
4343
#include "bifs_hash.h"
4444
#pragma GCC diagnostic pop
4545

46-
#define RAISE_ERROR(error_type_atom) \
47-
ctx->x[0] = ERROR_ATOM; \
48-
ctx->x[1] = (error_type_atom); \
46+
#define RAISE_ERROR(error_type_atom) \
47+
ctx->exception_class = ERROR_ATOM; \
48+
ctx->exception_reason = (error_type_atom); \
4949
return term_invalid_term();
5050

5151
#define RAISE_ERROR_BIF(fail_label, error_type_atom) \
5252
if (fail_label == 0) { \
53-
ctx->x[0] = ERROR_ATOM; \
54-
ctx->x[1] = (error_type_atom); \
53+
ctx->exception_class = ERROR_ATOM; \
54+
ctx->exception_reason = (error_type_atom); \
5555
} \
5656
return term_invalid_term();
5757

‎src/libAtomVM/context.c‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ Context *context_new(GlobalContext *glb)
123123
ctx->bs = term_invalid_term();
124124
ctx->bs_offset = 0;
125125

126+
ctx->exception_class = term_nil();
127+
ctx->exception_reason = term_nil();
128+
ctx->exception_stacktrace = term_nil();
129+
126130
ctx->exit_reason = NORMAL_ATOM;
127131

128132
globalcontext_init_process(glb, ctx);
@@ -1171,12 +1175,12 @@ COLD_FUNC void context_dump(Context *ctx)
11711175
cp_mod->module_index, label, offset);
11721176
}
11731177

1174-
fprintf(stderr, "x[0]: ");
1175-
term_display(stderr, ctx->x[0], ctx);
1176-
fprintf(stderr, "\nx[1]: ");
1177-
term_display(stderr, ctx->x[1], ctx);
1178-
fprintf(stderr, "\nx[2]: ");
1179-
term_display(stderr, ctx->x[2], ctx);
1178+
fprintf(stderr, "Exception:\n- Class: ");
1179+
term_display(stderr, ctx->exception_class, ctx);
1180+
fprintf(stderr, "\n- Reason: ");
1181+
term_display(stderr, ctx->exception_reason, ctx);
1182+
fprintf(stderr, "\n- Stacktrace: ");
1183+
term_display(stderr, ctx->exception_stacktrace, ctx);
11801184
fprintf(stderr, "\n\nStack \n-----\n\n");
11811185

11821186
term *ct = ctx->e;

‎src/libAtomVM/context.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,10 @@ struct Context
157157
term group_leader;
158158

159159
term exit_reason;
160+
161+
term exception_class;
162+
term exception_reason;
163+
term exception_stacktrace;
160164
};
161165

162166
#ifndef TYPEDEF_CONTEXT

‎src/libAtomVM/memory.c‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,13 @@ static enum MemoryGCResult memory_gc(Context *ctx, size_t new_size, size_t num_r
298298
TRACE("- Running copy GC on exit reason\n");
299299
ctx->exit_reason = memory_shallow_copy_term(old_root_fragment, ctx->exit_reason, &ctx->heap.heap_ptr, true);
300300

301+
TRACE("- Running copy GC on exception data\n");
302+
// ctx->exception_class is always an atom
303+
ctx->exception_reason = memory_shallow_copy_term(
304+
old_root_fragment, ctx->exception_reason, &ctx->heap.heap_ptr, true);
305+
ctx->exception_stacktrace = memory_shallow_copy_term(
306+
old_root_fragment, ctx->exception_stacktrace, &ctx->heap.heap_ptr, true);
307+
301308
TRACE("- Running copy GC on group leader\n");
302309
ctx->group_leader = memory_shallow_copy_term(old_root_fragment, ctx->group_leader, &ctx->heap.heap_ptr, true);
303310

‎src/libAtomVM/nifs.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,9 @@
6868

6969
#define FLOAT_BUF_SIZE 64
7070

71-
#define RAISE(a, b) \
72-
ctx->x[0] = (a); \
73-
ctx->x[1] = (b); \
71+
#define RAISE(a, b) \
72+
ctx->exception_class = (a); \
73+
ctx->exception_reason = (b); \
7474
return term_invalid_term();
7575

7676
#ifndef MAX

‎src/libAtomVM/nifs.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,14 @@ extern "C" {
3636

3737
#define VALIDATE_VALUE(value, verify_function) \
3838
if (UNLIKELY(!verify_function((value)))) { \
39-
argv[0] = ERROR_ATOM; \
40-
argv[1] = BADARG_ATOM; \
39+
ctx->exception_class = ERROR_ATOM; \
40+
ctx->exception_reason = BADARG_ATOM; \
4141
return term_invalid_term(); \
4242
}
4343

44-
#define RAISE_ERROR(error_type_atom) \
45-
ctx->x[0] = ERROR_ATOM; \
46-
ctx->x[1] = (error_type_atom); \
44+
#define RAISE_ERROR(error_type_atom) \
45+
ctx->exception_class = ERROR_ATOM; \
46+
ctx->exception_reason = (error_type_atom); \
4747
return term_invalid_term();
4848

4949
const struct Nif *nifs_get(const char *mfa);

‎src/libAtomVM/opcodesswitch.h‎

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -59,23 +59,24 @@ extern "C" {
5959
#ifdef IMPL_EXECUTE_LOOP
6060

6161
#if AVM_NO_JIT
62-
#define SET_ERROR(error_type_atom) \
63-
x_regs[0] = ERROR_ATOM; \
64-
x_regs[1] = error_type_atom; \
65-
x_regs[2] = stacktrace_create_raw(ctx, mod, pc - code, ERROR_ATOM);
62+
#define SET_ERROR(error_type_atom) \
63+
ctx->exception_class = ERROR_ATOM; \
64+
ctx->exception_reason = error_type_atom; \
65+
ctx->exception_stacktrace = stacktrace_create_raw(ctx, mod, pc - code, ERROR_ATOM);
6666
#elif AVM_NO_EMU
67-
#define SET_ERROR(error_type_atom) \
68-
x_regs[0] = ERROR_ATOM; \
69-
x_regs[1] = error_type_atom; \
70-
x_regs[2] = stacktrace_create_raw(ctx, mod, (const uint8_t *) native_pc - (const uint8_t *) mod->native_code, ERROR_ATOM);
67+
#define SET_ERROR(error_type_atom) \
68+
ctx->exception_class = ERROR_ATOM; \
69+
ctx->exception_reason = error_type_atom; \
70+
ctx->exception_stacktrace = stacktrace_create_raw(ctx, mod, (const uint8_t *) native_pc - (const uint8_t *) mod->native_code, ERROR_ATOM);
7171
#else
72-
#define SET_ERROR(error_type_atom) \
73-
x_regs[0] = ERROR_ATOM; \
74-
x_regs[1] = error_type_atom; \
75-
if (mod->native_code) { \
76-
x_regs[2] = stacktrace_create_raw(ctx, mod, (const uint8_t *) native_pc - (const uint8_t *) mod->native_code, ERROR_ATOM); \
77-
} else { \
78-
x_regs[2] = stacktrace_create_raw(ctx, mod, pc - code, ERROR_ATOM); \
72+
#define SET_ERROR(error_type_atom) \
73+
ctx->exception_class = ERROR_ATOM; \
74+
ctx->exception_reason = error_type_atom; \
75+
if (mod->native_code) { \
76+
ctx->exception_stacktrace = stacktrace_create_raw(ctx, mod, \
77+
(const uint8_t *) native_pc - (const uint8_t *) mod->native_code, ERROR_ATOM); \
78+
} else { \
79+
ctx->exception_stacktrace = stacktrace_create_raw(ctx, mod, pc - code, ERROR_ATOM); \
7980
}
8081
#endif
8182

@@ -1312,7 +1313,7 @@ static void destroy_extended_registers(Context *ctx, unsigned int live)
13121313
#endif
13131314

13141315
#define HANDLE_ERROR() \
1315-
x_regs[2] = stacktrace_create_raw(ctx, mod, pc - code, x_regs[0]); \
1316+
ctx->exception_stacktrace = stacktrace_create_raw(ctx, mod, pc - code, ctx->exception_class); \
13161317
goto handle_error;
13171318

13181319
#define VERIFY_IS_INTEGER(t, opcode_name, label) \
@@ -3973,6 +3974,9 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
39733974
#ifdef IMPL_EXECUTE_LOOP
39743975
// clears the catch value on stack
39753976
WRITE_REGISTER(dreg, term_nil());
3977+
ctx->x[0] = ctx->exception_class;
3978+
ctx->x[1] = ctx->exception_reason;
3979+
ctx->x[2] = ctx->exception_stacktrace;
39763980
#endif
39773981
break;
39783982
}
@@ -4020,9 +4024,9 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
40204024

40214025
#ifdef IMPL_EXECUTE_LOOP
40224026
TRACE("raise/2 stacktrace=0x%" TERM_X_FMT " exc_value=0x%" TERM_X_FMT "\n", stacktrace, exc_value);
4023-
x_regs[0] = stacktrace_exception_class(stacktrace);
4024-
x_regs[1] = exc_value;
4025-
x_regs[2] = stacktrace_create_raw(ctx, mod, saved_pc - code, x_regs[0]);
4027+
ctx->exception_class = stacktrace_exception_class(stacktrace);
4028+
ctx->exception_reason = exc_value;
4029+
ctx->exception_stacktrace = stacktrace_create_raw(ctx, mod, saved_pc - code, ctx->exception_class);
40264030
goto handle_error;
40274031
#endif
40284032

@@ -4053,19 +4057,20 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
40534057
#ifdef IMPL_EXECUTE_LOOP
40544058
WRITE_REGISTER(dreg, term_nil());
40554059
// C.f. https://www.erlang.org/doc/reference_manual/expressions.html#catch-and-throw
4056-
switch (term_to_atom_index(x_regs[0])) {
4060+
switch (term_to_atom_index(ctx->exception_class)) {
40574061
case THROW_ATOM_INDEX:
4058-
x_regs[0] = x_regs[1];
4062+
x_regs[0] = ctx->exception_reason;
40594063
break;
40604064

40614065
case ERROR_ATOM_INDEX: {
4066+
x_regs[2] = ctx->exception_stacktrace;
40624067
x_regs[2] = stacktrace_build(ctx, &x_regs[2], 3);
40634068
// MEMORY_CAN_SHRINK because catch_end is classified as gc in beam_ssa_codegen.erl
40644069
if (UNLIKELY(memory_ensure_free_with_roots(ctx, TUPLE_SIZE(2) * 2, 2, x_regs + 1, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
40654070
RAISE_ERROR(OUT_OF_MEMORY_ATOM);
40664071
}
40674072
term reason_tuple = term_alloc_tuple(2, &ctx->heap);
4068-
term_put_tuple_element(reason_tuple, 0, x_regs[1]);
4073+
term_put_tuple_element(reason_tuple, 0, ctx->exception_reason);
40694074
term_put_tuple_element(reason_tuple, 1, x_regs[2]);
40704075
term exit_tuple = term_alloc_tuple(2, &ctx->heap);
40714076
term_put_tuple_element(exit_tuple, 0, EXIT_ATOM);
@@ -4081,7 +4086,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
40814086
}
40824087
term exit_tuple = term_alloc_tuple(2, &ctx->heap);
40834088
term_put_tuple_element(exit_tuple, 0, EXIT_ATOM);
4084-
term_put_tuple_element(exit_tuple, 1, x_regs[1]);
4089+
term_put_tuple_element(exit_tuple, 1, ctx->exception_reason);
40854090
x_regs[0] = exit_tuple;
40864091

40874092
break;
@@ -6569,9 +6574,8 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
65696574
TRACE("build_stacktrace/0\n");
65706575

65716576
#ifdef IMPL_EXECUTE_LOOP
6572-
6577+
x_regs[0] = ctx->exception_stacktrace;
65736578
x_regs[0] = stacktrace_build(ctx, &x_regs[0], 1);
6574-
65756579
#endif
65766580
break;
65776581
}
@@ -6590,6 +6594,9 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
65906594
ex_class != THROW_ATOM)) {
65916595
x_regs[0] = BADARG_ATOM;
65926596
} else {
6597+
ctx->exception_class = x_regs[0];
6598+
ctx->exception_reason = x_regs[1];
6599+
ctx->exception_stacktrace = x_regs[2];
65936600
goto handle_error;
65946601
}
65956602
#endif
@@ -7567,15 +7574,15 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
75677574

75687575
#ifdef AVM_PRINT_PROCESS_CRASH_DUMPS
75697576
// Do not print crash dump if reason is normal or shutdown.
7570-
if (x_regs[0] != LOWERCASE_EXIT_ATOM || (x_regs[1] != NORMAL_ATOM && x_regs[1] != SHUTDOWN_ATOM)) {
7577+
if (ctx->exception_class != LOWERCASE_EXIT_ATOM || (ctx->exception_reason != NORMAL_ATOM && ctx->exception_reason != SHUTDOWN_ATOM)) {
75717578
context_dump(ctx);
75727579
}
75737580
#endif
75747581

7575-
if (x_regs[0] == LOWERCASE_EXIT_ATOM) {
7576-
ctx->exit_reason = x_regs[1];
7582+
if (ctx->exception_class == LOWERCASE_EXIT_ATOM) {
7583+
ctx->exit_reason = ctx->exception_reason;
75777584
} else {
7578-
bool throw = ctx->x[0] == THROW_ATOM;
7585+
bool throw = ctx->exception_class == THROW_ATOM;
75797586

75807587
int exit_reason_tuple_size = (throw ? TUPLE_SIZE(2) : 0) + TUPLE_SIZE(2);
75817588
if (memory_ensure_free_with_roots(ctx, exit_reason_tuple_size, 1, x_regs + 1, MEMORY_CAN_SHRINK) != MEMORY_GC_OK) {
@@ -7585,10 +7592,10 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
75857592
if (throw) {
75867593
error_term = term_alloc_tuple(2, &ctx->heap);
75877594
term_put_tuple_element(error_term, 0, NOCATCH_ATOM);
7588-
term_put_tuple_element(error_term, 1, x_regs[1]);
7595+
term_put_tuple_element(error_term, 1, ctx->exception_reason);
75897596
} else {
75907597
// error
7591-
error_term = x_regs[1];
7598+
error_term = ctx->exception_reason;
75927599
}
75937600

75947601
term exit_reason_tuple = term_alloc_tuple(2, &ctx->heap);

‎src/libAtomVM/stacktrace.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,8 @@ term stacktrace_create_raw(Context *ctx, Module *mod, int current_offset, term e
168168

169169
// {num_frames, num_aux_terms, filename_lens, num_mods, [{module, offset}, ...]}
170170
size_t requested_size = TUPLE_SIZE(6) + num_frames * (2 + TUPLE_SIZE(2));
171-
// We need to preserve x0 and x1 that contain information on the current exception
172-
if (UNLIKELY(memory_ensure_free_with_roots(ctx, requested_size, 2, ctx->x, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
171+
// TODO: we need to know which registers contain function call arguments and save them
172+
if (UNLIKELY(memory_ensure_free_opt(ctx, requested_size, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
173173
fprintf(stderr, "WARNING: Unable to allocate heap space for raw stacktrace\n");
174174
return OUT_OF_MEMORY_ATOM;
175175
}

0 commit comments

Comments
 (0)