Skip to content

Commit dffbd53

Browse files
committed
Fix unaligned matching of strings for code compiled with OTP < 26
OTP 25 compiler generates bs_match_string, and AtomVM wouldn't support some unaligned matching that worked when compiled by OTP 26+. Also bump s390x CI to use bookworm as cc from bullseye crashes on our code. Signed-off-by: Paul Guyot <pguyot@kallisys.net>
1 parent d4b1c56 commit dffbd53

File tree

4 files changed

+51
-18
lines changed

4 files changed

+51
-18
lines changed

.github/workflows/build-and-test-other.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ jobs:
107107
# Required for testing big endian archs
108108
- arch: "s390x"
109109
platform: "s390x"
110-
tag: "bullseye"
110+
tag: "bookworm"
111111
cflags: "-O2"
112112
cmake_opts: "-DAVM_WARNINGS_ARE_ERRORS=ON"
113113

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ certain VM instructions are used.
4949
- Fixed a double free when esp32 uart driver was closed, yielding an assert abort
5050
- Fixed compilation with latest debian gcc-arm-none-eabi
5151
- Fix `network:stop/0` on ESP32 so the network can be started again
52+
- Fix matching of binaries on unaligned boundaries for code compiled with older versions of OTP
5253

5354
## [0.6.5] - 2024-10-15
5455

src/libAtomVM/opcodesswitch.h

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4703,34 +4703,57 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
47034703
#ifdef IMPL_EXECUTE_LOOP
47044704
VERIFY_IS_MATCH_STATE(src, "bs_match_string");
47054705

4706-
if (bits % 8 != 0) {
4707-
TRACE("bs_match_string: Unsupported bits size (must be evenly divisible by 8). bits=%u\n", (unsigned) bits);
4708-
RAISE_ERROR(UNSUPPORTED_ATOM);
4709-
}
4710-
avm_int_t bytes = bits / 8;
47114706
avm_int_t bs_offset = term_get_match_state_offset(src);
47124707
term bs_bin = term_get_match_state_binary(src);
47134708

4714-
if (bs_offset % 8 != 0) {
4715-
TRACE("bs_match_string: Unsupported offset (must be evenly divisible by 8). bs_offset=%li\n", bs_offset);
4716-
RAISE_ERROR(UNSUPPORTED_ATOM);
4717-
}
4718-
avm_int_t byte_offset = bs_offset / 8;
4719-
4720-
TRACE("bs_match_string/4, fail=%u src=%p bits=%u offset=%u\n", (unsigned) fail, (void *) src, (unsigned) bits, (unsigned) offset);
4721-
47224709
size_t remaining = 0;
47234710
const uint8_t *str = module_get_str(mod, offset, &remaining);
47244711
if (IS_NULL_PTR(str)) {
47254712
TRACE("bs_match_string: Bad offset in strings table.\n");
47264713
RAISE_ERROR(BADARG_ATOM);
47274714
}
4728-
if (memcmp(term_binary_data(bs_bin) + byte_offset, str, MIN(remaining, (unsigned int) bytes)) != 0) {
4729-
TRACE("bs_match_string: failed to match\n");
4730-
JUMP_TO_ADDRESS(mod->labels[fail]);
4715+
4716+
TRACE("bs_match_string/4, fail=%u src=%p bits=%u offset=%u\n", (unsigned) fail, (void *) src, (unsigned) bits, (unsigned) offset);
4717+
4718+
if (bits % 8 == 0 && bs_offset % 8 == 0) {
4719+
avm_int_t bytes = bits / 8;
4720+
avm_int_t byte_offset = bs_offset / 8;
4721+
4722+
if (memcmp(term_binary_data(bs_bin) + byte_offset, str, MIN(remaining, (unsigned int) bytes)) != 0) {
4723+
TRACE("bs_match_string: failed to match\n");
4724+
JUMP_TO_ADDRESS(mod->labels[fail]);
4725+
}
47314726
} else {
4732-
term_set_match_state_offset(src, bs_offset + bits);
4727+
// Compare unaligned bits
4728+
const uint8_t *bs_str = (const uint8_t *) term_binary_data(bs_bin) + (bs_offset / 8);
4729+
uint8_t bin_bit_offset = 7 - (bs_offset - (8 *(bs_offset / 8)));
4730+
uint8_t str_bit_offset = 7;
4731+
size_t remaining_bits = bits;
4732+
while (remaining_bits > 0) {
4733+
uint8_t str_ch = *str;
4734+
uint8_t bin_ch = *bs_str;
4735+
uint8_t str_ch_bit = (str_ch >> str_bit_offset) & 1;
4736+
uint8_t bin_ch_bit = (bin_ch >> bin_bit_offset) & 1;
4737+
if (str_ch_bit ^ bin_ch_bit) {
4738+
TRACE("bs_match_string: failed to match\n");
4739+
JUMP_TO_ADDRESS(mod->labels[fail]);
4740+
}
4741+
if (str_bit_offset) {
4742+
str_bit_offset--;
4743+
} else {
4744+
str_bit_offset = 7;
4745+
str++;
4746+
}
4747+
if (bin_bit_offset) {
4748+
bin_bit_offset--;
4749+
} else {
4750+
bin_bit_offset = 7;
4751+
bs_str++;
4752+
}
4753+
remaining_bits--;
4754+
}
47334755
}
4756+
term_set_match_state_offset(src, bs_offset + bits);
47344757
#endif
47354758
break;
47364759
}

tests/erlang_tests/test_bs.erl

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ start() ->
8686

8787
test_put_match_string(<<"foo">>, <<"bar">>),
8888
test_skip_bits(),
89+
ok = test_bs_match_string_unaligned(),
8990

9091
test_match_case_type(),
9192

@@ -329,6 +330,14 @@ skip_bits(Len, Bin) ->
329330
<<_First:Len, Rest/binary>> = Bin,
330331
Rest.
331332

333+
test_bs_match_string_unaligned() ->
334+
<<0:1, _:3, 42:7, _:5, 42>> = id(<<0:3, 42, 0:5, 42>>),
335+
<<0:1, _:3, 42:12, _:8, 42>> = id(<<0, 42, 0, 42>>),
336+
ok = expect_error(
337+
fun() -> <<0:1, _:4, 42:12, 0:7>> = id(<<0:5, 42, 0:3>>) end, {badmatch, <<1, 80>>}
338+
),
339+
ok.
340+
332341
test_match_case_type() ->
333342
foo = match_case_type([foo, bar]),
334343
$a = match_case_type(<<"abc">>),

0 commit comments

Comments
 (0)