From 85701edb6bdfe7c616a8486542dc2ca4bd787113 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Wed, 14 Jan 2015 07:05:21 +0100
Subject: sys_core_fold: Correct optimization of 'case'

The optimization of a 'case' statement could lead to incorrect
code that would cause an exception at run-time.

Here is an example to show how the optimization went wrong. Start
with the following code:

  f({r,#{key:=Val},X}=S) ->
    case S of
      {r,_,_} ->
        setelement(3, Val, S)
    end.

(The record operations have already been translated to the
corresponding tuple operations.) The first step in case_opt/3 is
to substitute S to obtain:

  f({r,#{key:=Val},X}=S) ->
    case {r,#{key:=Val},X} of
      {r,_,_} ->
        setelement(3, Val, S)
    end.

After that substitution the 'case' can be simplified to:

  f({r,#{key:=Val},_}=S) ->
    case #{key:=Val} of
      NewVar ->
       setelement(3, Val, S)
    end.

That is the result from case_opt/3. Now eval_case/2 notices
that since there is only one clause left in the 'case', the
'case' can eliminated:

  f({r,#{key:=Val},_}=S) ->
    NewVar = #{key:=Val},
    setelement(3, Val, S).

Since the map construction may have a side effect, it was not
eliminated, but assigned to a variable that is never used.

The problem is that '#{key:=Val}' is fine as a pattern, but in a
construction of a new map, the '=>' operator must be used. So the
map construction will fail, generating an exception.

As a conservative correction for a maintenance release, we will
abort the 'case' optimization if the substitution into the 'case'
expression is anything but data items (tuples, conses, or
literals) or variables.

Reported-by: Dmitry Aleksandrov
---
 lib/compiler/test/match_SUITE.erl | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

(limited to 'lib/compiler/test')

diff --git a/lib/compiler/test/match_SUITE.erl b/lib/compiler/test/match_SUITE.erl
index ae7d764535..e5aaf49d6f 100644
--- a/lib/compiler/test/match_SUITE.erl
+++ b/lib/compiler/test/match_SUITE.erl
@@ -22,7 +22,7 @@
 	 init_per_group/2,end_per_group/2,
 	 pmatch/1,mixed/1,aliases/1,match_in_call/1,
 	 untuplify/1,shortcut_boolean/1,letify_guard/1,
-	 selectify/1,underscore/1,coverage/1]).
+	 selectify/1,underscore/1,match_map/1,coverage/1]).
 	 
 -include_lib("test_server/include/test_server.hrl").
 
@@ -35,7 +35,8 @@ all() ->
 groups() -> 
     [{p,test_lib:parallel(),
       [pmatch,mixed,aliases,match_in_call,untuplify,
-       shortcut_boolean,letify_guard,selectify,underscore,coverage]}].
+       shortcut_boolean,letify_guard,selectify,
+       underscore,match_map,coverage]}].
 
 
 init_per_suite(Config) ->
@@ -400,6 +401,17 @@ underscore(Config) when is_list(Config) ->
     _ = is_list(Config),
     ok.
 
+-record(s, {map,t}).
+
+match_map(Config) when is_list(Config) ->
+    Map = #{key=>{x,y},ignore=>anything},
+    #s{map=Map,t={x,y}} = do_match_map(#s{map=Map}),
+    ok.
+
+do_match_map(#s{map=#{key:=Val}}=S) ->
+    %% Would crash with a 'badarg' exception.
+    S#s{t=Val}.
+
 coverage(Config) when is_list(Config) ->
     %% Cover beam_dead.
     ok = coverage_1(x, a),
-- 
cgit v1.2.3