Skip to content

Commit

Permalink
Fix bug where guards would raise exceptions (fix atomvm#779)
Browse files Browse the repository at this point in the history
Signed-off-by: Paul Guyot <[email protected]>
  • Loading branch information
pguyot committed Oct 16, 2023
1 parent f43a03a commit 359289d
Show file tree
Hide file tree
Showing 5 changed files with 193 additions and 41 deletions.
131 changes: 97 additions & 34 deletions src/libAtomVM/opcodesswitch.h
Original file line number Diff line number Diff line change
Expand Up @@ -1920,7 +1920,6 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
break;
}

//TODO: implement me
case OP_BIF1: {
uint32_t fail_label;
DECODE_LABEL(fail_label, pc);
Expand All @@ -1945,15 +1944,19 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
DEBUG_FAIL_NULL(func);
term ret = func(ctx, arg1);
if (UNLIKELY(term_is_invalid_term(ret))) {
HANDLE_ERROR();
if (fail_label) {
pc = mod->labels[fail_label];
break;
} else {
HANDLE_ERROR();
}
}

WRITE_REGISTER(dreg, ret);
#endif
break;
}

//TODO: implement me
case OP_BIF2: {
uint32_t fail_label;
DECODE_LABEL(fail_label, pc);
Expand Down Expand Up @@ -1981,7 +1984,12 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
DEBUG_FAIL_NULL(func);
term ret = func(ctx, arg1, arg2);
if (UNLIKELY(term_is_invalid_term(ret))) {
HANDLE_ERROR();
if (fail_label) {
pc = mod->labels[fail_label];
break;
} else {
HANDLE_ERROR();
}
}

WRITE_REGISTER(dreg, ret);
Expand Down Expand Up @@ -5026,8 +5034,8 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
}

case OP_GC_BIF1: {
uint32_t f_label;
DECODE_LABEL(f_label, pc);
uint32_t fail_label;
DECODE_LABEL(fail_label, pc);
uint32_t live;
DECODE_LITERAL(live, pc);
uint32_t bif;
Expand All @@ -5040,35 +5048,38 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
GCBifImpl1 func = EXPORTED_FUNCTION_TO_GCBIF(exported_bif)->gcbif1_ptr;
term ret = func(ctx, live, arg1);
if (UNLIKELY(term_is_invalid_term(ret))) {
HANDLE_ERROR();
if (fail_label) {
pc = mod->labels[fail_label];
break;
} else {
HANDLE_ERROR();
}
}
#endif

dreg_t dreg;
DECODE_DEST_REGISTER(dreg, pc);

#ifdef IMPL_EXECUTE_LOOP
TRACE("gc_bif1/5 fail_lbl=%i, live=%i, bif=%i, arg1=0x%lx, dest=%c%i\n", f_label, live, bif, arg1, T_DEST_REG(dreg));
TRACE("gc_bif1/5 fail_lbl=%i, live=%i, bif=%i, arg1=0x%lx, dest=%c%i\n", fail_label, live, bif, arg1, T_DEST_REG(dreg));
WRITE_REGISTER(dreg, ret);
#endif

#ifdef IMPL_CODE_LOADER
TRACE("gc_bif1/5\n");

UNUSED(f_label)
UNUSED(fail_label)
UNUSED(live)
UNUSED(bif)
UNUSED(arg1)
UNUSED(dreg)
#endif

UNUSED(f_label)
break;
}

