8000 Potential compiler bug with OTP-28 release candidates · Issue #9813 · erlang/otp · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Potential compiler bug with OTP-28 release candidates #9813

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

Closed
alexandrejbr opened this issue May 8, 2025 · 7 comments · Fixed by #9834
Closed

Potential compiler bug with OTP-28 release candidates #9813

alexandrejbr opened this issue May 8, 2025 · 7 comments · Fixed by #9834
Assignees
Labels
bug Issue is reported as a bug team:VM Assigned to OTP team VM

Comments

@alexandrejbr
Copy link
Contributor
alexandrejbr commented May 8, 2025

Describe the bug
It's hard to describe since I am still investigating it, but we have one of our tests failing due to a badmatch. As soon as I changed the code to add an io:format/2 to print the variable R the test passes.

The test is doing something like this:

    ?line {ok, R} = somemodule:somefunction(linkstate1),   %1
    ?line {_, DT} = Creation = datetime(0),                %2
    ?line Aged = {19, setelement(1, DT, 38)},              %3
    ?line {linkstate1, _, external, {Creation, Aged}} = R, %4
datetime(Sec) ->
    {19, {2038, 1, 2, 3, 4, Sec, 6, 7, 0}}.

We get a badmatch in the line 4.

I compared the output of beam_disasm:file/1 and the diff between OTP 27.3.3 and OTP-28-rc4 is something like bellow:

                        {call_ext,2,{extfunc,erlang,put,2}},
                        {test_heap,13,0},
                        {get_tuple_element,{y,1},1,{x,0}},
-                       {update_record,{atom,copy},
+                       {update_record,{atom,inplace},
                                       9,
                                       {tr,{x,0},{t_tuple,0,false,#{}}},
                                       {x,0},

To Reproduce
I can't create a minimal reproduction as of now.

Expected behavior
The badmatch is unexpected and should not be happening.

Affected versions
All the release candidates of OTP-28.0

Additional context
I'm trying to locate the potential offender commit, if you have any tips as what commits may be the offenders I would be very happy if you could tell me since reverting commits usually is easier than bisecting for us (we have some patches on top of the official OTP).

I'm happy to provide more information, just let me know what can be helpful.

@alexandrejbr alexandrejbr added the bug Issue is reported as a bug label May 8, 2025
@lucioleKi
Copy link
Contributor
lucioleKi commented May 8, 2025

Maybe one of the commits that changed beam_ssa_destructive_update.erl? We could be doing something wrong when doing destructive update for nested tuples. @frej would be the best judge here.

Edit: I think the offender is either this refactor 2680c84 or one of the following two fixes bb17fb6, 10aa614. Could you check if the error is still present after you revert any of the 3 commits? As a sanity check, could you confirm that rc1 produces the same error?

@frej
Copy link
Contributor
frej commented May 9, 2025

Maybe one of the commits that changed beam_ssa_destructive_update.erl?

It could equally well be the alias analysis pass claiming that something that's aliased is unique.

The easy way to check if it is the destructive update pass which triggers the error is to compile with +no_ssa_opt_destructive_update, if the error disappears with the flag enabled, there is something wrong.

If it is the destructive-update pass that is the culprit (which looks like a low odds bet), an example which triggers the error would be helpful. The tricky part to that is that you'll want to reduce the size of it, but not in a way that makes the in-place update legal.

I would start by enabling the internal debugging printouts in the alias and destructive update passes and check the reasoning behind the inplace vs copy flags.

@alexandrejbr
Copy link
Contributor Author

I reverted back to release candidate 1 and the issue is still there. As soon as I compiled with the flag +no_ssa_opt_destructive_update the issue is gone.

@frej how can I enable those internal debugging printouts you mentioned?

@frej
Copy link
Contributor
frej commented May 9, 2025
-module(gh9813).

-export([do/0,datetime/1]).

do() ->
    R = e:f(),
    {_, DT} = Creation = datetime(0),
    Aged = {19, setelement(1, DT, 38)},
    {Creation, Aged} = R.

datetime(Sec) ->
    {19, {2038, Sec}}.

Seems like a reproducer. @lucioleKi and I will take it from here.

how can I enable those internal debugging printouts you mentioned?

Uncomment the DEBUG define in lib/compiler/src/beam_ssa_destructive_update.erl and DEBUG_ALIAS in lib/compiler/src/beam_ssa_alias_debug.hrl.

@lucioleKi lucioleKi self-assigned this May 9, 2025
@lucioleKi lucioleKi added the team:VM Assigned to OTP team VM label May 9, 2025
@lucioleKi
Copy link
Contributor

Hi! Could you test if removing this line can get rid of the compilation error you have?

orelse erl_internal:comp_op(Bif, Arity)

This is a simple and safe fix that's likely too pessimistic. We'll optimize it after the release.

@alexandrejbr
Copy link
Contributor Author

Hi! Could you test if removing this line can get rid of the compilation error you have?

otp/lib/compiler/src/beam_ssa_alias.erl

Line 1203 in 78f7684

orelse erl_internal:comp_op(Bif, Arity)

This is a simple and safe fix that's likely too pessimistic. We'll optimize it after the release.

I’ll test it tomorrow. Thank you.

@alexandrejbr
Copy link
Contributor Author

@lucioleKi seems to work fine after commenting that line.

lucioleKi added a commit to lucioleKi/otp that referenced this issue May 13, 2025
Fix erlang#9813. This is a safe but
pessimistic fix. Disabled test cases and optimizations will be enabled
again in a future commit.
lucioleKi added a commit to lucioleKi/otp that referenced this issue May 13, 2025
Fix erlang#9813. This is a safe but
pessimistic fix. Disabled test cases and optimizations will be enabled
again in a future commit.
frej added a commit to frej/otp that referenced this issue May 13, 2025
Previously all bifs selected by the erl_internal:comp_op/2 predicate
was erroneously considered safe from an aliasing perspective. This is
not true as they, depending on the argument types, could access
internal elements of a term which has been destructively updated. As
this makes the destructive update visible, this is illegal and should
not happen.

To fix this problem, we have to consider all comparison bifs as
aliasing operations, thus avoiding the problem described
above. Unfortunately this is a very pessimistic solution, and by
making use of type information we can avoid forced aliasing when it
can be determined that the comparison bif will be reduced to a simple
tag test.

This is less conservative fix than the one presented in erlang#9833.

Co-authored-by: Isabell H. <isabell@erlang.org>
Closes erlang#9813.
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
3 participants
0