-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Comments
Maybe one of the commits that changed 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? |
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 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 |
I reverted back to release candidate 1 and the issue is still there. As soon as I compiled with the flag @frej how can I enable those internal debugging printouts you mentioned? |
-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.
Uncomment the |
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
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. |
@lucioleKi seems to work fine after commenting that line. |
Fix erlang#9813. This is a safe but pessimistic fix. Disabled test cases and optimizations will be enabled again in a future commit.
Fix erlang#9813. This is a safe but pessimistic fix. Disabled test cases and optimizations will be enabled again in a future commit.
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.
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:
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:
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.
The text was updated successfully, but these errors were encountered: