Skip to content

Commit 4a25435

Browse files
bettiobettio
committed
Merge pull request #1740 from pguyot/w28/fix-bs-start-match
Fix fail cases in OP_BS_START_MATCH3 & OP_BS_START_MATCH4 Continuation of #1731 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
2 parents 4f81b85 + e5ff7d8 commit 4a25435

File tree

5 files changed

+228
-50
lines changed

5 files changed

+228
-50
lines changed

‎src/libAtomVM/opcodesswitch.h‎

Lines changed: 68 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,22 @@ typedef dreg_t dreg_gc_safe_t;
416416
DECODE_VALUE32(label, decode_pc); \
417417
}
418418

419+
#define DECODE_ATOM_OR_LABEL(atom, label, decode_pc) \
420+
{ \
421+
if ((*(decode_pc) & 0x7) != COMPACT_ATOM) { \
422+
if (UNLIKELY((*(decode_pc) & 0x7) != COMPACT_LABEL)) { \
423+
fprintf(stderr, "Unexpected operand, expected an atom or label (%x)\n", *(decode_pc)); \
424+
AVM_ABORT(); \
425+
} \
426+
atom = term_invalid_term(); \
427+
DECODE_VALUE32(label, decode_pc); \
428+
} else { \
429+
uint32_t atom_ix; \
430+
DECODE_VALUE32(atom_ix, decode_pc); \
431+
atom = module_get_atom_term_by_id(mod, atom_ix); \
432+
} \
433+
}
434+
419435
#define DECODE_LITERAL(literal, decode_pc) \
420436
{ \
421437
if (UNLIKELY((*(decode_pc) & 0x7) != COMPACT_LITERAL)) { \
@@ -912,6 +928,18 @@ static void destroy_extended_registers(Context *ctx, unsigned int live)
912928
#define DECODE_LABEL(label, decode_pc) \
913929
DECODE_VALUE(label, decode_pc)
914930

931+
#define DECODE_ATOM_OR_LABEL(atom, label, decode_pc) \
932+
{ \
933+
if ((*(decode_pc) & 0x7) != COMPACT_ATOM) { \
934+
atom = term_invalid_term(); \
935+
DECODE_VALUE(label, decode_pc); \
936+
} else { \
937+
uint32_t atom_ix; \
938+
DECODE_VALUE(atom_ix, decode_pc); \
939+
atom = module_get_atom_term_by_id(mod, atom_ix); \
940+
} \
941+
}
942+
915943
#define DECODE_LITERAL(val, decode_pc) \
916944
DECODE_VALUE(val, decode_pc)
917945

@@ -1192,10 +1220,14 @@ static void destroy_extended_registers(Context *ctx, unsigned int live)
11921220
} \
11931221
}
11941222

