Merge pull request #2256 from pguyot/w14/fix-binary-at

Fix sign of results of several `binary` functions

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

Author: bettio

CHANGELOG.md

@@ -712,6 +712,7 @@ functions that default to `?ATOMVM_NVS_NS` are deprecated now).
 - Fixed numerous bugs in memory allocations that could crash the VM
 - Fixed SNTP support that had been broken in IDF 4.x builds
 - Fixed `erlang:send/2` not sending to registered name
+- Fixed sign of `binary:at/2`, `binary:first/1` and `binary:last/1` results
 
 ### Breaking Changes
 

src/libAtomVM/nifs.c

@@ -3410,7 +3410,7 @@ static term nif_binary_at_2(Context *ctx, int argc, term argv[])
         RAISE_ERROR(BADARG_ATOM);
     }
 
-    return term_from_int11(term_binary_data(bin_term)[pos]);
+    return term_from_int11((uint8_t) term_binary_data(bin_term)[pos]);
 }
 
 static term nif_binary_copy(Context *ctx, int argc, term argv[])
@@ -3456,7 +3456,7 @@ static term nif_binary_first_1(Context *ctx, int argc, term argv[])
         RAISE_ERROR(BADARG_ATOM);
     }
 
-    return term_from_int11(term_binary_data(bin_term)[0]);
+    return term_from_int11((uint8_t) term_binary_data(bin_term)[0]);
 }
 
 static term nif_binary_last_1(Context *ctx, int argc, term argv[])
@@ -3473,7 +3473,7 @@ static term nif_binary_last_1(Context *ctx, int argc, term argv[])
         RAISE_ERROR(BADARG_ATOM);
     }
 
-    return term_from_int11(term_binary_data(bin_term)[size - 1]);
+    return term_from_int11((uint8_t) term_binary_data(bin_term)[size - 1]);
 }
 
 static term nif_binary_part_3(Context *ctx, int argc, term argv[])

tests/erlang_tests/binary_at_test.erl

@@ -23,11 +23,16 @@
 -export([start/0, id/1, atp10/1]).
 
 start() ->
-    atp10(id(<<"HelloWorld">>)) + atp10safe(id(<<"">>)) + atp10safe(42).
+    atp10(id(<<"HelloWorld">>)) + atp10safe(id(<<"">>)) + atp10safe(42) +
+        high_byte_test(id(<<200>>)).
 
 atp10(Bin) ->
     binary:at(Bin, 4) + 10.
 
+high_byte_test(Bin) ->
+    200 = binary:at(Bin, 0),
+    0.
+
 atp10safe(Bin) ->
     try atp10(Bin) of
         _Any -> 1

tests/erlang_tests/binary_first_test.erl

@@ -23,11 +23,26 @@
 -export([start/0, id/1, firstp10/1]).
 
 start() ->
-    firstp10(id(<<"HelloWorld">>)) + firstp10safe(<<>>) + firstp10safe(42) + firstp10safe({<<>>}).
+    firstp10(id(<<"HelloWorld">>)) + firstp10safe(<<>>) + firstp10safe(42) + firstp10safe({<<>>}) +
+        high_byte_first_test(id(<<200, 1, 2>>)) +
+        high_byte_first_boundary_test(id(<<128, 1, 2>>)) +
+        high_byte_first_max_test(id(<<255, 1, 2>>)).
 
 firstp10(Bin) ->
     binary:first(Bin) + 10.
 
+high_byte_first_test(Bin) ->
+    200 = binary:first(Bin),
+    0.
+
+high_byte_first_boundary_test(Bin) ->
+    128 = binary:first(Bin),
+    0.
+
+high_byte_first_max_test(Bin) ->
+    255 = binary:first(Bin),
+    0.
+
 firstp10safe(Bin) ->
     try firstp10(Bin) of
         _Any -> 1

tests/erlang_tests/binary_last_test.erl

@@ -23,11 +23,26 @@
 -export([start/0, id/1, lastp10/1]).
 
 start() ->
-    lastp10(id(<<"HelloWorld">>)) + lastp10safe(<<>>) + lastp10safe(42) + lastp10safe({<<>>}).
+    lastp10(id(<<"HelloWorld">>)) + lastp10safe(<<>>) + lastp10safe(42) + lastp10safe({<<>>}) +
+        high_byte_last_test(id(<<1, 2, 200>>)) +
+        high_byte_last_boundary_test(id(<<1, 2, 128>>)) +
+        high_byte_last_max_test(id(<<1, 2, 255>>)).
 
 lastp10(Bin) ->
     binary:last(Bin) + 10.
 
+high_byte_last_test(Bin) ->
+    200 = binary:last(Bin),
+    0.
+
+high_byte_last_boundary_test(Bin) ->
+    128 = binary:last(Bin),
+    0.
+
+high_byte_last_max_test(Bin) ->
+    255 = binary:last(Bin),
+    0.
+
 lastp10safe(Bin) ->
     try lastp10(Bin) of
         _Any -> 1