From d84eadb45fbea1a455a3cd1a90ededd7e4b2c57b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 23 Feb 2017 22:55:35 +0100 Subject: [PATCH] Improve checking of bitstrings --- lib/elixir/src/elixir_bitstring.erl | 316 +++++++++++------- .../test/elixir/kernel/expansion_test.exs | 128 +++++-- 2 files changed, 289 insertions(+), 155 deletions(-) diff --git a/lib/elixir/src/elixir_bitstring.erl b/lib/elixir/src/elixir_bitstring.erl index 85056f2d18..51d698e199 100644 --- a/lib/elixir/src/elixir_bitstring.erl +++ b/lib/elixir/src/elixir_bitstring.erl @@ -9,17 +9,17 @@ expand(Meta, Args, E) -> case ?key(E, context) of match -> - {EArgs, EA} = expand_bitstr(Meta, fun elixir_expand:expand/2, Args, [], E), + {EArgs, EA} = expand(Meta, fun elixir_expand:expand/2, Args, [], E), {{'<<>>', Meta, EArgs}, EA}; _ -> - {EArgs, {EC, EV}} = expand_bitstr(Meta, fun elixir_expand:expand_arg/2, Args, [], {E, E}), + {EArgs, {EC, EV}} = expand(Meta, fun elixir_expand:expand_arg/2, Args, [], {E, E}), {{'<<>>', Meta, EArgs}, elixir_env:mergea(EV, EC)} end. -expand_bitstr(_BitstrMeta, _Fun, [], Acc, E) -> +expand(_BitstrMeta, _Fun, [], Acc, E) -> {lists:reverse(Acc), E}; -expand_bitstr(BitstrMeta, Fun, [{'::', Meta, [Left, Right]} | T], Acc, E) -> - {ELeft, EL} = expand_bitstr_component(Meta, Left, Fun, E), +expand(BitstrMeta, Fun, [{'::', Meta, [Left, Right]} | T], Acc, E) -> + {ELeft, EL} = expand_expr(Meta, Left, Fun, E), %% Variables defined outside the binary can be accounted %% on subparts, however we can't assign new variables. @@ -28,18 +28,37 @@ expand_bitstr(BitstrMeta, Fun, [{'::', Meta, [Left, Right]} | T], Acc, E) -> _ -> E#{context := nil} %% expand_each, revert assigns end, - ERight = expand_bit_info(ELeft, Meta, Right, ER), - expand_bitstr(BitstrMeta, Fun, T, [{'::', Meta, [ELeft, ERight]} | Acc], EL); -expand_bitstr(BitstrMeta, Fun, [{_, Meta, _} = H | T], Acc, E) -> - {Expr, ES} = expand_bitstr_component(Meta, H, Fun, E), - expand_bitstr(BitstrMeta, Fun, T, [Expr | Acc], ES); -expand_bitstr(Meta, Fun, [H | T], Acc, E) -> - {Expr, ES} = expand_bitstr_component(Meta, H, Fun, E), - expand_bitstr(Meta, Fun, T, [Expr | Acc], ES). + ERight = expand_specs(expr_type(ELeft), Meta, Right, ER), + expand(BitstrMeta, Fun, T, [{'::', Meta, [ELeft, ERight]} | Acc], EL); +expand(BitstrMeta, Fun, [{_, Meta, _} = H | T], Acc, E) -> + {Expr, ES} = expand_expr(Meta, H, Fun, E), + expand(BitstrMeta, Fun, T, [wrap_expr(Expr) | Acc], ES); +expand(Meta, Fun, [H | T], Acc, E) -> + {Expr, ES} = expand_expr(Meta, H, Fun, E), + expand(Meta, Fun, T, [wrap_expr(Expr) | Acc], ES). -%% Expands a "component" of the bitstring, that is, either the LHS of a :: or an -%% argument of the bitstring (such as "foo" in "<>"). -expand_bitstr_component(Meta, Component, Fun, E) -> +wrap_expr(Expr) -> + case expr_type(Expr) of + bitstring -> + {'::', [], [Expr, {bitstring, [], []}]}; + binary -> + {'::', [], [Expr, {binary, [], []}]}; + float -> + {'::', [], [Expr, {float, [], []}]}; + _ -> + {'::', [], [Expr, {integer, [], []}]} + end. + +expr_type(Integer) when is_integer(Integer) -> integer; +expr_type(Float) when is_float(Float) -> float; +expr_type(Binary) when is_binary(Binary) -> binary; +expr_type({'<<>>', _, _}) -> bitstring; +expr_type(_) -> default. + +%% Expands the expression of a bitstring, that is, the LHS of :: or +%% an argument of the bitstring (such as "foo" in "<>"). + +expand_expr(Meta, Component, Fun, E) -> case Fun(Component, E) of {EComponent, _} when is_list(EComponent); is_atom(EComponent) -> ErrorE = env_for_error(E), @@ -51,138 +70,177 @@ expand_bitstr_component(Meta, Component, Fun, E) -> env_for_error({E, _}) -> E; env_for_error(E) -> E. -%% Expand bit info +%% Expands and normalizes types of a bitstring. -expand_bit_info(Left, Meta, Info, E) -> - expand_bit_info(Left, Meta, unpack_bit_info(Info, []), default, [], E). - -expand_bit_info(Left, Meta, [{size, _, [_] = Args} | T], Size, Types, E) -> - case {Left, Size} of - %% If we see a "size" part but the LHS of :: is a literal binary, then we can - %% safely raise; we're sure that this is the first size we encountered - %% (default) otherwise we would have raised before. - {Bin, default} when is_binary(Bin) -> - form_error(Meta, ?key(E, file), ?MODULE, size_for_literal_string); - {_, default} -> - {[EArg], EE} = elixir_expand:expand_args(Args, E), - - case EArg of - {Var, _, Context} when is_atom(Var) and is_atom(Context) -> - ok; - _ when is_integer(EArg) -> - ok; - _ -> - form_error(Meta, ?key(E, file), ?MODULE, {bad_bitsize, EArg}) - end, - - expand_bit_info(Left, Meta, T, {size, [], [EArg]}, Types, EE); - _ -> - form_error(Meta, ?key(E, file), ?MODULE, duplicated_size_definition) - end; - -expand_bit_info(Left, Meta, [{Expr, ExprMeta, Args} | T], Size, Types, E) when is_atom(Expr) -> - case expand_bit_type(Expr, Args) of - type -> - {EArgs, EE} = elixir_expand:expand_args(Args, E), - validate_bit_type_args(Meta, Expr, EArgs, EE), - validate_bit_type_if_literal_bin(Left, Meta, Expr, E), - expand_bit_info(Left, Meta, T, Size, [{Expr, [], EArgs} | Types], EE); - none -> - handle_unknown_bit_info(Left, Meta, {Expr, ExprMeta, Args}, T, Size, Types, E) - end; - -expand_bit_info(_Left, Meta, [Expr | _], _Size, _Types, E) -> - form_error(Meta, ?key(E, file), ?MODULE, {undefined_bittype, Expr}); -expand_bit_info(_Left, Meta, [], Size, Types, _) -> - [H | T] = case Size of - default -> lists:reverse(Types); - _ -> lists:reverse(Types, [Size]) - end, +expand_specs(ExprType, Meta, Info, E) -> + Default = + #{size => default, + unit => default, + sign => default, + type => default, + endianess => default}, + #{size := Size, unit := Unit, type := Type, endianess := Endianess, sign := Sign} = + expand_each_spec(Meta, unpack_specs(Info, []), Default, E), + MergedType = type(Meta, ExprType, Type, E), + SizeAndUnit = size_and_unit(Meta, ExprType, Size, Unit, E), + [H | T] = build_spec(Meta, Size, Unit, MergedType, Endianess, Sign, SizeAndUnit, E), lists:foldl(fun(I, Acc) -> {'-', Meta, [Acc, I]} end, H, T). -expand_bit_type(binary, []) -> type; -expand_bit_type(integer, []) -> type; -expand_bit_type(float, []) -> type; -expand_bit_type(bitstring, []) -> type; -expand_bit_type(bytes, []) -> type; -expand_bit_type(bits, []) -> type; -expand_bit_type(utf8, []) -> type; -expand_bit_type(utf16, []) -> type; -expand_bit_type(utf32, []) -> type; -expand_bit_type(signed, []) -> type; -expand_bit_type(unsigned, []) -> type; -expand_bit_type(big, []) -> type; -expand_bit_type(little, []) -> type; -expand_bit_type(native, []) -> type; -expand_bit_type(unit, [_]) -> type; -expand_bit_type(_, _) -> none. +type(_, default, default, _) -> + integer; +type(_, ExprType, default, _) -> + ExprType; +type(_, binary, Type, _) when Type == binary; Type == bitstring; Type == utf8; Type == utf16; Type == utf32 -> + Type; +type(_, bitstring, Type, _) when Type == binary; Type == bitstring -> + Type; +type(_, integer, Type, _) when Type == integer; Type == float -> + Type; +type(_, float, Type, _) when Type == float -> + Type; +type(_, default, Type, _) -> + Type; +type(Meta, Other, Value, E) -> + form_error(Meta, ?key(E, file), ?MODULE, {bittype_mismatch, Value, Other, type}). -validate_bit_type_args(Meta, unit, [Unit], E) when not is_integer(Unit) -> - form_error(Meta, ?key(E, file), ?MODULE, {bad_unit_argument, Unit}); -validate_bit_type_args(_Meta, _Expr, _Args, _E) -> - ok. +expand_each_spec(Meta, [{Expr, _, Args} = H | T], Map, E) when is_atom(Expr) -> + case validate_spec(Expr, Args) of + {Key, Arg} -> + {Value, EE} = expand_spec_arg(Arg, E), + validate_spec_arg(Meta, Key, Value, EE), -validate_bit_type_if_literal_bin(Left, Meta, Type, E) when is_binary(Left) -> - case valid_bit_type_for_literal_bin(Type) of - true -> - ok; - false -> - form_error(Meta, ?key(E, file), ?MODULE, invalid_type_for_literal_string) + case maps:get(Key, Map) of + default -> ok; + Value -> ok; + Other -> form_error(Meta, ?key(E, file), ?MODULE, {bittype_mismatch, Value, Other, Key}) + end, + + expand_each_spec(Meta, T, maps:put(Key, Value, Map), EE); + none -> + case 'Elixir.Macro':expand(H, elixir_env:linify({?line(Meta), E})) of + H -> + form_error(Meta, ?key(E, file), ?MODULE, {undefined_bittype, H}); + NewTypes -> + expand_each_spec(Meta, unpack_specs(NewTypes, []) ++ T, Map, E) + end end; -validate_bit_type_if_literal_bin(_Left, _Meta, _Type, _E) -> - ok. +expand_each_spec(Meta, [Expr | _], _Map, E) -> + form_error(Meta, ?key(E, file), ?MODULE, {undefined_bittype, Expr}); +expand_each_spec(_Meta, [], Map, _E) -> + Map. -valid_bit_type_for_literal_bin(little) -> true; -valid_bit_type_for_literal_bin(big) -> true; -valid_bit_type_for_literal_bin(utf8) -> true; -valid_bit_type_for_literal_bin(utf16) -> true; -valid_bit_type_for_literal_bin(utf32) -> true; -valid_bit_type_for_literal_bin(bytes) -> true; -valid_bit_type_for_literal_bin(binary) -> true; -valid_bit_type_for_literal_bin(bits) -> true; -valid_bit_type_for_literal_bin(bitstring) -> true; -valid_bit_type_for_literal_bin(default) -> true; -valid_bit_type_for_literal_bin(_) -> false. - -handle_unknown_bit_info(Left, Meta, Expr, T, Size, Types, E) -> - case 'Elixir.Macro':expand(Expr, elixir_env:linify({?line(Meta), E})) of - Expr -> - form_error(Meta, ?key(E, file), ?MODULE, {undefined_bittype, Expr}); - Info -> - expand_bit_info(Left, Meta, unpack_bit_info(Info, []) ++ T, Size, Types, E) - end. - -unpack_bit_info({'-', _, [H, T]}, Acc) -> - unpack_bit_info(H, unpack_bit_info(T, Acc)); -unpack_bit_info({'*', _, [{'_', _, Atom}, Unit]}, Acc) when is_atom(Atom) and is_integer(Unit) -> +unpack_specs({'-', _, [H, T]}, Acc) -> + unpack_specs(H, unpack_specs(T, Acc)); +unpack_specs({'*', _, [{'_', _, Atom}, Unit]}, Acc) when is_atom(Atom) and is_integer(Unit) -> [{unit, [], [Unit]} | Acc]; -unpack_bit_info({'*', _, [Size, Unit]}, Acc) when is_integer(Size) and is_integer(Unit) -> +unpack_specs({'*', _, [Size, Unit]}, Acc) when is_integer(Size) and is_integer(Unit) -> [{size, [], [Size]}, {unit, [], [Unit]} | Acc]; -unpack_bit_info(Size, Acc) when is_integer(Size) -> +unpack_specs(Size, Acc) when is_integer(Size) -> [{size, [], [Size]} | Acc]; -unpack_bit_info({Expr, Meta, Args}, Acc) when is_atom(Expr) -> +unpack_specs({Expr, Meta, Args}, Acc) when is_atom(Expr) -> ListArgs = if is_atom(Args) -> []; is_list(Args) -> Args end, [{Expr, Meta, ListArgs} | Acc]; -unpack_bit_info(Other, Acc) -> +unpack_specs(Other, Acc) -> [Other | Acc]. +validate_spec(big, []) -> {endianess, big}; +validate_spec(little, []) -> {endianess, little}; +validate_spec(native, []) -> {endianess, native}; +validate_spec(size, [Size]) -> {size, Size}; +validate_spec(unit, [Unit]) -> {unit, Unit}; +validate_spec(integer, []) -> {type, integer}; +validate_spec(float, []) -> {type, float}; +validate_spec(binary, []) -> {type, binary}; +validate_spec(bytes, []) -> {type, binary}; +validate_spec(bitstring, []) -> {type, bitstring}; +validate_spec(bits, []) -> {type, bitstring}; +validate_spec(utf8, []) -> {type, utf8}; +validate_spec(utf16, []) -> {type, utf16}; +validate_spec(utf32, []) -> {type, utf32}; +validate_spec(signed, []) -> {sign, signed}; +validate_spec(unsigned, []) -> {sign, unsigned}; +validate_spec(_, _) -> none. + +expand_spec_arg(Expr, E) when is_atom(Expr); is_integer(Expr) -> {Expr, E}; +expand_spec_arg(Expr, E) -> elixir_expand:expand(Expr, E). + +validate_spec_arg(Meta, size, Value, E) -> + case Value of + {Var, _, Context} when is_atom(Var) and is_atom(Context) -> ok; + _ when is_integer(Value) -> ok; + _ -> form_error(Meta, ?key(E, file), ?MODULE, {bad_size_argument, Value}) + end; +validate_spec_arg(Meta, unit, Value, E) when not is_integer(Value) -> + form_error(Meta, ?key(E, file), ?MODULE, {bad_unit_argument, Value}); +validate_spec_arg(_Meta, _Key, _Value, _E) -> + ok. + +build_spec(Meta, Size, Unit, Type, Endianess, Sign, Spec, E) when Type == utf8; Type == utf16; Type == utf32 -> + if + Size /= default; Unit /= default -> + form_error(Meta, ?key(E, file), ?MODULE, bittype_utf); + Sign /= default -> + form_error(Meta, ?key(E, file), ?MODULE, bittype_signed); + true -> + add_spec(Type, add_spec(Endianess, Spec)) + end; + +build_spec(Meta, _Size, Unit, Type, _Endianess, Sign, Spec, E) when Type == binary; Type == bitstring -> + if + Type == bitstring, Unit /= default, Unit /= 1 -> + form_error(Meta, ?key(E, file), ?MODULE, {bittype_mismatch, Unit, 1, unit}); + Sign /= default -> + form_error(Meta, ?key(E, file), ?MODULE, bittype_signed); + true -> + %% Endianess is supported but has no effect, so we just ignore it. + add_spec(Type, Spec) + end; + +build_spec(Meta, Size, Unit, Type, Endianess, Sign, Spec, E) when Type == integer; Type == float -> + if + Size == default, Unit /= default -> + form_error(Meta, ?key(E, file), ?MODULE, bittype_unit); + true -> + add_spec(Type, add_spec(Endianess, add_spec(Sign, Spec))) + end. + +add_spec(default, Spec) -> Spec; +add_spec(Key, Spec) -> [{Key, [], []} | Spec]. + +size_and_unit(Meta, bitstring, Size, Unit, E) when Size /= default; Unit /= default -> + form_error(Meta, ?key(E, file), ?MODULE, bittype_literal_bitstring); +size_and_unit(Meta, binary, Size, Unit, E) when Size /= default; Unit /= default -> + form_error(Meta, ?key(E, file), ?MODULE, bittype_literal_string); +size_and_unit(_Meta, _ExprType, Size, Unit, _E) -> + add_arg(unit, Unit, add_arg(size, Size, [])). + +add_arg(_Key, default, Spec) -> Spec; +add_arg(Key, Arg, Spec) -> [{Key, [], [Arg]} | Spec]. + +format_error(bittype_literal_bitstring) -> + "literal <<>> in bitstring supports only type specifiers, which must be one of: " + "binary or bitstring"; +format_error(bittype_literal_string) -> + "literal string in bitstring supports only endianess and type specifiers, which must be one of: " + "little, big, native, utf8, utf16, utf32, bits, bytes, binary or bitstring"; +format_error(bittype_utf) -> + "size and unit are not supported on utf types"; +format_error(bittype_signed) -> + "signed and unsigned specifiers are supported only on integer and float types"; +format_error(bittype_unit) -> + "integer and float types require a size specifier if the unit specifier is given"; format_error({invalid_literal, Literal}) -> io_lib:format("invalid literal ~ts in <<>>", ['Elixir.Macro':to_string(Literal)]); -format_error(size_for_literal_string) -> - "size is not supported for literal string in <<>>"; -format_error({bad_bitsize, Expr}) -> - io_lib:format("size in bitstring expects an integer or a variable as argument, got: ~ts", - ['Elixir.Macro':to_string(Expr)]); -format_error(duplicated_size_definition) -> - "duplicated size definition in bitstring"; +format_error({undefined_bittype, Expr}) -> + io_lib:format("unknown bitstring specifier: ~ts", ['Elixir.Macro':to_string(Expr)]); +format_error({bittype_mismatch, Val1, Val2, Where}) -> + io_lib:format("conflicting ~ts specification for bit field: \"~p\" and \"~p\"", [Where, Val1, Val2]); format_error({bad_unit_argument, Unit}) -> io_lib:format("unit in bitstring expects an integer as argument, got: ~ts", ['Elixir.Macro':to_string(Unit)]); -format_error(invalid_type_for_literal_string) -> - "invalid types for literal string in <<>>. Accepted types are: " - "little, big, utf8, utf16, utf32, bits, bytes, binary, bitstring"; -format_error({undefined_bittype, Expr}) -> - io_lib:format("unknown bitstring specifier ~ts", ['Elixir.Macro':to_string(Expr)]). +format_error({bad_size_argument, Size}) -> + io_lib:format("size in bitstring expects an integer or a variable as argument, got: ~ts", + ['Elixir.Macro':to_string(Size)]). %% Translation diff --git a/lib/elixir/test/elixir/kernel/expansion_test.exs b/lib/elixir/test/elixir/kernel/expansion_test.exs index 850f76527f..56de0fd15e 100644 --- a/lib/elixir/test/elixir/kernel/expansion_test.exs +++ b/lib/elixir/test/elixir/kernel/expansion_test.exs @@ -438,7 +438,7 @@ defmodule Kernel.ExpansionTest do test "variables do not leak with binaries" do assert expand(quote do: (for(<>, do: c = 1); c)) == - quote do: (for(<< <> <- b() >>, do: c = 1); c()) + quote do: (for(<< <> <- b() >>, do: c = 1); c()) end test "variables inside filters are available in blocks" do @@ -974,48 +974,124 @@ defmodule Kernel.ExpansionTest do end describe "bitstrings" do - test "bitstrings: size * unit" do + test "expands size * unit" do import Kernel, except: [-: 2] assert expand(quote do: <>) == - quote do: <> + quote do: <> assert expand(quote do: <>) == - quote do: <> + quote do: <> - assert expand(quote do: <>) == - quote do: <> + assert expand(quote do: <>) == + quote do: <> assert expand(quote do: <>) == - quote do: <> + quote do: <> - assert expand(quote do: <>) == + assert expand(quote do: <>) == quote do: <> end - test "expands modifiers" do - assert expand(quote do: (import Kernel.ExpansionTarget; <>)) == - quote do: (:"Elixir.Kernel.ExpansionTarget"; <>) + test "expands binary/bitstring specifiers" do + import Kernel, except: [-: 2] - assert expand(quote do: (import Kernel.ExpansionTarget; <> = 1)) == - quote do: (:"Elixir.Kernel.ExpansionTarget"; - <> = 1) - end + assert expand(quote do: <>) == quote do: <> + assert expand(quote do: <>) == quote do: <> + assert expand(quote do: <>) == quote do: <> + assert expand(quote do: <>) == quote do: <> + assert expand(quote do: <>) == quote do: <> - test "expands modifiers args" do - assert expand(quote do: (require Kernel.ExpansionTarget; <>)) == - quote do: (:"Elixir.Kernel.ExpansionTarget"; <>) - end - - test "raises if a size is provided with a literal binary" do - assert_raise CompileError, ~r"size is not supported for literal string in <<>>", fn -> - expand(quote do: <<"foo"::binary-size(3)>>) + assert_raise CompileError, ~r"signed and unsigned specifiers are supported only on integer and float type", fn -> + expand(quote do: <>) end end - test "raises for types that are not valid with a literal binary" do - assert_raise CompileError, ~r"invalid types for literal string in <<>>", fn -> - expand(quote do: <<"foo"::integer>>) + test "expands utf* specifiers" do + import Kernel, except: [-: 2] + + assert expand(quote do: <>) == quote do: <> + assert expand(quote do: <>) == quote do: <> + assert expand(quote do: <>) == quote do: <> + + assert_raise CompileError, ~r"signed and unsigned specifiers are supported only on integer and float type", fn -> + expand(quote do: <>) + end + + assert_raise CompileError, ~r"size and unit are not supported on utf types", fn -> + expand(quote do: <>) + end + end + + test "expands numbers specifiers" do + import Kernel, except: [-: 2] + + assert expand(quote do: <>) == quote do: <> + assert expand(quote do: <>) == quote do: <> + assert expand(quote do: <>) == quote do: <> + assert expand(quote do: <>) == quote do: <> + assert expand(quote do: <>) == quote do: <> + + assert_raise CompileError, ~r"integer and float types require a size specifier if the unit specifier is given", fn -> + expand(quote do: <>) + end + end + + test "expands macro specifiers" do + import Kernel, except: [-: 2] + + assert expand(quote do: (import Kernel.ExpansionTarget; <>)) == + quote do: (:"Elixir.Kernel.ExpansionTarget"; <>) + + assert expand(quote do: (import Kernel.ExpansionTarget; <> = 1)) == + quote do: (:"Elixir.Kernel.ExpansionTarget"; + <> = 1) + end + + test "expands macro in args" do + import Kernel, except: [-: 2] + + assert expand(quote do: (require Kernel.ExpansionTarget; <>)) == + quote do: (:"Elixir.Kernel.ExpansionTarget"; <>) + end + + test "raises on size or unit for literal bitstrings" do + assert_raise CompileError, ~r"literal <<>> in bitstring supports only type specifiers", fn -> + expand(quote do: << <<"foo">>::32 >>) + end + end + + test "raises on size or unit for literal strings" do + assert_raise CompileError, ~r"literal string in bitstring supports only endianess and type specifiers", fn -> + expand(quote do: <<"foo"::32>>) + end + end + + test "raises for invalid size" do + assert_raise CompileError, ~r"size in bitstring expects an integer or a variable as argument, got: :oops", fn -> + expand(quote do: <<"foo"::size(:oops)>>) + end + end + + test "raises for invalid unit" do + assert_raise CompileError, ~r"unit in bitstring expects an integer as argument, got: :oops", fn -> + expand(quote do: <<"foo"::size(8)-unit(:oops)>>) + end + end + + test "raises for unknown specifier" do + assert_raise CompileError, ~r"unknown bitstring specifier: unknown()", fn -> + expand(quote do: <<1::unknown>>) + end + end + + test "raises for conflicting specifiers" do + assert_raise CompileError, ~r"conflicting endianess specification for bit field", fn -> + expand(quote do: <<1::little-big>>) + end + + assert_raise CompileError, ~r"conflicting unit specification for bit field", fn -> + expand(quote do: <>) end end