beam_validator: Tighten and simplify map validation code

The assert_strict_literal_termorder/1 function is used to validate the
get_map_elements and has_map_fields instructions. In neither case is
it useful to allow an empty lists of fields, so we should no longer
allow an empty list.

The mmap/2 function is cute, but it is used in only one place, so it
is much simpler to write a special-purpose function to extract the
keys from the list of map pairs.

3 files changed, 71 insertions, 32 deletions

diff --git a/lib/compiler/src/beam_validator.erl b/lib/compiler/src/beam_validator.erl
index c156cf79fe..1f2448d433 100644
--- a/lib/compiler/src/beam_validator.erl
+++ b/lib/compiler/src/beam_validator.erl
@@ -891,10 +891,14 @@ valfun_4(_, _) ->
 verify_get_map(Fail, Src, List, Vst0) ->
     assert_term(Src, Vst0),
     Vst1 = branch_state(Fail, Vst0),
-    Lits = mmap(fun(L,_R) -> [L] end, List),
-    assert_strict_literal_termorder(Lits),
+    Keys = extract_map_keys(List),
+    assert_strict_literal_termorder(Keys),
     verify_get_map_pair(List,Vst0,Vst1).
 
+extract_map_keys([Key,_Val|T]) ->
+    [Key|extract_map_keys(T)];
+extract_map_keys([]) -> [].
+
 verify_get_map_pair([],_,Vst) -> Vst;
 verify_get_map_pair([Src,Dst|Vs],Vst0,Vsti) ->
     assert_term(Src, Vst0),
@@ -1122,38 +1126,30 @@ assert_freg_set(Fr, _) -> error({bad_source,Fr}).
 
 %%% Maps
 
-%% ensure that a list of literals has a strict
-%% ascending term order (also meaning unique literals).
-%% Single item lists may have registers.
-assert_strict_literal_termorder([_]) -> ok;
-assert_strict_literal_termorder(Ls) ->
-    Vs = lists:map(fun (L) -> get_literal(L) end, Ls),
+%% A single item list may be either a list or a register.
+%%
+%% A list with more than item must contain literals in
+%% ascending term order.
+%%
+%% An empty list is not allowed.
+
+assert_strict_literal_termorder([]) ->
+    %% There is no reason to use the get_map_elements and
+    %% has_map_fields instructions with empty lists.
+    error(empty_field_list);
+assert_strict_literal_termorder([_]) ->
+    ok;
+assert_strict_literal_termorder([_,_|_]=Ls) ->
+    Vs = [get_literal(L) || L <- Ls],
     case check_strict_value_termorder(Vs) of
 	true ->  ok;
-	false -> error({not_strict_order, Ls})
+	false -> error(not_strict_order)
     end.
 
-%% usage:
-%% mmap(fun(A,B) -> [{A,B}] end, [1,2,3,4]),
-%% [{1,2},{3,4}]
-
-mmap(F,List) ->
-    {arity,Ar} = erlang:fun_info(F,arity),
-    mmap(F,Ar,List).
-mmap(_F,_,[]) -> [];
-mmap(F,Ar,List) ->
-    {Hd,Tl} = lists:split(Ar,List),
-    apply(F,Hd) ++ mmap(F,Ar,Tl).
-
-check_strict_value_termorder([]) -> true;
-check_strict_value_termorder([_]) -> true;
-check_strict_value_termorder([V1,V2]) ->
-    erts_internal:cmp_term(V1,V2) < 0;
-check_strict_value_termorder([V1,V2|Vs]) ->
-    case erts_internal:cmp_term(V1,V2) < 0 of
-	true -> check_strict_value_termorder([V2|Vs]);
-	false -> false
-    end.
+check_strict_value_termorder([V1|[V2|_]=Vs]) ->
+    erts_internal:cmp_term(V1, V2) < 0 andalso
+	check_strict_value_termorder(Vs);
+check_strict_value_termorder([_]) -> true.
 
 %%%
 %%% Binary matching.
diff --git a/lib/compiler/test/beam_validator_SUITE.erl b/lib/compiler/test/beam_validator_SUITE.erl
index 27979647c6..8bbbcb21f2 100644
--- a/lib/compiler/test/beam_validator_SUITE.erl
+++ b/lib/compiler/test/beam_validator_SUITE.erl
@@ -30,7 +30,8 @@
 	 freg_range/1,freg_uninit/1,freg_state/1,
 	 bin_match/1,bad_bin_match/1,bin_aligned/1,bad_dsetel/1,
 	 state_after_fault_in_catch/1,no_exception_in_catch/1,
-	 undef_label/1,illegal_instruction/1,failing_gc_guard_bif/1]).
+	 undef_label/1,illegal_instruction/1,failing_gc_guard_bif/1,
+	 map_field_lists/1]).
 	 
 -include_lib("test_server/include/test_server.hrl").
 
@@ -58,7 +59,8 @@ groups() ->
        bad_catch_try,cons_guard,freg_range,freg_uninit,
        freg_state,bin_match,bad_bin_match,bin_aligned,bad_dsetel,
        state_after_fault_in_catch,no_exception_in_catch,
-       undef_label,illegal_instruction,failing_gc_guard_bif]}].
+       undef_label,illegal_instruction,failing_gc_guard_bif,
+       map_field_lists]}].
 
 init_per_suite(Config) ->
     Config.
@@ -414,6 +416,18 @@ process_request_foo(_) ->
 process_request_bar(Pid, [Response]) when is_pid(Pid) ->
     Response.
 
+map_field_lists(Config) ->
+    Errors = do_val(map_field_lists, Config),
+    [{{map_field_lists,x,1},
+      {{test,has_map_fields,{f,1},{x,0},
+	{list,[{atom,z},{atom,a}]}},
+       5,
+       not_strict_order}},
+     {{map_field_lists,y,1},
+      {{test,has_map_fields,{f,3},{x,0},{list,[]}},
+       5,
+       empty_field_list}}
+    ] = Errors.
 
 %%%-------------------------------------------------------------------------
 
diff --git a/lib/compiler/test/beam_validator_SUITE_data/map_field_lists.S b/lib/compiler/test/beam_validator_SUITE_data/map_field_lists.S
new file mode 100644
index 0000000000..9af68c82d4
--- /dev/null
+++ b/lib/compiler/test/beam_validator_SUITE_data/map_field_lists.S
@@ -0,0 +1,29 @@
+{module, map_field_lists}.  %% version = 0
+
+{exports, [{x,1},{y,1}]}.
+
+{attributes, []}.
+
+{labels, 5}.
+
+
+{function, x, 1, 2}.
+  {label,1}.
+    {line,[{location,"map_field_lists.erl",4}]}.
+    {func_info,{atom,map_field_lists},{atom,x},1}.
+  {label,2}.
+    {test,is_map,{f,1},[{x,0}]}.
+    {test,has_map_fields,{f,1},{x,0},{list,[{atom,z},{atom,a}]}}.
+    {move,{atom,ok},{x,0}}.
+    return.
+
+
+{function, y, 1, 4}.
+  {label,3}.
+    {line,[{location,"map_field_lists.erl",7}]}.
+    {func_info,{atom,map_field_lists},{atom,y},1}.
+  {label,4}.
+    {test,is_map,{f,3},[{x,0}]}.
+    {test,has_map_fields,{f,3},{x,0},{list,[]}}.
+    {move,{atom,ok},{x,0}}.
+    return.