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

ERL-1179: Compiler/runtime yields invalid semantic behaviour (boolean value is flipped) #4149

Closed
OTP-Maintainer opened this issue Feb 24, 2020 · 5 comments
Assignees
Labels
bug Issue is reported as a bug priority:medium team:VM Assigned to OTP team VM
Milestone

Comments

@OTP-Maintainer
Copy link

Original reporter: josevalim
Affected version: OTP-23
Fixed in version: OTP-23.0
Component: compiler
Migrated from: https://bugs.erlang.org/browse/ERL-1179


This bug was found in master at commit:
{code:java}
commit a4e8656d1aa4bde0bbfab6a52585add5d7951bd1
Merge: b7d1866d48 75d4701fff
Author: Sverker Eriksson <sverker@erlang.org>
Date:   Tue Feb 18 16:38:24 2020 +0100
{code}
Take the following Erlang code:
{code:java}
-module(foo).
-export([bar/2]).

bar(_, Caller) ->
  erlang:display({find(module, Caller), find(function, Caller), find(context, Caller)}),
  IsFunction = find(function, Caller) /= nil,

  case not true of
    true ->
      ok;
    false ->
      case (not IsFunction) andalso (find(context, Caller) == match) of
        true -> ok;
        false -> erlang:display({IsFunction, Caller})
      end
  end.

-compile({inline, [find/2]}).

find(Key, Map) ->
  case Map of
    #{Key := Value} -> Value;
    #{} -> erlang:error(badmap)
  end.
{code}
Compile it with `erlc foo.erl` and now in `erl` type this:
{code:java}
1> foo:bar(baz, #{module => regex, function => nil, context => nil}).
{regex,nil,nil}
{true,#{context=>nil,function=>nil,module=>regex}}
{code}
As you can see, it is saying that `IsFunction` is true but it should be false (as it is not different to nil).

Thank you!
@OTP-Maintainer
Copy link
Author

john said:

Thanks for your report! It's a bit too late to get a fix into OTP 23 RC1, but I've got a preliminary one below:

https://github.com/jhogberg/otp/tree/john/compiler/ssa_opt_ne-flipped/ERL-1179

@OTP-Maintainer
Copy link
Author

josevalim said:

Thank you for the fix. Btw, do you have a straight-forward workaround to suggest? Without this fix, Elixir won't be usable with Erlang/OTP 23 RC1. But if there is a simple fix, I can release a new Elixir version with it. :)

@OTP-Maintainer
Copy link
Author

john said:

The fix is to not use {{/=}} or {{=/=}}, which might be a bit tricky. :D

Having slept on it I've decided to revert the change that caused this since there's little point to releasing RC1 with such a serious bug present. We'll include a variation of the above fix in RC2.

@OTP-Maintainer
Copy link
Author

josevalim said:

Ah, thank you! I have figured out that adding a "_ = atom_to_binary(IsFunction, utf8)" also fixes it but you are right, other code may be affected by this, so reverting the optimization before release makes sense.

@OTP-Maintainer
Copy link
Author

bjorn said:

The optimization has been reintroduced in a safe way in master and will be included in RC2.

@OTP-Maintainer OTP-Maintainer added bug Issue is reported as a bug team:VM Assigned to OTP team VM priority:medium labels Feb 10, 2021
@OTP-Maintainer OTP-Maintainer added this to the OTP-23.0 milestone Feb 10, 2021
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 priority:medium team:VM Assigned to OTP team VM
Projects
None yet
Development

No branches or pull requests

2 participants