From e553fbc8d6e55773172358f75c7e83ab38f0cb4c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Mon, 29 Jul 2019 14:21:51 +0200
Subject: Fix unsafe code sharing

---
 lib/compiler/src/beam_ssa_share.erl  |  8 ++++++--
 lib/compiler/test/beam_ssa_SUITE.erl | 26 ++++++++++++++++++++++++--
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/lib/compiler/src/beam_ssa_share.erl b/lib/compiler/src/beam_ssa_share.erl
index 426efa2cc9..73983bd34a 100644
--- a/lib/compiler/src/beam_ssa_share.erl
+++ b/lib/compiler/src/beam_ssa_share.erl
@@ -303,8 +303,12 @@ canonical_is([#b_ret{arg=Arg}], VarMap, Acc0) ->
                    Acc0
            end,
     {{ret,canonical_arg(Arg, VarMap),Acc1},VarMap};
-canonical_is([#b_br{bool=#b_var{},fail=Fail}], VarMap, Acc) ->
-    {{br,succ,Fail,Acc},VarMap};
+canonical_is([#b_br{bool=#b_var{}=Arg,fail=Fail}], VarMap, Acc) ->
+    %% A previous buggy version of this code omitted the canonicalized
+    %% argument in the return value. Unfortunately, that worked most
+    %% of the time, except when `br` terminator referenced a variable
+    %% defined in a previous block instead of in the same block.
+    {{br,canonical_arg(Arg, VarMap),succ,Fail,Acc},VarMap};
 canonical_is([#b_br{succ=Succ}], VarMap, Acc) ->
     {{br,Succ,Acc},VarMap};
 canonical_is([], VarMap, Acc) ->
diff --git a/lib/compiler/test/beam_ssa_SUITE.erl b/lib/compiler/test/beam_ssa_SUITE.erl
index dd1b7ddcd3..669ce6f5cf 100644
--- a/lib/compiler/test/beam_ssa_SUITE.erl
+++ b/lib/compiler/test/beam_ssa_SUITE.erl
@@ -524,9 +524,11 @@ do_comb_sw_2(X) ->
     erase(?MODULE).
 
 share_opt(_Config) ->
-    ok = do_share_opt(0).
+    ok = do_share_opt_1(0),
+    ok = do_share_opt_2(),
+    ok.
 
-do_share_opt(A) ->
+do_share_opt_1(A) ->
     %% The compiler would be stuck in an infinite loop in beam_ssa_share.
     case A of
         0 -> a;
@@ -535,6 +537,26 @@ do_share_opt(A) ->
     end,
     receive after 1 -> ok end.
 
+do_share_opt_2() ->
+    ok = sopt_2({[pointtopoint], [{dstaddr,any}]}, ok),
+    ok = sopt_2({[broadcast], [{broadaddr,any}]}, ok),
+    ok = sopt_2({[], []}, ok),
+    ok.
+
+sopt_2({Flags, Opts}, ok) ->
+    Broadcast = lists:member(broadcast, Flags),
+    P2P = lists:member(pointtopoint, Flags),
+    case Opts of
+        %% The following two clauses would be combined to one, silently
+        %% discarding the guard test of the P2P variable.
+        [{broadaddr,_}|Os] when Broadcast ->
+            sopt_2({Flags, Os}, ok);
+        [{dstaddr,_}|Os] when P2P ->
+            sopt_2({Flags, Os}, ok);
+        [] ->
+            ok
+    end.
+    
 beam_ssa_dead_crash(_Config) ->
     not_A_B = do_beam_ssa_dead_crash(id(false), id(true)),
     not_A_not_B = do_beam_ssa_dead_crash(false, false),
-- 
cgit v1.2.3