Skip to content

Commit 7d18a53

Browse files
bettiobettio
committed
Merge pull request atomvm#2116 from pguyot/w08/improve-jit-opcode-coverage
Fix bug in `OP_BS_CREATE_BIN` (and improve JIT coverage) 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 8f0f7c6 + 45c26be commit 7d18a53

File tree

12 files changed

+480
-71
lines changed

12 files changed

+480
-71
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ instead `badarg`.
105105
- Supervisor now honors period and intensity options.
106106
- Fix supervisor crash if a `one_for_one` child fails to restart.
107107
- Fix collision in references created with `make_ref/0` on 32 bits platforms.
108+
- Fixed a bug in `OP_BS_CREATE_BIN`
108109

109110
## [0.6.7] - Unreleased
110111

‎libs/jit/src/jit.erl‎

Lines changed: 43 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -2568,48 +2568,34 @@ first_pass_bs_create_bin_compute_size(
25682568
float, Src, Size, _SegmentUnit, Fail, AccLiteralSize0, AccSizeReg0, MMod, MSt0, State0
25692569
) ->
25702570
MSt1 = verify_is_number(Src, Fail, MMod, MSt0),
2571-
% Verify and get the float size (defaults to 64 if nil)
2572-
case Size of
2573-
?TERM_NIL ->
2574-
{MSt1, AccLiteralSize0 + 64, AccSizeReg0, State0};
2575-
_ ->
2576-
{MSt2, SizeValue} = term_to_int(Size, Fail, MMod, MSt1),
2577-
if
2578-
is_integer(SizeValue) ->
2579-
% If size is a literal, compiler would only allow 16/32/64.
2580-
{MSt2, AccLiteralSize0 + SizeValue, AccSizeReg0, State0};
2581-
is_atom(SizeValue) ->
2582-
% Check if size is 16, 32, or 64 using 'and' of '!=' checks
2583-
MSt3 = cond_raise_badarg_or_jump_to_fail_label(
2584-
{'and', [
2585-
{SizeValue, '!=', 16},
2586-
{SizeValue, '!=', 32},
2587-
{SizeValue, '!=', 64}
2588-
]},
2589-
Fail,
2590-
MMod,
2591-
MSt2
2592-
),
2593-
case AccSizeReg0 of
2594-
undefined ->
2595-
{MSt3, AccLiteralSize0, SizeValue, State0};
2596-
_ ->
2597-
MSt4 = MMod:add(MSt3, AccSizeReg0, SizeValue),
2598-
MSt5 = MMod:free_native_registers(MSt4, [SizeValue]),
2599-
{MSt5, AccLiteralSize0, AccSizeReg0, State0}
2600-
end
2571+
{MSt2, SizeValue} = term_to_int(Size, Fail, MMod, MSt1),
2572+
if
2573+
is_integer(SizeValue) ->
2574+
{MSt2, AccLiteralSize0 + SizeValue, AccSizeReg0, State0};
2575+
is_atom(SizeValue) ->
2576+
MSt3 = cond_raise_badarg_or_jump_to_fail_label(
2577+
{'and', [
2578+
{SizeValue, '!=', 16},
2579+
{SizeValue, '!=', 32},
2580+
{SizeValue, '!=', 64}
2581+
]},
2582+
Fail,
2583+
MMod,
2584+
MSt2
2585+
),
2586+
case AccSizeReg0 of
2587+
undefined ->
2588+
{MSt3, AccLiteralSize0, SizeValue, State0};
2589+
_ ->
2590+
MSt4 = MMod:add(MSt3, AccSizeReg0, SizeValue),
2591+
MSt5 = MMod:free_native_registers(MSt4, [SizeValue]),
2592+
{MSt5, AccLiteralSize0, AccSizeReg0, State0}
26012593
end
26022594
end;
26032595
first_pass_bs_create_bin_compute_size(
26042596
integer, Src, Size, SegmentUnit, Fail, AccLiteralSize0, AccSizeReg0, MMod, MSt0, State0
26052597
) ->
26062598
MSt1 = verify_is_any_integer(Src, Fail, MMod, MSt0),
2607-
first_pass_bs_create_bin_compute_size(
2608-
string, Src, Size, SegmentUnit, Fail, AccLiteralSize0, AccSizeReg0, MMod, MSt1, State0
2609-
);
2610-
first_pass_bs_create_bin_compute_size(
2611-
string, _Src, Size, SegmentUnit, Fail, AccLiteralSize0, AccSizeReg0, MMod, MSt1, State0
2612-
) ->
26132599
MSt2 = verify_is_integer(Size, Fail, MMod, MSt1),
26142600
{MSt3, SizeValue} = term_to_int(Size, 0, MMod, MSt2),
26152601
MSt5 =
@@ -2641,6 +2627,23 @@ first_pass_bs_create_bin_compute_size(
26412627
{MSt8, AccLiteralSize0, AccSizeReg0, State0}
26422628
end
26432629
end;
2630+
first_pass_bs_create_bin_compute_size(
2631+
string, _Src, Size, SegmentUnit, Fail, AccLiteralSize0, AccSizeReg0, MMod, MSt1, State0
2632+
) ->
2633+
MSt2 = verify_is_integer(Size, Fail, MMod, MSt1),
2634+
{MSt3, SizeValue} = term_to_int(Size, 0, MMod, MSt2),
2635+
MSt5 =
2636+
if
2637+
is_integer(SizeValue) andalso SizeValue > 0 ->
2638+
MSt3;
2639+
is_integer(SizeValue) andalso Fail =:= 0 ->
2640+
MMod:call_primitive_last(MSt3, ?PRIM_RAISE_ERROR, [
2641+
ctx, jit_state, offset, ?BADARG_ATOM
2642+
]);
2643+
is_integer(SizeValue) andalso Fail =/= 0 ->
2644+
MMod:jump_to_label(MSt3, Fail)
2645+
end,
2646+
{MSt5, AccLiteralSize0 + (SizeValue * SegmentUnit), AccSizeReg0, State0};
26442647
first_pass_bs_create_bin_compute_size(
26452648
AtomType, Src, ?ALL_ATOM, _SegmentUnit, Fail, AccLiteralSize0, AccSizeReg0, MMod, MSt0, State0
26462649
) when AtomType =:= binary orelse AtomType =:= append orelse AtomType =:= private_append ->
@@ -2758,17 +2761,9 @@ first_pass_bs_create_bin_insert_value(
27582761
first_pass_bs_create_bin_insert_value(
27592762
float, Flags, Src, Size, _SegmentUnit, Fail, CreatedBin, Offset, MMod, MSt0
27602763
) ->
2761-
% Src is a term (boxed float or integer)
27622764
{MSt1, SrcReg} = MMod:move_to_native_register(MSt0, Src),
27632765
{MSt2, FlagsValue} = decode_flags_list(Flags, MMod, MSt1),
2764-
% Get the float size (defaults to 64 if nil)
2765-
{MSt3, SizeValue} =
2766-
case Size of
2767-
?TERM_NIL ->
2768-
{MSt2, 64};
2769-
_ ->
2770-
term_to_int(Size, Fail, MMod, MSt2)
2771-
end,
2766+
{MSt3, SizeValue} = term_to_int(Size, Fail, MMod, MSt2),
27722767
% Call single primitive with size parameter
27732768
{MSt4, BoolResult} = MMod:call_primitive(MSt3, ?PRIM_BITSTRING_INSERT_FLOAT, [
27742769
CreatedBin, Offset, {free, SrcReg}, SizeValue, {free, FlagsValue}
@@ -2785,14 +2780,9 @@ first_pass_bs_create_bin_insert_value(
27852780
) ->
27862781
{MSt1, SrcValue} = term_to_int(Src, Fail, MMod, MSt0),
27872782
{MSt2, SizeValue} = term_to_int(Size, Fail, MMod, MSt1),
2788-
{MSt3, BitSize} =
2789-
if
2790-
is_integer(SizeValue) andalso is_integer(SegmentUnit) ->
2791-
{MSt2, SizeValue * SegmentUnit};
2792-
true ->
2793-
{MMod:mul(MSt2, SizeValue, SegmentUnit), SizeValue}
2794-
end,
2795-
{MSt4, VoidResult} = MMod:call_primitive(MSt3, ?PRIM_BITSTRING_COPY_MODULE_STR, [
2783+
true = is_integer(SizeValue) andalso is_integer(SegmentUnit),
2784+
BitSize = SizeValue * SegmentUnit,
2785+
{MSt4, VoidResult} = MMod:call_primitive(MSt2, ?PRIM_BITSTRING_COPY_MODULE_STR, [
27962786
ctx, jit_state, CreatedBin, Offset, {free, SrcValue}, BitSize
27972787
]),
27982788
MSt5 = MMod:free_native_registers(MSt4, [VoidResult]),

‎src/libAtomVM/opcodesswitch.h‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7005,17 +7005,13 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
70057005
JUMP_TO_LABEL(mod, fail);
70067006
}
70077007
}
7008-
// size is optional for floats, defaults to 64
7009-
avm_int_t signed_size_value = 64;
7010-
if (size != term_nil()) {
7011-
VERIFY_IS_INTEGER(size, "bs_create_bin/6", fail);
7012-
signed_size_value = term_to_int(size);
7013-
if (UNLIKELY(signed_size_value != 16 && signed_size_value != 32 && signed_size_value != 64)) {
7014-
if (fail == 0) {
7015-
RAISE_ERROR(BADARG_ATOM);
7016-
} else {
7017-
JUMP_TO_LABEL(mod, fail);
7018-
}
7008+
VERIFY_IS_INTEGER(size, "bs_create_bin/6", fail);
7009+
avm_int_t signed_size_value = term_to_int(size);
7010+
if (UNLIKELY(signed_size_value != 16 && signed_size_value != 32 && signed_size_value != 64)) {
7011+
if (fail == 0) {
7012+
RAISE_ERROR(BADARG_ATOM);
7013+
} else {
7014+
JUMP_TO_LABEL(mod, fail);
70197015
}
70207016
}
70217017
segment_size = signed_size_value;
@@ -7150,11 +7146,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
71507146
size_value = (size_t) term_to_int(size);
71517147
break;
71527148
case FLOAT_ATOM:
7153-
if (size != term_nil()) {
7154-
size_value = (size_t) term_to_int(size);
7155-
} else {
7156-
size_value = 64;
7157-
}
7149+
size_value = (size_t) term_to_int(size);
71587150
break;
71597151
default:
71607152
break;
@@ -7195,7 +7187,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
71957187
TRACE("bs_create_bin/6: Failed to insert integer into binary\n");
71967188
RAISE_ERROR(BADARG_ATOM);
71977189
}
7198-
segment_size = size_value;
7190+
segment_size = size_value * segment_unit;
71997191
break;
72007192
}
72017193
case FLOAT_ATOM: {

‎tests/erlang_tests/CMakeLists.txt‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,13 @@ compile_erlang(test_list_to_tuple)
541541

542542
compile_erlang(bs_context_byte_size)
543543
compile_erlang(bs_get_binary_fixed_size)
544+
compile_erlang(bs_get_integer_fixed_size)
545+
compile_erlang(bs_get_float_dynamic_size)
546+
compile_erlang(test_is_not_equal)
547+
compile_assembler(test_is_not_equal_asm)
548+
compile_erlang(test_has_map_fields)
549+
compile_erlang(test_bs_create_bin_accum)
550+
compile_assembler(test_bs_create_bin_accum_asm)
544551
compile_erlang(bs_context_to_binary_with_offset)
545552
compile_erlang(bs_restore2_start_offset)
546553
compile_erlang(test_refc_binaries)
@@ -614,7 +621,6 @@ compile_assembler(test_op_bs_start_match_asm)
614621
compile_erlang(test_op_bs_create_bin)
615622
compile_assembler(test_op_bs_create_bin_asm)
616623

617-
compile_erlang(bs_get_binary_fixed_size)
618624
compile_assembler(bs_get_binary2_all_asm)
619625

620626
compile_erlang(bigint)
@@ -637,6 +643,7 @@ endif()
637643
if(Erlang_VERSION VERSION_GREATER_EQUAL "25")
638644
set(OTP25_OR_GREATER_TESTS
639645
test_op_bs_create_bin_asm.beam
646+
test_bs_create_bin_accum_asm.beam
640647
)
641648
else()
642649
set(OTP25_OR_GREATER_TESTS)
@@ -1080,6 +1087,12 @@ set(erlang_test_beams
10801087

10811088
bs_context_byte_size.beam
10821089
bs_get_binary_fixed_size.beam
1090+
bs_get_integer_fixed_size.beam
1091+
bs_get_float_dynamic_size.beam
1092+
test_is_not_equal.beam
1093+
test_is_not_equal_asm.beam
1094+
test_has_map_fields.beam
1095+
test_bs_create_bin_accum.beam
10831096
bs_context_to_binary_with_offset.beam
10841097
bs_restore2_start_offset.beam
10851098
bs_append_extra_words.beam

‎tests/erlang_tests/bs_get_float_dynamic_size.erl‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
%
2+
% This file is part of AtomVM.
3+
%
4+
% Copyright 2026 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(bs_get_float_dynamic_size).
22+
23+
%% Force the compiler to use bs_get_float2 opcode instead of the
24+
%% newer bs_match opcode (OTP 25+). On older OTP this is ignored.
25+
%% The no_bs_match option was removed in OTP 29.
26+
-ifdef(OTP_RELEASE).
27+
-if(?OTP_RELEASE =< 28).
28+
-compile([no_bs_match]).
29+
-endif.
30+
-endif.
31+
32+
-export([start/0]).
33+
34+
start() ->
35+
%% Test bs_get_float2 with runtime-variable size.
36+
Bin64 = id(<<3.14:64/float>>),
37+
ok = test_float64_dynamic(Bin64, id(64)),
38+
Bin32 = id(<<3.14:32/float>>),
39+
ok = test_float32_dynamic(Bin32, id(32)),
40+
0.
41+
42+
test_float64_dynamic(Bin, Size) ->
43+
<<F:Size/float>> = Bin,
44+
true = (F > 3.13),
45+
true = (F < 3.15),
46+
ok.
47+
48+
test_float32_dynamic(Bin, Size) ->
49+
<<F:Size/float>> = Bin,
50+
true = (F > 3.13),
51+
true = (F < 3.15),
52+
ok.
53+
54+
id(X) -> X.

‎tests/erlang_tests/bs_get_integer_fixed_size.erl‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
%
2+
% This file is part of AtomVM.
3+
%
4+
% Copyright 2026 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(bs_get_integer_fixed_size).
22+
23+
%% Force the compiler to use bs_get_integer2 opcode instead of the
24+
%% newer bs_match opcode (OTP 25+). On older OTP this is ignored.
25+
%% The no_bs_match option was removed in OTP 29.
26+
-ifdef(OTP_RELEASE).
27+
-if(?OTP_RELEASE =< 28).
28+
-compile([no_bs_match]).
29+
-endif.
30+
-endif.
31+
32+
-export([start/0]).
33+
34+
start() ->
35+
Bin = id(<<1, 2, 3, 4, 5, 6, 7, 8>>),
36+
ok = test_match_8bit(Bin),
37+
ok = test_match_16bit(Bin),
38+
ok = test_match_32bit(Bin),
39+
ok = test_match_fail(Bin),
40+
0.
41+
42+
test_match_8bit(Bin) ->
43+
<<X:8, Rest/binary>> = Bin,
44+
1 = X,
45+
7 = byte_size(Rest),
46+
<<2, 3, 4, 5, 6, 7, 8>> = Rest,
47+
ok.
48+
49+
test_match_16bit(Bin) ->
50+
<<X:16, Rest/binary>> = Bin,
51+
258 = X,
52+
6 = byte_size(Rest),
53+
<<3, 4, 5, 6, 7, 8>> = Rest,
54+
ok.
55+
56+
test_match_32bit(Bin) ->
57+
<<X:32, Rest/binary>> = Bin,
58+
16909060 = X,
59+
4 = byte_size(Rest),
60+
<<5, 6, 7, 8>> = Rest,
61+
ok.
62+
63+
test_match_fail(Bin) ->
64+
case Bin of
65+
<<_X:72, _Rest/binary>> ->
66+
error;
67+
_ ->
68+
ok
69+
end.
70+
71+
id(X) -> X.

0 commit comments

Comments
 (0)