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-1344: Compiler bug in optimization with redundant move statements #4405

Closed
OTP-Maintainer opened this issue Sep 12, 2020 · 3 comments
Closed
Assignees
Labels
bug Issue is reported as a bug priority:low team:VM Assigned to OTP team VM
Milestone

Comments

@OTP-Maintainer
Copy link

Original reporter: JIRAUSER16603
Affected version: OTP-23
Fixed in version: OTP-23.1
Component: erts
Migrated from: https://bugs.erlang.org/browse/ERL-1344


I wanted to see what the emulator would output for a binary matching object which is a total hack admittedly.  I did some binary comparisons and clever hex editing hacks to get it to work.  Incidentally, I found you do not even need to do this.   (It was interesting to see uninitialized registers have logically the value "[]" which would be "nil" in terms of BEAM or AST.)

Anyway the following code:
{code:java}
dumpbinmatch() ->
  erlang:display({<<X, _, _>> = <<3, 4, 5>>}).
{code}
Compiles to (with line number statements removed and a commented line added):
{code:java}
 {function, dumpbinmatch, 0, 6}.
  {label,5}.
    {func_info,{atom,dumpbeam},{atom,dumpbinmatch},0}.
  {label,6}.
    {move,{literal,<<3,4,5>>},{x,0}}.
    {bs_start_match4,{atom,no_fail},0,{x,0},{x,0}}.
    {test,bs_test_tail2,{f,7},[{x,0},24]}.
    {move,{literal,{<<3,4,5>>}},{x,0}}.
    %{move,{x,0},{x,0}}.
 
     {call_ext_only,1,{extfunc,erlang,display,1}}.
 
   {label,7}.
 
     {badmatch,{literal,<<3,4,5>>}}.
{code}
Remove the comment for
{code:java}
{move,{x,0},{x,0}}{code}
and the output instead of being "<<3,4,5>>" will be "#MatchState" which is very interesting as well.  The virtual machine handles this quite nicely and of course the validator will always fail if you try to compile code containing such a reference.

But here we see a redundant move statement literally corrupting the compiler output probably due to a bug there and the unexpected "No Operation" type statement occurring.  Likely it is trying to optimize dead-code and managed to delete that move statement as well as the move statement above it.  Which is just semantically plain wrong.
@OTP-Maintainer
Copy link
Author

bjorn said:

Thanks for reporting this bug.

By the way, the bug is in the runtime system and not in the compiler. Here is the pull request with the correction:

https://github.com/erlang/otp/pull/2747

@OTP-Maintainer
Copy link
Author

JIRAUSER16603 said:

Wow thank you very much for the clarification and the very fast patch for the issue!  In fact, such behavior is even desirable for the "seeing what happens in the VM" probing some of us do :).  But in general, if the loader is doing such optimizations, of course it would be more of a bug than a feature.  I am surprised, my assumption was that all optimizations would have been done at compile time, so that the loader could avoid having to do extra work - which is why I wrongly assumed it was a compiler bug, should have checked the BEAM output.

@OTP-Maintainer
Copy link
Author

bjorn said:

The loader only does optimizations that the compiler can't do. As the comment explains, the optimization is  only there to clean up after previous transformations done by the loader itself. The previous transformations replace calls to certains BIFs with move instructions when the runtime system is compiled without support for dtrace (which is usually the case).

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

No branches or pull requests

2 participants