Bug in HiPE for map matching

gistfile1.txt

Demonstrating the bug.

The hipe_bug.S file was generated using the new SSA-based compiler.

Erlang/OTP 21 [erts-10.0] [source] [64-bit] [smp:4:4] [ds:4:4:10] [async-threads:1] [hipe]

Eshell V10.0  (abort with ^G)
1> c(hipe_bug, [from_asm,native]), hipe_bug:run().
** exception error: {badmap,{cons,{arg,0},nil}}
     in function  hipe_bug:dig_out_fc/1 
2> c(hipe_bug, [from_asm]), hipe_bug:run().       
{cons,{arg,0},nil}
3> 

The problem seems to be in the following instruction:

    {get_map_elements,{f,5},
                      {x,0},
                      {list,[{literal,{x,1}},{x,0},{literal,{x,0}},{x,1}]}}.

Note that the source register for the map ({x,0}) is also one of the destinations.
BEAM has no problem with this, as it executes the instruction atomically.

hipe_bug.erl

-module(hipe_bug).
-compile(native).

-export([run/0]).

run() ->
    Regs = #{{x,0} => {atom,function_clause},{x,1} => {cons,{arg,0},nil}},
    dig_out_fc(Regs).

dig_out_fc(Regs) ->
    case Regs of
        #{{x,0}:={atom,function_clause},{x,1}:=Args} ->
            Args;
        #{} ->
            no
    end.

hipe_bug.S

{module, hipe_bug}.  %% version = 0

{exports, [{module_info,0},{module_info,1},{run,0}]}.

{attributes, []}.

{labels, 11}.


{function, run, 0, 2}.
  {label,1}.
    {line,[{location,"hipe_bug.erl",6}]}.
    {func_info,{atom,hipe_bug},{atom,run},0}.
  {label,2}.
    {move,{literal,#{{x,0} => {atom,function_clause},
                     {x,1} => {cons,{arg,0},nil}}},
          {x,0}}.
    {call_only,1,{f,4}}.


{function, dig_out_fc, 1, 4}.
  {label,3}.
    {line,[{location,"hipe_bug.erl",10}]}.
    {func_info,{atom,hipe_bug},{atom,dig_out_fc},1}.
  {label,4}.
    {test,is_map,{f,6},[{x,0}]}.
    {get_map_elements,{f,5},
                      {x,0},
                      {list,[{literal,{x,1}},{x,0},{literal,{x,0}},{x,1}]}}.
    {test,is_eq_exact,{f,5},[{x,1},{literal,{atom,function_clause}}]}.
    return.
  {label,5}.
    {move,{atom,no},{x,0}}.
    return.
  {label,6}.
    {line,[{location,"hipe_bug.erl",11}]}.
    {case_end,{x,0}}.


{function, module_info, 0, 8}.
  {label,7}.
    {line,[]}.
    {func_info,{atom,hipe_bug},{atom,module_info},0}.
  {label,8}.
    {move,{atom,hipe_bug},{x,0}}.
    {line,[]}.
    {call_ext_only,1,{extfunc,erlang,get_module_info,1}}.


{function, module_info, 1, 10}.
  {label,9}.
    {line,[]}.
    {func_info,{atom,hipe_bug},{atom,module_info},1}.
  {label,10}.
    {move,{x,0},{x,1}}.
    {move,{atom,hipe_bug},{x,0}}.
    {line,[]}.
    {call_ext_only,2,{extfunc,erlang,get_module_info,2}}.

patch.diff

diff --git a/lib/hipe/icode/hipe_beam_to_icode.erl b/lib/hipe/icode/hipe_beam_to_icode.erl
index 3fcda7a..c6df228 100644
--- a/lib/hipe/icode/hipe_beam_to_icode.erl
+++ b/lib/hipe/icode/hipe_beam_to_icode.erl
@@ -1153,9 +1153,10 @@ trans_fun([{test,has_map_fields,{f,Lbl},Map,{list,Keys}}|Instructions], Env) ->
 		    lists:flatten([[K, {r, 0}] || K <- Keys])),
   [MapMove, TestInstructions | trans_fun(Instructions, Env2)];
 trans_fun([{get_map_elements,{f,Lbl},Map,{list,KVPs}}|Instructions], Env) ->
+  KVPs1 = overwrite_map_last(Map, KVPs),
   {MapMove, MapVar, Env1} = mk_move_and_var(Map, Env),
   {TestInstructions, GetInstructions, Env2} =
-    trans_map_query(MapVar, map_label(Lbl), Env1, KVPs),
+    trans_map_query(MapVar, map_label(Lbl), Env1, KVPs1),
   [MapMove, TestInstructions, GetInstructions | trans_fun(Instructions, Env2)];
 %%--- put_map_assoc ---
 trans_fun([{put_map_assoc,{f,Lbl},Map,Dst,_N,{list,Pairs}}|Instructions], Env) ->
@@ -1577,6 +1578,21 @@ trans_type_test2(function2, Lbl, Arg, Arity, Env) ->
 			 hipe_icode:label_name(True), map_label(Lbl)),
   {[Move1,Move2,I,True],Env2}.
 
+
+%%
+%% Makes sure that if a get_map_elements instruction will overwrite
+%% the map source, it will be done last.
+%%
+overwrite_map_last(Map, KVPs) ->
+  overwrite_map_last2(Map, KVPs, []).
+
+overwrite_map_last2(Map, [Key,Map|KVPs], _Last) ->
+  overwrite_map_last2(Map, KVPs, [Key,Map]);
+overwrite_map_last2(Map, [Key,Val|KVPs], Last) ->
+  [Key,Val|overwrite_map_last2(Map, KVPs, Last)];
+overwrite_map_last2(_Map, [], Last) ->
+  Last.
+
 %%
 %% Handles the get_map_elements instruction and the has_map_fields
 %% test instruction.