Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong binary data generated on Mac OS with OTP 26 #7566

Closed
ansd opened this issue Aug 16, 2023 · 1 comment · Fixed by #7567
Closed

Wrong binary data generated on Mac OS with OTP 26 #7566

ansd opened this issue Aug 16, 2023 · 1 comment · Fixed by #7567
Assignees
Labels
bug Issue is reported as a bug team:VM Assigned to OTP team VM

Comments

@ansd
Copy link
Contributor

ansd commented Aug 16, 2023

Describe the bug
Wrong binary data is generated.

To Reproduce
Compile and run the following Erlang program a.erl:

-module(a).
-export([b/1]).

b(Bin) ->
    c(byte_size(Bin)).

c(N) ->
    <<(N rem 128), (N div 128)>>.
$ erl
Erlang/OTP 26 [erts-14.0.2] [source] [64-bit] [smp:12:12] [ds:12:12:10] [async-threads:1] [jit]

Eshell V14.0.2 (press Ctrl+G to abort, type help(). for help)
1> c(a).
{ok,a}
2> a:b(binary:copy(<<"a">>, 128)).
<<1,1>>

Expected behavior

2> a:b(binary:copy(<<"a">>, 128)).
<<0,1>>

Affected versions
OTP 26.0.2 on Mac M2.

Additional context
Seems unrelated to #7469 as #7471 does not fix this issue. Tested via

kerl build git https://github.com/jhogberg/otp.git john/erts/fix-bitstring-match-bug/GH-7469/OTP-18672 otp-7469-fix
@ansd ansd added the bug Issue is reported as a bug label Aug 16, 2023
@michaelklishin
Copy link
Contributor

I can reproduce this on an M1 Mac as well.

ansd added a commit to rabbitmq/rabbitmq-server that referenced this issue Aug 16, 2023
On Mac OS M1 and M2 with OTP 26, prior to this commit,
the following test failed:
```
make -C deps/rabbitmq_mqtt ct-reader t=tests:rabbit_mqtt_qos0_queue_overflow FULL=1
```
with:
```
reader_SUITE > tests > rabbit_mqtt_qos0_queue_overflow
    #1. {'EXIT',
            {function_clause,
                {emqtt_frame,parse_packet,
                    [{mqtt_packet_header,7,true,0,false},
                     <<120,120,120,120,120,120,120,120,120,120,120,120,120,120,
                       120,120,120,120,120,120,120,120,120,120,120,120,120,120,
                       120,120,120,120,48,159,31,0,31,114,97,98,98,105,116,95,
                       109,113,116,116,95,113,111,115,48,95,113,117,101,117,
                       101,95,111,118,101,114,102,108,111,119,120,120,120,120,
                       120,120,120,120,120,120,120,120,120,120,120,120,120,120,
                       120,120,120,120,120,120,120,120,120,120,120,120,120,120,
                       120,120,120,120,120,120,120,120,120,120,120,120,120,120,
                       120,120,120,120,120,120>>,
                     #{version => 4,max_size => 268435455,
                       strict_mode => false}],
                    [{file,"src/emqtt_frame.erl"},{line,182}]}}}
```

This is due to the OTP bug in erlang/otp#7566
Simply exporting the serialise_len/1 fixes this OTP bug.
ansd added a commit to rabbitmq/rabbitmq-server that referenced this issue Aug 16, 2023
On Mac OS M1 and M2 with OTP 26, prior to this commit,
the following test failed:
```
make -C deps/rabbitmq_mqtt ct-reader t=tests:rabbit_mqtt_qos0_queue_overflow FULL=1
```
with:
```
reader_SUITE > tests > rabbit_mqtt_qos0_queue_overflow
    #1. {'EXIT',
            {function_clause,
                {emqtt_frame,parse_packet,
                    [{mqtt_packet_header,7,true,0,false},
                     <<120,120,120,120,120,120,120,120,120,120,120,120,120,120,
                       120,120,120,120,120,120,120,120,120,120,120,120,120,120,
                       120,120,120,120,48,159,31,0,31,114,97,98,98,105,116,95,
                       109,113,116,116,95,113,111,115,48,95,113,117,101,117,
                       101,95,111,118,101,114,102,108,111,119,120,120,120,120,
                       120,120,120,120,120,120,120,120,120,120,120,120,120,120,
                       120,120,120,120,120,120,120,120,120,120,120,120,120,120,
                       120,120,120,120,120,120,120,120,120,120,120,120,120,120,
                       120,120,120,120,120,120>>,
                     #{version => 4,max_size => 268435455,
                       strict_mode => false}],
                    [{file,"src/emqtt_frame.erl"},{line,182}]}}}
```

This is due to the OTP bug in erlang/otp#7566
Simply exporting the serialise_len/1 fixes this OTP bug.
@bjorng bjorng self-assigned this Aug 16, 2023
@bjorng bjorng added the team:VM Assigned to OTP team VM label Aug 16, 2023
bjorng added a commit to bjorng/otp that referenced this issue Aug 17, 2023
The JIT would generate code that calculated the remainder incorrect
for the following example:

    bug(Bin) ->
        N = byte_size(Bin),
        {N rem 128, N div 128}.

Essentially, the JIT would rewrite the code like this:

    bug(Bin) ->
        N = byte_size(Bin),
        Q = N bsr 7,
        {Q band 16#7f, Q}.

That is, the remainder would be calculated as `(N div 128) rem 128`.

Fixes erlang#7566
bjorng added a commit to bjorng/otp that referenced this issue Aug 17, 2023
The JIT would generate code that calculated the remainder incorrectly
for the following example:

    bug(Bin) ->
        N = byte_size(Bin),
        {N rem 128, N div 128}.

Essentially, the JIT would rewrite the code like this:

    bug(Bin) ->
        N = byte_size(Bin),
        Q = N bsr 7,
        {Q band 16#7f, Q}.

That is, the remainder would be calculated as `(N div 128) rem 128`.

Fixes erlang#7566
@bjorng bjorng closed this as completed in 478cefa Aug 18, 2023
bjorng added a commit that referenced this issue Aug 18, 2023
* bjorn/jit/fix-rem-div/GH-7566/OTP-18724:
  Fix incorrect fusion of `div` and `rem`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is reported as a bug team:VM Assigned to OTP team VM
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants