Skip to content

Commit e8830d4

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. Also, simplify stacktrace_build(): it requires just Context * parameter. Signed-off-by: Davide Bettio <[email protected]>
1 parent f744fd4 commit e8830d4

File tree

9 files changed

+92
-71
lines changed

9 files changed

+92
-71
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: 12 additions & 8 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);
@@ -1159,7 +1163,7 @@ COLD_FUNC void context_dump(Context *ctx)
11591163
fprintf(stderr, "\n");
11601164

11611165
fprintf(stderr, "\nStacktrace:\n");
1162-
term_display(stderr, stacktrace_build(ctx, &ctx->x[2], 3), ctx);
1166+
term_display(stderr, stacktrace_build(ctx), ctx);
11631167
fprintf(stderr, "\n\n");
11641168

11651169
{
@@ -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;
@@ -1272,7 +1276,7 @@ COLD_FUNC void context_dump(Context *ctx)
12721276
synclist_unlock(&glb->processes_table);
12731277

12741278
// If crash is caused by out_of_memory, print more data about memory usage
1275-
if (ctx->x[0] == ERROR_ATOM && ctx->x[1] == OUT_OF_MEMORY_ATOM) {
1279+
if (ctx->exception_class == ERROR_ATOM && ctx->exception_reason == OUT_OF_MEMORY_ATOM) {
12761280
fprintf(stderr, "\n\nContext memory info\n-------------------\n");
12771281
fprintf(stderr, "context_size = %zu\n", context_size(ctx));
12781282
fprintf(stderr, "context_avail_free_memory = %zu\n", context_avail_free_memory(ctx));

‎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 & 33 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,19 @@ 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: {
4062-
x_regs[2] = stacktrace_build(ctx, &x_regs[2], 3);
4066+
x_regs[2] = stacktrace_build(ctx);
40634067
// MEMORY_CAN_SHRINK because catch_end is classified as gc in beam_ssa_codegen.erl
40644068
if (UNLIKELY(memory_ensure_free_with_roots(ctx, TUPLE_SIZE(2) * 2, 2, x_regs + 1, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
40654069
RAISE_ERROR(OUT_OF_MEMORY_ATOM);
40664070
}
40674071
term reason_tuple = term_alloc_tuple(2, &ctx->heap);
4068-
term_put_tuple_element(reason_tuple, 0, x_regs[1]);
4072+
term_put_tuple_element(reason_tuple, 0, ctx->exception_reason);
40694073
term_put_tuple_element(reason_tuple, 1, x_regs[2]);
40704074
term exit_tuple = term_alloc_tuple(2, &ctx->heap);
40714075
term_put_tuple_element(exit_tuple, 0, EXIT_ATOM);
@@ -4081,7 +4085,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
40814085
}
40824086
term exit_tuple = term_alloc_tuple(2, &ctx->heap);
40834087
term_put_tuple_element(exit_tuple, 0, EXIT_ATOM);
4084-
term_put_tuple_element(exit_tuple, 1, x_regs[1]);
4088+
term_put_tuple_element(exit_tuple, 1, ctx->exception_reason);
40854089
x_regs[0] = exit_tuple;
40864090

40874091
break;
@@ -6569,9 +6573,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
65696573
TRACE("build_stacktrace/0\n");
65706574

65716575
#ifdef IMPL_EXECUTE_LOOP
6572-
6573-
x_regs[0] = stacktrace_build(ctx, &x_regs[0], 1);
6574-
6576+
x_regs[0] = stacktrace_build(ctx);
65756577
#endif
65766578
break;
65776579
}
@@ -6590,6 +6592,9 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
65906592
ex_class != THROW_ATOM)) {
65916593
x_regs[0] = BADARG_ATOM;
65926594
} else {
6595+
ctx->exception_class = x_regs[0];
6596+
ctx->exception_reason = x_regs[1];
6597+
ctx->exception_stacktrace = x_regs[2];
65936598
goto handle_error;
65946599
}
65956600
#endif
@@ -7567,15 +7572,15 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
75677572

75687573
#ifdef AVM_PRINT_PROCESS_CRASH_DUMPS
75697574
// 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)) {
7575+
if (ctx->exception_class != LOWERCASE_EXIT_ATOM || (ctx->exception_reason != NORMAL_ATOM && ctx->exception_reason != SHUTDOWN_ATOM)) {
75717576
context_dump(ctx);
75727577
}
75737578
#endif
75747579

7575-
if (x_regs[0] == LOWERCASE_EXIT_ATOM) {
7576-
ctx->exit_reason = x_regs[1];
7580+
if (ctx->exception_class == LOWERCASE_EXIT_ATOM) {
7581+
ctx->exit_reason = ctx->exception_reason;
75777582
} else {
7578-
bool throw = ctx->x[0] == THROW_ATOM;
7583+
bool throw = ctx->exception_class == THROW_ATOM;
75797584

75807585
int exit_reason_tuple_size = (throw ? TUPLE_SIZE(2) : 0) + TUPLE_SIZE(2);
75817586
if (memory_ensure_free_with_roots(ctx, exit_reason_tuple_size, 1, x_regs + 1, MEMORY_CAN_SHRINK) != MEMORY_GC_OK) {
@@ -7585,10 +7590,10 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
75857590
if (throw) {
75867591
error_term = term_alloc_tuple(2, &ctx->heap);
75877592
term_put_tuple_element(error_term, 0, NOCATCH_ATOM);
7588-
term_put_tuple_element(error_term, 1, x_regs[1]);
7593+
term_put_tuple_element(error_term, 1, ctx->exception_reason);
75897594
} else {
75907595
// error
7591-
error_term = x_regs[1];
7596+
error_term = ctx->exception_reason;
75927597
}
75937598

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

‎src/libAtomVM/stacktrace.c‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,9 @@ term stacktrace_create_raw(Context *ctx, Module *mod, int current_offset, term e
3333
return exception_class;
3434
}
3535

36-
term stacktrace_build(Context *ctx, term *stack_info, uint32_t live)
36+
term stacktrace_build(Context *ctx)
3737
{
3838
UNUSED(ctx);
39-
UNUSED(stack_info);
40-
UNUSED(live);
4139
return UNDEFINED_ATOM;
4240
}
4341

@@ -168,8 +166,8 @@ term stacktrace_create_raw(Context *ctx, Module *mod, int current_offset, term e
168166

169167
// {num_frames, num_aux_terms, filename_lens, num_mods, [{module, offset}, ...]}
170168
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)) {
169+
// TODO: we need to know which registers contain function call arguments and save them
170+
if (UNLIKELY(memory_ensure_free_opt(ctx, requested_size, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
173171
fprintf(stderr, "WARNING: Unable to allocate heap space for raw stacktrace\n");
174172
return OUT_OF_MEMORY_ATOM;
175173
}
@@ -257,38 +255,41 @@ static term find_path_created(const void *key, struct ModulePathPair *module_pat
257255
return term_invalid_term();
258256
}
259257

260-
term stacktrace_build(Context *ctx, term *stack_info, uint32_t live)
258+
term stacktrace_build(Context *ctx)
261259
{
262260
GlobalContext *glb = ctx->global;
263261

264-
if (*stack_info == OUT_OF_MEMORY_ATOM) {
265-
return *stack_info;
262+
term stack_info = ctx->exception_stacktrace;
263+
264+
if (stack_info == OUT_OF_MEMORY_ATOM) {
265+
return stack_info;
266266
}
267-
if (!term_is_tuple(*stack_info)) {
267+
if (!term_is_tuple(stack_info)) {
268268
return UNDEFINED_ATOM;
269269
}
270270

271-
int num_frames = term_to_int(term_get_tuple_element(*stack_info, 0));
272-
int num_aux_terms = term_to_int(term_get_tuple_element(*stack_info, 1));
273-
int filename_lens = term_to_int(term_get_tuple_element(*stack_info, 2));
274-
int num_mods = term_to_int(term_get_tuple_element(*stack_info, 3));
271+
int num_frames = term_to_int(term_get_tuple_element(stack_info, 0));
272+
int num_aux_terms = term_to_int(term_get_tuple_element(stack_info, 1));
273+
int filename_lens = term_to_int(term_get_tuple_element(stack_info, 2));
274+
int num_mods = term_to_int(term_get_tuple_element(stack_info, 3));
275275

276276
struct ModulePathPair *module_paths = malloc(num_mods * sizeof(struct ModulePathPair));
277277
if (IS_NULL_PTR(module_paths)) {
278278
fprintf(stderr, "Unable to allocate space for module paths. Returning raw stacktrace.\n");
279-
return *stack_info;
279+
return stack_info;
280280
}
281281

282282
//
283283
// [{module, function, arity, [{file, string()}, {line, int}]}, ...]
284284
//
285285
size_t requested_size = (TUPLE_SIZE(4) + 2) * num_frames + num_aux_terms * (4 + 2 * TUPLE_SIZE(2)) + 2 * filename_lens;
286-
if (UNLIKELY(memory_ensure_free_with_roots(ctx, requested_size, live, ctx->x, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
286+
if (UNLIKELY(memory_ensure_free_opt(ctx, requested_size, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
287287
free(module_paths);
288288
return OUT_OF_MEMORY_ATOM;
289289
}
290+
stack_info = ctx->exception_stacktrace;
290291

291-
term raw_stacktrace = term_get_tuple_element(*stack_info, 4);
292+
term raw_stacktrace = term_get_tuple_element(stack_info, 4);
292293

293294
term stacktrace = term_nil();
294295
term el = raw_stacktrace;

‎src/libAtomVM/stacktrace.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ term stacktrace_create_raw(Context *ctx, Module *mod, int current_offset, term e
3737
* @param live number of x registers to preserve, which should include stack_info
3838
* @return the built stack trace
3939
*/
40-
term stacktrace_build(Context *ctx, term *stack_info, uint32_t live);
40+
term stacktrace_build(Context *ctx);
4141
term stacktrace_exception_class(term stack_info);
4242

4343
#ifdef __cplusplus

0 commit comments

Comments
 (0)