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 CI on arm64v8 to use bookworm as cc crashes on bullseye Signed-off-by: Paul Guyot <[email protected]>
pguyot · Jan 25, 2025 · 98cb365 · 98cb365
1 parent 0d6a00e
commit 98cb365
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 18 deletions.
diff --git a/.github/workflows/build-and-test-other.yaml b/.github/workflows/build-and-test-other.yaml
@@ -100,7 +100,7 @@ jobs:
 
         - arch: "arm64v8"
           platform: "arm64/v8"
-          tag: "bullseye"
+          tag: "bookworm"
           cflags: "-O2"
           cmake_opts: "-DAVM_WARNINGS_ARE_ERRORS=ON"
 

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -33,6 +33,7 @@ certain VM instructions are used.
 - Fixed compilation with latest debian gcc-arm-none-eabi
 - Fix `network:stop/0` on ESP32 so the network can be started again
 - Fix a memory corruption caused by `binary:split/2,3`
+- Fix matching of binaries on unaligned boundaries for code compiled with older versions of OTP
 
 ## [0.6.5] - 2024-10-15
 

diff --git a/src/libAtomVM/opcodesswitch.h b/src/libAtomVM/opcodesswitch.h
@@ -4703,34 +4703,57 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb)
                 #ifdef IMPL_EXECUTE_LOOP
                     VERIFY_IS_MATCH_STATE(src, "bs_match_string");
 
-                    if (bits % 8 != 0) {
-                        TRACE("bs_match_string: Unsupported bits size (must be evenly divisible by 8). bits=%u\n", (unsigned) bits);
-                        RAISE_ERROR(UNSUPPORTED_ATOM);
-                    }
-                    avm_int_t bytes = bits / 8;
                     avm_int_t bs_offset = term_get_match_state_offset(src);
                     term bs_bin = term_get_match_state_binary(src);
 
-                    if (bs_offset % 8 != 0) {
-                        TRACE("bs_match_string: Unsupported offset (must be evenly divisible by 8). bs_offset=%li\n", bs_offset);
-                        RAISE_ERROR(UNSUPPORTED_ATOM);
-                    }
-                    avm_int_t byte_offset = bs_offset / 8;
-
-                    TRACE("bs_match_string/4, fail=%u src=%p bits=%u offset=%u\n", (unsigned) fail, (void *) src, (unsigned) bits, (unsigned) offset);
-
                     size_t remaining = 0;
                     const uint8_t *str = module_get_str(mod, offset, &remaining);
                     if (IS_NULL_PTR(str)) {
                         TRACE("bs_match_string: Bad offset in strings table.\n");
                         RAISE_ERROR(BADARG_ATOM);
                     }
-                    if (memcmp(term_binary_data(bs_bin) + byte_offset, str, MIN(remaining, (unsigned int) bytes)) != 0) {
-                        TRACE("bs_match_string: failed to match\n");
-                        JUMP_TO_ADDRESS(mod->labels[fail]);
+
+                    TRACE("bs_match_string/4, fail=%u src=%p bits=%u offset=%u\n", (unsigned) fail, (void *) src, (unsigned) bits, (unsigned) offset);
+
+                    if (bits % 8 == 0 && bs_offset % 8 == 0) {
+                        avm_int_t bytes = bits / 8;
+                        avm_int_t byte_offset = bs_offset / 8;
+
+                        if (memcmp(term_binary_data(bs_bin) + byte_offset, str, MIN(remaining, (unsigned int) bytes)) != 0) {
+                            TRACE("bs_match_string: failed to match\n");
+                            JUMP_TO_ADDRESS(mod->labels[fail]);
+                        }
                     } else {
-                        term_set_match_state_offset(src, bs_offset + bits);
+                        // Compare unaligned bits
+                        const uint8_t *bs_str = (const uint8_t *) term_binary_data(bs_bin) + (bs_offset / 8);
+                        uint8_t bin_bit_offset = 7 - (bs_offset - (8 *(bs_offset / 8)));
+                        uint8_t str_bit_offset = 7;
+                        size_t remaining_bits = bits;
+                        while (remaining_bits > 0) {
+                            uint8_t str_ch = *str;
+                            uint8_t bin_ch = *bs_str;
+                            uint8_t str_ch_bit = (str_ch >> str_bit_offset) & 1;
+                            uint8_t bin_ch_bit = (bin_ch >> bin_bit_offset) & 1;
+                            if (str_ch_bit ^ bin_ch_bit) {
+                                TRACE("bs_match_string: failed to match\n");
+                                JUMP_TO_ADDRESS(mod->labels[fail]);
+                            }
+                            if (str_bit_offset) {
+                                str_bit_offset--;
+                            } else {
+                                str_bit_offset = 7;
+                                str++;
+                            }
+                            if (bin_bit_offset) {
+                                bin_bit_offset--;
+                            } else {
+                                bin_bit_offset = 7;
+                                bs_str++;
+                            }
+                            remaining_bits--;
+                        }
                     }
+                    term_set_match_state_offset(src, bs_offset + bits);
                 #endif
                 break;
             }

diff --git a/tests/erlang_tests/test_bs.erl b/tests/erlang_tests/test_bs.erl
@@ -86,6 +86,7 @@ start() ->
 
     test_put_match_string(<<"foo">>, <<"bar">>),
     test_skip_bits(),
+    ok = test_bs_match_string_unaligned(),
 
     test_match_case_type(),
 
@@ -329,6 +330,14 @@ skip_bits(Len, Bin) ->
     <<_First:Len, Rest/binary>> = Bin,
     Rest.
 
+test_bs_match_string_unaligned() ->
+    <<0:1, _:3, 42:7, _:5, 42>> = id(<<0:3, 42, 0:5, 42>>),
+    <<0:1, _:3, 42:12, _:8, 42>> = id(<<0, 42, 0, 42>>),
+    ok = expect_error(
+        fun() -> <<0:1, _:4, 42:12, 0:7>> = id(<<0:5, 42, 0:3>>) end, {badmatch, <<1, 80>>}
+    ),
+    ok.
+
 test_match_case_type() ->
     foo = match_case_type([foo, bar]),
     $a = match_case_type(<<"abc">>),