1195-
#define VERIFY_IS_MATCH_STATE(t, opcode_name) \
1223+
#define VERIFY_IS_MATCH_STATE(t, opcode_name, label) \
11961224
if (UNLIKELY(!term_is_match_state(t))) { \
11971225
TRACE(opcode_name ": " #t " is not a match context.\n"); \
1198-
RAISE_ERROR(BADARG_ATOM); \
1226+
if (label == 0) { \
1227+
RAISE_ERROR(BADARG_ATOM); \
1228+
} else { \
1229+
JUMP_TO_LABEL(mod, label); \
1230+
} \
11991231
}
12001232

12011233
#define VERIFY_IS_MATCH_OR_BINARY(t, opcode_name) \
@@ -4059,7 +4091,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
40594091
#ifdef IMPL_EXECUTE_LOOP
40604092
TRACE("bs_get_utf8/5, fail=%i src=0x%lx arg2=0x%lx arg3=0x%lx dreg=%c%i\n", fail, src, arg2, arg3, T_DEST_REG(dreg));
40614093

4062-
VERIFY_IS_MATCH_STATE(src, "bs_get_utf8");
4094+
VERIFY_IS_MATCH_STATE(src, "bs_get_utf8", 0);
40634095

40644096
term src_bin = term_get_match_state_binary(src);
40654097
avm_int_t offset_bits = term_get_match_state_offset(src);
@@ -4096,7 +4128,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
40964128
#ifdef IMPL_EXECUTE_LOOP
40974129
TRACE("bs_skip_utf8/4, fail=%i src=0x%lx arg2=0x%lx arg3=0x%lx\n", fail, src, arg2, arg3);
40984130

4099-
VERIFY_IS_MATCH_STATE(src, "bs_get_utf8");
4131+
VERIFY_IS_MATCH_STATE(src, "bs_get_utf8", 0);
41004132

41014133
term src_bin = term_get_match_state_binary(src);
41024134
avm_int_t offset_bits = term_get_match_state_offset(src);
@@ -4196,7 +4228,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
41964228
#ifdef IMPL_EXECUTE_LOOP
41974229
TRACE("bs_get_utf16/5, fail=%i src=0x%lx arg2=0x%lx flags=0x%"PRIu32" dreg=%c%i\n", fail, src, arg2, flags_value, T_DEST_REG(dreg));
41984230

4199-
VERIFY_IS_MATCH_STATE(src, "bs_get_utf16");
4231+
VERIFY_IS_MATCH_STATE(src, "bs_get_utf16", 0);
42004232

42014233
term src_bin = term_get_match_state_binary(src);
42024234
avm_int_t offset_bits = term_get_match_state_offset(src);
@@ -4233,7 +4265,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
42334265
#ifdef IMPL_EXECUTE_LOOP
42344266
TRACE("bs_skip_utf16/5, fail=%i src=0x%lx arg2=0x%lx flags=0x%lx\n", fail, src, arg2, flags);
42354267

4236-
VERIFY_IS_MATCH_STATE(src, "bs_skip_utf16");
4268+
VERIFY_IS_MATCH_STATE(src, "bs_skip_utf16", 0);
42374269

42384270
term src_bin = term_get_match_state_binary(src);
42394271
avm_int_t offset_bits = term_get_match_state_offset(src);
@@ -4309,7 +4341,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
43094341
#ifdef IMPL_EXECUTE_LOOP
43104342
TRACE("bs_get_utf32/5, fail=%i src=0x%lx arg2=0x%lx flags=0x%"PRIu32" dreg=%c%i\n", fail, src, arg2, flags_value, T_DEST_REG(dreg));
43114343

4312-
VERIFY_IS_MATCH_STATE(src, "bs_get_utf32");
4344+
VERIFY_IS_MATCH_STATE(src, "bs_get_utf32", 0);
43134345

43144346
term src_bin = term_get_match_state_binary(src);
43154347
avm_int_t offset_bits = term_get_match_state_offset(src);
@@ -4345,7 +4377,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
43454377
#ifdef IMPL_EXECUTE_LOOP
43464378
TRACE("bs_skip_utf32/5, fail=%i src=0x%lx arg2=0x%lx flags=0x%lx\n", fail, src, arg2, flags);
43474379

4348-
VERIFY_IS_MATCH_STATE(src, "bs_skip_utf32");
4380+
VERIFY_IS_MATCH_STATE(src, "bs_skip_utf32", 0);
43494381

43504382
term src_bin = term_get_match_state_binary(src);
43514383
avm_int_t offset_bits = term_get_match_state_offset(src);
@@ -4689,17 +4721,14 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
46894721

46904722
#ifdef IMPL_EXECUTE_LOOP
46914723
// MEMORY_CAN_SHRINK because bs_start_match is classified as gc in beam_ssa_codegen.erl
4692-
#ifdef IMPL_EXECUTE_LOOP
4693-
TRIM_LIVE_REGS(live);
4694-
x_regs[live] = src;
4695-
if (memory_ensure_free_with_roots(ctx, TERM_BOXED_BIN_MATCH_STATE_SIZE, live + 1, x_regs, MEMORY_CAN_SHRINK) != MEMORY_GC_OK) {
4696-
RAISE_ERROR(OUT_OF_MEMORY_ATOM);
4697-
}
4698-
src = x_regs[live];
4699-
#endif
4724+
TRIM_LIVE_REGS(live);
4725+
x_regs[live] = src;
4726+
if (memory_ensure_free_with_roots(ctx, TERM_BOXED_BIN_MATCH_STATE_SIZE, live + 1, x_regs, MEMORY_CAN_SHRINK) != MEMORY_GC_OK) {
4727+
RAISE_ERROR(OUT_OF_MEMORY_ATOM);
4728+
}
4729+
src = x_regs[live];
47004730
TRACE("bs_start_match3/4, fail=%i src=0x%lx live=%u dreg=%c%i\n", fail, src, live, T_DEST_REG_GC_SAFE(dreg));
47014731
if (!(term_is_binary(src) || term_is_match_state(src))) {
4702-
WRITE_REGISTER_GC_SAFE(dreg, src);
47034732
pc = mod->labels[fail];
47044733
} else {
47054734
term match_state = term_alloc_bin_match_state(src, 0, &ctx->heap);
@@ -4725,7 +4754,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
47254754
#endif
47264755

47274756
#ifdef IMPL_EXECUTE_LOOP
4728-
VERIFY_IS_MATCH_STATE(src, "bs_get_position");
4757+
VERIFY_IS_MATCH_STATE(src, "bs_get_position", 0);
47294758

47304759
TRACE("bs_get_position/3 src=0x%lx dreg=%c%i live=%u\n", src, T_DEST_REG(dreg), live);
47314760

@@ -4750,7 +4779,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
47504779
#endif
47514780

47524781
#ifdef IMPL_EXECUTE_LOOP
4753-
VERIFY_IS_MATCH_STATE(src, "bs_get_tail");
4782+
VERIFY_IS_MATCH_STATE(src, "bs_get_tail", 0);
47544783

47554784
avm_int_t bs_offset = term_get_match_state_offset(src);
47564785
term bs_bin = term_get_match_state_binary(src);
@@ -4797,7 +4826,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
47974826
#endif
47984827

47994828
#ifdef IMPL_EXECUTE_LOOP
4800-
VERIFY_IS_MATCH_STATE(src, "bs_set_position");
4829+
VERIFY_IS_MATCH_STATE(src, "bs_set_position", 0);
48014830
VERIFY_IS_INTEGER(pos, "bs_set_position", 0);
48024831

48034832
avm_int_t pos_val = term_to_int(pos);
@@ -4823,7 +4852,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
48234852
#endif
48244853

48254854
#ifdef IMPL_EXECUTE_LOOP
4826-
VERIFY_IS_MATCH_STATE(src, "bs_match_string");
4855+
VERIFY_IS_MATCH_STATE(src, "bs_match_string", 0);
48274856

48284857
avm_int_t bs_offset = term_get_match_state_offset(src);
48294858
term bs_bin = term_get_match_state_binary(src);
@@ -4902,7 +4931,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
49024931
#endif
49034932

49044933
#ifdef IMPL_EXECUTE_LOOP
4905-
VERIFY_IS_MATCH_STATE(src, "bs_save2");
4934+
VERIFY_IS_MATCH_STATE(src, "bs_save2", 0);
49064935

49074936
avm_int_t index_val;
49084937
if (index == START_ATOM) {
@@ -4931,7 +4960,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
49314960
#endif
49324961

49334962
#ifdef IMPL_EXECUTE_LOOP
4934-
VERIFY_IS_MATCH_STATE(src, "bs_restore2");
4963+
VERIFY_IS_MATCH_STATE(src, "bs_restore2", 0);
49354964

49364965
avm_int_t index_val;
49374966
if (index == START_ATOM) {
@@ -4966,7 +4995,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
49664995
#endif
49674996

49684997
#ifdef IMPL_EXECUTE_LOOP
4969-
VERIFY_IS_MATCH_STATE(src, "bs_skip_bits2");
4998+
VERIFY_IS_MATCH_STATE(src, "bs_skip_bits2", 0);
49704999
VERIFY_IS_INTEGER(size, "bs_skip_bits2", 0);
49715000
if (flags_value != 0) {
49725001
TRACE("bs_skip_bits2: neither signed nor native or little endian encoding supported.\n");
@@ -5004,7 +5033,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
50045033
#endif
50055034

50065035
#ifdef IMPL_EXECUTE_LOOP
5007-
VERIFY_IS_MATCH_STATE(src, "bs_test_unit");
5036+
VERIFY_IS_MATCH_STATE(src, "bs_test_unit", 0);
50085037

50095038
TRACE("bs_test_unit/3, fail=%u src=%p unit=%u\n", (unsigned) fail, (void *) src, (unsigned) unit);
50105039

@@ -5030,7 +5059,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
50305059
#endif
50315060

50325061
#ifdef IMPL_EXECUTE_LOOP
5033-
VERIFY_IS_MATCH_STATE(src, "bs_test_tail2");
5062+
VERIFY_IS_MATCH_STATE(src, "bs_test_tail2", 0);
50345063

50355064
TRACE("bs_test_tail2/3, fail=%u src=%p bits=%u\n", (unsigned) fail, (void *) src, (unsigned) bits);
50365065

@@ -5066,7 +5095,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
50665095
#endif
50675096

50685097
#ifdef IMPL_EXECUTE_LOOP
5069-
VERIFY_IS_MATCH_STATE(src, "bs_get_integer");
5098+
VERIFY_IS_MATCH_STATE(src, "bs_get_integer", 0);
50705099
VERIFY_IS_INTEGER(size, "bs_get_integer", 0);
50715100

50725101
avm_int_t size_val = term_to_int(size);
@@ -5120,7 +5149,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
51205149
#endif
51215150

51225151
#ifdef IMPL_EXECUTE_LOOP
5123-
VERIFY_IS_MATCH_STATE(src, "bs_get_float");
5152+
VERIFY_IS_MATCH_STATE(src, "bs_get_float", 0);
51245153
VERIFY_IS_INTEGER(size, "bs_get_float", 0);
51255154

51265155
avm_int_t size_val = term_to_int(size);
@@ -5186,7 +5215,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
51865215
#endif
51875216

51885217
#ifdef IMPL_EXECUTE_LOOP
5189-
VERIFY_IS_MATCH_STATE(src, "bs_get_binary2");
5218+
VERIFY_IS_MATCH_STATE(src, "bs_get_binary2", 0);
51905219

51915220
term bs_bin = term_get_match_state_binary(src);
51925221
avm_int_t bs_offset = term_get_match_state_offset(src);
@@ -6425,18 +6454,9 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
64256454
}
64266455

64276456
case OP_BS_START_MATCH4: {
6428-
// fail since OTP 23 might be either 'no_fail', 'resume' or a fail label
6429-
// TODO: figure out what could fail
6430-
term fail;
6431-
DECODE_COMPACT_TERM(fail, pc);
6432-
#ifdef IMPL_EXECUTE_LOOP
6433-
if (!term_is_integer(fail) && !term_is_atom(fail)) {
6434-
fprintf(stderr, "Unexpected fail term ");
6435-
term_display(stderr, fail, ctx);
6436-
fprintf(stderr, "\n");
6437-
AVM_ABORT();
6438-
}
6439-
#endif
6457+
term fail_atom;
6458+
uint32_t fail_label = 0;
6459+
DECODE_ATOM_OR_LABEL(fail_atom, fail_label, pc);
64406460
uint32_t live;
64416461
DECODE_LITERAL(live, pc);
64426462
term src;
@@ -6458,16 +6478,14 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
64586478
#endif
64596479

64606480
#ifdef IMPL_EXECUTE_LOOP
6461-
TRACE("bs_start_match4/4, fail=%u live=%u src=%p dreg=%c%i\n", (unsigned) fail, (unsigned) live, (void *) src, T_DEST_REG(dreg));
6481+
TRACE("bs_start_match4/4, fail_atom=%u fail_label=%u live=%u src=%p dreg=%c%i\n", (unsigned) fail_atom, (unsigned) fail_label, (unsigned) live, (void *) src, T_DEST_REG(dreg));
64626482

6463-
if (!(term_is_binary(src) || term_is_match_state(src))) {
6464-
WRITE_REGISTER(dreg, src);
6465-
if (term_is_integer(fail)) {
6466-
pc = mod->labels[term_to_int(fail)];
6467-
} else {
6468-
RAISE_ERROR(BADARG_ATOM);
6469-
}
6483+
// no_fail: we know it's a binary or a match_state
6484+
// resume: we know it's a match_state
6485+
if (term_is_invalid_term(fail_atom) && !(term_is_binary(src) || term_is_match_state(src))) {
6486+
pc = mod->labels[fail_label];
64706487
} else {
6488+
assert(term_is_binary(src) || term_is_match_state(src));
64716489
term match_state = term_alloc_bin_match_state(src, 0, &ctx->heap);
64726490

64736491
WRITE_REGISTER(dreg, match_state);
@@ -6985,7 +7003,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
69857003
term match_state;
69867004
DECODE_COMPACT_TERM(match_state, pc);
69877005
#ifdef IMPL_EXECUTE_LOOP
6988-
VERIFY_IS_MATCH_STATE(match_state, "bs_match/3")
7006+
VERIFY_IS_MATCH_STATE(match_state, "bs_match/3", fail)
69897007
term bs_bin = term_get_match_state_binary(match_state);
69907008
size_t bs_offset = term_get_match_state_offset(match_state);
69917009
#endif

‎tests/erlang_tests/CMakeLists.txt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,9 +528,19 @@ compile_erlang(maps_nifs)
528528

529529
compile_erlang(test_raw_raise)
530530

531+
compile_erlang(test_op_bs_start_match)
532+
compile_assembler(test_op_bs_start_match_asm)
531533
compile_erlang(test_op_bs_create_bin)
532534
compile_assembler(test_op_bs_create_bin_asm)
533535

536+
if(Erlang_VERSION VERSION_GREATER_EQUAL "23")
537+
set(OTP23_OR_GREATER_TESTS
538+
test_op_bs_start_match_asm.beam
539+
)
540+
else()
541+
set(OTP23_OR_GREATER_TESTS)
542+
endif()
543+
534544
if(Erlang_VERSION VERSION_GREATER_EQUAL "25")
535545
set(OTP25_OR_GREATER_TESTS
536546
test_op_bs_create_bin_asm.beam
@@ -1023,7 +1033,9 @@ add_custom_target(erlang_test_modules DEPENDS
10231033

10241034
test_raw_raise.beam
10251035

1036+
test_op_bs_start_match.beam
10261037
test_op_bs_create_bin.beam
10271038

1039+
${OTP23_OR_GREATER_TESTS}
10281040
${OTP25_OR_GREATER_TESTS}
10291041
)

‎tests/erlang_tests/test_op_bs_start_match.erl‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
%
2+
% This file is part of AtomVM.
3+
%
4+
% Copyright 2025 Paul Guyot <pguyot@kallisys.net>
5+
%
6+
% Licensed under the Apache License, Version 2.0 (the "License");
7+
% you may not use this file except in compliance with the License.
8+
% You may obtain a copy of the License at
9+
%
10+
% http://www.apache.org/licenses/LICENSE-2.0
11+
%
12+
% Unless required by applicable law or agreed to in writing, software
13+
% distributed under the License is distributed on an "AS IS" BASIS,
14+
% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
% See the License for the specific language governing permissions and
16+
% limitations under the License.
17+
%
18+
% SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later
19+
%
20+
21+
-module(test_op_bs_start_match).
22+
23+
-export([start/0]).
24+
25+
start() ->
26+
HasBSStartMatch4 =
27+
case erlang:system_info(machine) of
28+
"BEAM" ->
29+
erlang:system_info(otp_release) >= "23";
30+
"ATOM" ->
31+
% If code was compiled with OTP < 23, we won't have bs_start_match asm file
32+
?OTP_RELEASE >= 23
33+
end,
34+
ok =
35+
if
36+
HasBSStartMatch4 ->
37+
ok = test_bs_start_match3_fail_label(),
38+
ok = test_bs_start_match4_fail_label(),
39+
ok = test_bs_start_match4_nofail_resume();
40+
true ->
41+
ok
42+
end,
43+
0.
44+
45+
test_bs_start_match3_fail_label() ->
46+
2 = test_op_bs_start_match_asm:bs_start_match3(<<"foo">>, <<"bar">>),
47+
2 = test_op_bs_start_match_asm:bs_start_match3(<<"foo">>, <<>>),
48+
3 = test_op_bs_start_match_asm:bs_start_match3(foo, bar),
49+
<<"foo">> = test_op_bs_start_match_asm:bs_start_match3(<<"foo">>, bar),
50+
5 = test_op_bs_start_match_asm:bs_start_match3(<<"foot">>, bar),
51+
ok.
52+
53+
test_bs_start_match4_fail_label() ->
54+
2 = test_op_bs_start_match_asm:bs_start_match4_label(<<"foo">>, <<"bar">>),
55+
2 = test_op_bs_start_match_asm:bs_start_match4_label(<<"foo">>, <<>>),
56+
3 = test_op_bs_start_match_asm:bs_start_match4_label(foo, bar),
57+
<<"foo">> = test_op_bs_start_match_asm:bs_start_match4_label(<<"foo">>, bar),
58+
5 = test_op_bs_start_match_asm:bs_start_match4_label(<<"foot">>, bar),
59+
ok.
60+
61+
% Other arguments will crash VM
62+
test_bs_start_match4_nofail_resume() ->
63+
<<"foo">> = test_op_bs_start_match_asm:bs_start_match4_nofail_resume(<<"foo">>),
64+
13 = test_op_bs_start_match_asm:bs_start_match4_nofail_resume(<<"foot">>),
65+
ok.

0 commit comments

Comments
 (0)