Skip to content

Commit

Permalink
Merge pull request #785 from pguyot/w34/fix-raising-guards
Browse files Browse the repository at this point in the history 
Fix bug where guards would raise exceptions (fix #779)

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
  • Loading branch information
@bettio
bettio committed Oct 16, 2023
2 parents b2d9812 + 43ab00a commit 8308b43
Show file tree
Hide file tree
Showing 6 changed files with 197 additions and 41 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [0.6.0-alpha.2] - Unreleased

### Fixed

- Fixed a bug where guards would raise exceptions instead of just being false

## [0.6.0-alpha.1] - 2023-10-09

### Added
Expand Down
131 changes: 97 additions & 34 deletions src/libAtomVM/opcodesswitch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1918,7 +1918,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 @@ -1943,15 +1942,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 @@ -1979,7 +1982,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 @@ -5048,8 +5056,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 @@ -5062,35 +5070,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 @@ -5105,30 +5116,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 @@ -5153,9 +5167,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 @@ -5172,31 +5188,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 @@ -5674,11 +5693,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 @@ -5711,11 +5740,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 @@ -5748,11 +5787,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 @@ -5785,11 +5834,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 @@ -5963,7 +6022,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
View file
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 8308b43

Please sign in to comment.