Eliminate crash for map updates in guards

beam_validator would complain that x(1) is uninitialized
in a test_heap instruction when attempting to compile
the following code with sys_core_fold turned off:

  foo(M) when not (M#{true := 0}); [M] ->
    ok.

Simplified, the generated BEAM assembly code looked like
this:

    test is_map BadMap x(0)
    put_map_exact Fail x(0) => x(1) ...
    jump BooleanStuff

  BadMap:
    move ok => x(1)
    jump Fail

  BooleanStuff:
    ...
    move Boolean => x(2)
    jump Build

  Fail:
    move false => x(2)

  Build:
    test_heap 2 3  %% x(0), x(1), x(2) must be live.
    ...

That is, if put_map_exact failed, control would transfer
to the label Fail without initializing x(1).

Fix that by making sure that x(1) is initilized even if
put_map_exact fails:

    test is_map BadMap x(0)
    put_map_exact BadLbl x(0) => x(1) ...
    jump OkLbl

  BadLbl:
    move ok => x(1)
    jump Fail

  OkLbl:
    jump BooleanStuff

  BadMap:
    move ok => x(1)
    jump Fail

  BooleanStuff:
    ...
    move Boolean => x(2)
    jump Build

  Fail:
    move false => x(2)

  Build:
    test_heap 2 3  %% x(0), x(1), x(2) must be live.
    ...

Note that this situation is rare, and that other optimization passes
(beam_dead and beam_jump in particular) will clean up this mess.

3 files changed, 67 insertions, 21 deletions

diff --git a/lib/compiler/src/beam_validator.erl b/lib/compiler/src/beam_validator.erl
index 13aa31b7c9..4c0cb6780a 100644
--- a/lib/compiler/src/beam_validator.erl
+++ b/lib/compiler/src/beam_validator.erl
@@ -658,8 +658,10 @@ valfun_4({test,is_map,{f,Lbl},[Src]}, Vst0) ->
     case Src of
 	{Tag,_} when Tag =:= x; Tag =:= y ->
 	    set_type_reg(map, Src, Vst);
+	{literal,Map} when is_map(Map) ->
+	    Vst;
 	_ ->
-	    Vst
+	    kill_state(Vst)
     end;
 valfun_4({test,_Op,{f,Lbl},Src}, Vst) ->
     validate_src(Src, Vst),
diff --git a/lib/compiler/src/v3_codegen.erl b/lib/compiler/src/v3_codegen.erl
index f5f3c73793..4df1aadd0a 100644
--- a/lib/compiler/src/v3_codegen.erl
+++ b/lib/compiler/src/v3_codegen.erl
@@ -1551,12 +1551,10 @@ set_cg([{var,R}], {binary,Segs}, Le, Vdb, Bef, #cg{bfail=Bfail}=St) ->
     %% Now generate the complete code for constructing the binary.
     Code = cg_binary(PutCode, Target, Temp, Fail, MaxRegs, Le#l.a),
     {Sis++Code,Aft,St};
-% Map single variable key
-set_cg([{var,R}], {map,Op,Map,[{map_pair,{var,_}=K,V}]}, Le, Vdb, Bef,
-       #cg{bfail=Bfail}=St) ->
 
-    Fail = {f,Bfail},
-    {Sis,Int0} = maybe_adjust_stack(Bef, Le#l.i, Le#l.i+1, Vdb, St),
+%% Map: single variable key.
+set_cg([{var,R}], {map,Op,Map,[{map_pair,{var,_}=K,V}]}, Le, Vdb, Bef, St0) ->
+    {Sis,Int0} = maybe_adjust_stack(Bef, Le#l.i, Le#l.i+1, Vdb, St0),
 
     SrcReg = cg_reg_arg_prefer_y(Map, Int0),
     Line = line(Le#l.a),
@@ -1570,21 +1568,16 @@ set_cg([{var,R}], {map,Op,Map,[{map_pair,{var,_}=K,V}]}, Le, Vdb, Bef,
     Aft = Aft0#sr{reg=put_reg(R, Aft0#sr.reg)},
     Target = fetch_reg(R, Aft#sr.reg),
 
-    I = case Op of
-	assoc -> put_map_assoc;
-	exact -> put_map_exact
-    end,
-    {Sis++[Line]++[{I,Fail,SrcReg,Target,Live,{list,List}}],Aft,St};
+    {Is,St1} = set_cg_map(Line, Op, SrcReg, Target, Live, List, St0),
+    {Sis++Is,Aft,St1};
 
-% Map (possibly) multiple literal keys
-set_cg([{var,R}], {map,Op,Map,Es}, Le, Vdb, Bef,
-       #cg{bfail=Bfail}=St) ->
+%% Map: (possibly) multiple literal keys.
+set_cg([{var,R}], {map,Op,Map,Es}, Le, Vdb, Bef, St0) ->
 
     %% assert key literals
     [] = [Var||{map_pair,{var,_}=Var,_} <- Es],
 
-    Fail = {f,Bfail},
-    {Sis,Int0} = maybe_adjust_stack(Bef, Le#l.i, Le#l.i+1, Vdb, St),
+    {Sis,Int0} = maybe_adjust_stack(Bef, Le#l.i, Le#l.i+1, Vdb, St0),
     SrcReg = cg_reg_arg_prefer_y(Map, Int0),
     Line = line(Le#l.a),
 
@@ -1599,11 +1592,10 @@ set_cg([{var,R}], {map,Op,Map,Es}, Le, Vdb, Bef,
     Aft = Aft0#sr{reg=put_reg(R, Aft0#sr.reg)},
     Target = fetch_reg(R, Aft#sr.reg),
 
-    I = case Op of
-	assoc -> put_map_assoc;
-	exact -> put_map_exact
-    end,
-    {Sis++[Line]++[{I,Fail,SrcReg,Target,Live,{list,List}}],Aft,St};
+    {Is,St1} = set_cg_map(Line, Op, SrcReg, Target, Live, List, St0),
+    {Sis++Is,Aft,St1};
+
+%% Everything else.
 set_cg([{var,R}], Con, Le, Vdb, Bef, St) ->
     %% Find a place for the return register first.
     Int = Bef#sr{reg=put_reg(R, Bef#sr.reg)},
@@ -1616,6 +1608,34 @@ set_cg([{var,R}], Con, Le, Vdb, Bef, St) ->
 	  end,
     {Ais,clear_dead(Int, Le#l.i, Vdb),St}.
 
+
+set_cg_map(Line, Op0, SrcReg, Target, Live, List, St0) ->
+    Bfail = St0#cg.bfail,
+    Fail = {f,St0#cg.bfail},
+    Op = case Op0 of
+	     assoc -> put_map_assoc;
+	     exact -> put_map_exact
+	 end,
+    {OkLbl,St1} = new_label(St0),
+    {BadLbl,St2} = new_label(St1),
+    Is = if
+	     Bfail =:= 0 orelse Op =:= put_map_assoc ->
+		 [Line,{Op,{f,0},SrcReg,Target,Live,{list,List}}];
+	     true ->
+		 %% Ensure that Target is always set, even if
+		 %% the map update operation fails. That is necessary
+		 %% because Target may be included in a test_heap
+		 %% instruction.
+		 [Line,
+		  {Op,{f,BadLbl},SrcReg,Target,Live,{list,List}},
+		  {jump,{f,OkLbl}},
+		  {label,BadLbl},
+		  {move,{atom,ok},Target},
+		  {jump,Fail},
+		  {label,OkLbl}]
+	 end,
+    {Is,St2}.
+
 %%%
 %%% Code generation for constructing binaries.
 %%%
diff --git a/lib/compiler/test/map_SUITE.erl b/lib/compiler/test/map_SUITE.erl
index c3c4862794..36e82c1459 100644
--- a/lib/compiler/test/map_SUITE.erl
+++ b/lib/compiler/test/map_SUITE.erl
@@ -1287,6 +1287,7 @@ t_guard_update(Config) when is_list(Config) ->
     first  = map_guard_update(#{}, #{x=>first}),
     second = map_guard_update(#{y=>old}, #{x=>second,y=>old}),
     third  = map_guard_update(#{x=>old,y=>old}, #{x=>third,y=>old}),
+    bad_map_guard_update(),
     ok.
 
 t_guard_update_large(Config) when is_list(Config) ->
@@ -1353,6 +1354,29 @@ map_guard_update(M1, M2) when M1#{x=>second} =:= M2 -> second;
 map_guard_update(M1, M2) when M1#{x:=third}  =:= M2 -> third;
 map_guard_update(_, _) -> error.
 
+bad_map_guard_update() ->
+    do_bad_map_guard_update(fun burns/1),
+    do_bad_map_guard_update(fun turns/1),
+    ok.
+
+do_bad_map_guard_update(Fun) ->
+    do_bad_map_guard_update_1(Fun, #{}),
+    do_bad_map_guard_update_1(Fun, #{true=>1}),
+    ok.
+
+do_bad_map_guard_update_1(Fun, Value) ->
+    %% Note: The business with the seemingly redundant fun
+    %% disables inlining, which would otherwise change the
+    %% EXIT reason.
+    {'EXIT',{function_clause,_}} = (catch Fun(Value)),
+    ok.
+
+burns(Richmond) when not (Richmond#{true := 0}); [Richmond] ->
+    specification.
+
+turns(Richmond) when not (Richmond#{true => 0}); [Richmond] ->
+    specification.
+
 t_guard_receive(Config) when is_list(Config) ->
     M0  = #{ id => 0 },
     Pid = spawn_link(fun() -> guard_receive_loop() end),