case OP_GC_BIF2: {
uint32_t f_label;
DECODE_LABEL(f_label, pc);
uint32_t fail_label;
DECODE_LABEL(fail_label, pc);
uint32_t live;
DECODE_LITERAL(live, pc);
uint32_t bif;
Expand All @@ -5083,30 +5094,33 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
GCBifImpl2 func = EXPORTED_FUNCTION_TO_GCBIF(exported_bif)->gcbif2_ptr;
term ret = func(ctx, live, arg1, arg2);
if (UNLIKELY(term_is_invalid_term(ret))) {
HANDLE_ERROR();
if (fail_label) {
pc = mod->labels[fail_label];
break;
} else {
HANDLE_ERROR();
}
}
#endif

dreg_t dreg;
DECODE_DEST_REGISTER(dreg, pc);

#ifdef IMPL_EXECUTE_LOOP
TRACE("gc_bif2/6 fail_lbl=%i, live=%i, bif=%i, arg1=0x%lx, arg2=0x%lx, dest=%c%i\n", f_label, live, bif, arg1, arg2, T_DEST_REG(dreg));
TRACE("gc_bif2/6 fail_lbl=%i, live=%i, bif=%i, arg1=0x%lx, arg2=0x%lx, dest=%c%i\n", fail_label, live, bif, arg1, arg2, T_DEST_REG(dreg));
WRITE_REGISTER(dreg, ret);
#endif

#ifdef IMPL_CODE_LOADER
TRACE("gc_bif2/6\n");

UNUSED(f_label)
UNUSED(fail_label)
UNUSED(live)
UNUSED(bif)
UNUSED(arg1)
UNUSED(arg2)
UNUSED(dreg)
#endif

UNUSED(f_label)
break;
}

Expand All @@ -5131,9 +5145,11 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
break;
}

// TODO: This opcode is currently uncovered by tests.
// We need to implement GC bifs with arity 3, e.g. binary_part/3.
case OP_GC_BIF3: {
uint32_t f_label;
DECODE_LABEL(f_label, pc);
uint32_t fail_label;
DECODE_LABEL(fail_label, pc);
uint32_t live;
DECODE_LITERAL(live, pc);
uint32_t bif;
Expand All @@ -5150,31 +5166,34 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
GCBifImpl3 func = EXPORTED_FUNCTION_TO_GCBIF(exported_bif)->gcbif3_ptr;
term ret = func(ctx, live, arg1, arg2, arg3);
if (UNLIKELY(term_is_invalid_term(ret))) {
HANDLE_ERROR();
if (fail_label) {
pc = mod->labels[fail_label];
break;
} else {
HANDLE_ERROR();
}
}
#endif

dreg_t dreg;
DECODE_DEST_REGISTER(dreg, pc);

#ifdef IMPL_EXECUTE_LOOP
TRACE("gc_bif3/7 fail_lbl=%i, live=%i, bif=%i, arg1=0x%lx, arg2=0x%lx, arg3=0x%lx, dest=%c%i\n", f_label, live, bif, arg1, arg2, arg3, T_DEST_REG(dreg));
TRACE("gc_bif3/7 fail_lbl=%i, live=%i, bif=%i, arg1=0x%lx, arg2=0x%lx, arg3=0x%lx, dest=%c%i\n", fail_label, live, bif, arg1, arg2, arg3, T_DEST_REG(dreg));
WRITE_REGISTER(dreg, ret);
#endif

#ifdef IMPL_CODE_LOADER
TRACE("gc_bif2/6\n");

UNUSED(f_label)
UNUSED(fail_label)
UNUSED(live)
UNUSED(bif)
UNUSED(arg1)
UNUSED(arg2)
UNUSED(arg3)
UNUSED(dreg)
#endif

UNUSED(f_label)
break;
}

Expand Down Expand Up @@ -5648,11 +5667,21 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
ctx->fr[freg3] = ctx->fr[freg1] + ctx->fr[freg2];
#ifdef HAVE_PRAGMA_STDC_FENV_ACCESS
if (fetestexcept(FE_OVERFLOW)) {
RAISE_ERROR(BADARITH_ATOM);
if (fail_label) {
// Not sure this can happen, float operations
// in guards are translated to gc_bif calls
pc = mod->labels[fail_label];
} else {
RAISE_ERROR(BADARITH_ATOM);
}
}
#else
if (!isfinite(ctx->fr[freg3])) {
RAISE_ERROR(BADARITH_ATOM);
if (fail_label) {
pc = mod->labels[fail_label];
} else {
RAISE_ERROR(BADARITH_ATOM);
}
}
#endif
#endif
Expand Down Expand Up @@ -5685,11 +5714,21 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
ctx->fr[freg3] = ctx->fr[freg1] - ctx->fr[freg2];
#ifdef HAVE_PRAGMA_STDC_FENV_ACCESS
if (fetestexcept(FE_OVERFLOW)) {
RAISE_ERROR(BADARITH_ATOM);
if (fail_label) {
// Not sure this can happen, float operations
// in guards are translated to gc_bif calls
pc = mod->labels[fail_label];
} else {
RAISE_ERROR(BADARITH_ATOM);
}
}
#else
if (!isfinite(ctx->fr[freg3])) {
RAISE_ERROR(BADARITH_ATOM);
if (fail_label) {
pc = mod->labels[fail_label];
} else {
RAISE_ERROR(BADARITH_ATOM);
}
}
#endif
#endif
Expand Down Expand Up @@ -5722,11 +5761,21 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
ctx->fr[freg3] = ctx->fr[freg1] * ctx->fr[freg2];
#ifdef HAVE_PRAGMA_STDC_FENV_ACCESS
if (fetestexcept(FE_OVERFLOW)) {
RAISE_ERROR(BADARITH_ATOM);
if (fail_label) {
// Not sure this can happen, float operations
// in guards are translated to gc_bif calls
pc = mod->labels[fail_label];
} else {
RAISE_ERROR(BADARITH_ATOM);
}
}
#else
if (!isfinite(ctx->fr[freg3])) {
RAISE_ERROR(BADARITH_ATOM);
if (fail_label) {
pc = mod->labels[fail_label];
} else {
RAISE_ERROR(BADARITH_ATOM);
}
}
#endif
#endif
Expand Down Expand Up @@ -5759,11 +5808,21 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
ctx->fr[freg3] = ctx->fr[freg1] / ctx->fr[freg2];
#ifdef HAVE_PRAGMA_STDC_FENV_ACCESS
if (fetestexcept(FE_OVERFLOW | FE_DIVBYZERO)) {
RAISE_ERROR(BADARITH_ATOM);
if (fail_label) {
// Not sure this can happen, float operations
// in guards are translated to gc_bif calls
pc = mod->labels[fail_label];
} else {
RAISE_ERROR(BADARITH_ATOM);
}
}
#else
if (!isfinite(ctx->fr[freg3])) {
RAISE_ERROR(BADARITH_ATOM);
if (fail_label) {
pc = mod->labels[fail_label];
} else {
RAISE_ERROR(BADARITH_ATOM);
}
}
#endif
#endif
Expand Down Expand Up @@ -5939,7 +5998,11 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)

if (!(term_is_binary(src) || term_is_match_state(src))) {
WRITE_REGISTER(dreg, src);
pc = mod->labels[fail];
if (term_is_integer(fail)) {
pc = mod->labels[term_to_int(fail)];
} else {
RAISE_ERROR(BADARG_ATOM);
}
} else {
term match_state = term_alloc_bin_match_state(src, 0, &ctx->heap);

Expand Down
6 changes: 4 additions & 2 deletions tests/erlang_tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ compile_erlang(guards2)
compile_erlang(guards3)
compile_erlang(guards4)
compile_erlang(guards5)
compile_erlang(test_guards_do_not_raise)
compile_erlang(test_min_max_guard)
compile_erlang(prime)
compile_erlang(match)
compile_erlang(if_test)
Expand Down Expand Up @@ -479,7 +481,6 @@ compile_erlang(test_add_avm_pack_binary)
compile_erlang(test_add_avm_pack_file)
compile_erlang(test_close_avm_pack)

compile_erlang(test_min_max_guard)
compile_erlang(test_module_info)

compile_erlang(int64_build_binary)
Expand Down Expand Up @@ -526,6 +527,8 @@ add_custom_target(erlang_test_modules DEPENDS
guards3.beam
guards4.beam
guards5.beam
test_guards_do_not_raise.beam
test_min_max_guard.beam
prime.beam
match.beam
if_test.beam
Expand Down Expand Up @@ -927,7 +930,6 @@ add_custom_target(erlang_test_modules DEPENDS
test_add_avm_pack_file.beam
test_close_avm_pack.beam

test_min_max_guard.beam
test_module_info.beam

int64_build_binary.beam
Expand Down
Loading

0 comments on commit 359289d

Please sign in to comment.