diff options
-rw-r--r-- | erts/emulator/drivers/common/inet_drv.c | 7 | ||||
-rw-r--r-- | erts/emulator/nifs/common/prim_file_nif.c | 3 | ||||
-rw-r--r-- | lib/compiler/src/beam_bsm.erl | 13 | ||||
-rw-r--r-- | lib/compiler/src/sys_core_bsm.erl | 62 | ||||
-rw-r--r-- | lib/compiler/test/bs_match_SUITE.erl | 38 | ||||
-rw-r--r-- | lib/kernel/test/gen_tcp_misc_SUITE.erl | 4 |
6 files changed, 106 insertions, 21 deletions
diff --git a/erts/emulator/drivers/common/inet_drv.c b/erts/emulator/drivers/common/inet_drv.c index 7f20477363..259a27cf57 100644 --- a/erts/emulator/drivers/common/inet_drv.c +++ b/erts/emulator/drivers/common/inet_drv.c @@ -7839,8 +7839,8 @@ static ErlDrvSSizeT inet_fill_opts(inet_descriptor* desc, * cmsg options and values */ PLACE_FOR(1+4, ptr); - *ptr = opt; - arg_ptr = ptr+1; /* Where to put total length */ + *ptr++ = opt; + arg_ptr = ptr; /* Where to put total length */ arg_sz = 0; /* Total length */ for (cmsg_top = (struct cmsghdr*)(cmsgbuf.buf + cmsg_sz), cmsg = (struct cmsghdr*)cmsgbuf.buf; @@ -7852,7 +7852,6 @@ static ErlDrvSSizeT inet_fill_opts(inet_descriptor* desc, PLACE_FOR(1+4, ptr); \ *ptr++ = OPT; \ put_cmsg_int32(cmsg, ptr); \ - ptr += 4; \ arg_sz += 1+4; \ continue; \ } @@ -7866,7 +7865,6 @@ static ErlDrvSSizeT inet_fill_opts(inet_descriptor* desc, PUT_CMSG_INT32(IPPROTO_IP, IP_TTL, INET_OPT_TTL); #endif /* BSD uses the RECV* names in CMSG fields */ - } #if defined(IPPROTO_IP) && defined(IP_RECVTOS) PUT_CMSG_INT32(IPPROTO_IP, IP_RECVTOS, INET_OPT_TOS); #endif @@ -7877,6 +7875,7 @@ static ErlDrvSSizeT inet_fill_opts(inet_descriptor* desc, PUT_CMSG_INT32(IPPROTO_IP, IP_RECVTTL, INET_OPT_TTL); #endif #undef PUT_CMSG_INT32 + } put_int32(arg_sz, arg_ptr); /* Put total length */ continue; } diff --git a/erts/emulator/nifs/common/prim_file_nif.c b/erts/emulator/nifs/common/prim_file_nif.c index a05d50b333..fd6aaa61f6 100644 --- a/erts/emulator/nifs/common/prim_file_nif.c +++ b/erts/emulator/nifs/common/prim_file_nif.c @@ -514,6 +514,7 @@ static ERL_NIF_TERM read_nif_impl(efile_data_t *d, ErlNifEnv *env, int argc, con ASSERT(bytes_read <= block_size); if(bytes_read < 0) { + enif_release_binary(&result); return posix_error_to_tuple(env, d->posix_errno); } else if(bytes_read == 0) { enif_release_binary(&result); @@ -576,6 +577,7 @@ static ERL_NIF_TERM pread_nif_impl(efile_data_t *d, ErlNifEnv *env, int argc, co bytes_read = efile_preadv(d, offset, read_vec, 1); if(bytes_read < 0) { + enif_release_binary(&result); return posix_error_to_tuple(env, d->posix_errno); } else if(bytes_read == 0) { enif_release_binary(&result); @@ -802,6 +804,7 @@ static ERL_NIF_TERM ipread_s32bu_p32bu_nif_impl(efile_data_t *d, ErlNifEnv *env, bytes_read = efile_preadv(d, payload_offset, read_vec, 1); if(bytes_read < 0) { + enif_release_binary(&payload); return posix_error_to_tuple(env, d->posix_errno); } else if(bytes_read == 0) { enif_release_binary(&payload); diff --git a/lib/compiler/src/beam_bsm.erl b/lib/compiler/src/beam_bsm.erl index abc6e96c85..1c8e0e9854 100644 --- a/lib/compiler/src/beam_bsm.erl +++ b/lib/compiler/src/beam_bsm.erl @@ -310,18 +310,7 @@ btb_reaches_match_2([{test,bs_start_match2,{f,F},Live,[Bin,_],Ctx}|Is], end; btb_reaches_match_2([{test,_,{f,F},Ss}=I|Is], Regs, D0) -> btb_ensure_not_used(Ss, I, Regs), - D1 = btb_follow_branch(F, Regs, D0), - D = case Is of - [{bs_context_to_binary,_}|_] -> - %% bs_context_to_binary following a test instruction - %% probably needs the current position to be saved as - %% the new start position, but we can't be sure. - %% Therefore, conservatively disable the optimization - %% (instead of forcing a saving of the position). - D1#btb{must_save=true,must_not_save=true}; - _ -> - D1 - end, + D = btb_follow_branch(F, Regs, D0), btb_reaches_match_1(Is, Regs, D); btb_reaches_match_2([{test,_,{f,F},_,Ss,_}=I|Is], Regs, D0) -> btb_ensure_not_used(Ss, I, Regs), diff --git a/lib/compiler/src/sys_core_bsm.erl b/lib/compiler/src/sys_core_bsm.erl index d7b26c3a56..62657933ee 100644 --- a/lib/compiler/src/sys_core_bsm.erl +++ b/lib/compiler/src/sys_core_bsm.erl @@ -44,6 +44,14 @@ function([{#c_var{name={F,Arity}}=Name,B0}|Fs], FsAcc, Ws0) -> {B,Ws} -> function(Fs, [{Name,B}|FsAcc], Ws) catch + throw:unsafe_bs_context_to_binary -> + %% Unsafe bs_context_to_binary (in the sense that the + %% contents of the binary will probably be wrong). + %% Disable binary optimizations for the entire function. + %% We don't generate an INFO message, because this happens + %% very infrequently and it would be hard to explain in + %% a comprehensible way in an INFO message. + function(Fs, [{Name,B0}|FsAcc], Ws0); Class:Error:Stack -> io:fwrite("Function: ~w/~w\n", [F,Arity]), erlang:raise(Class, Error, Stack) @@ -116,12 +124,66 @@ move_from_col(Pos, L) -> [Col|First] ++ Rest. bsm_do_an([#c_var{name=Vname}=V0|Vs0], Cs0, Case) -> + bsm_inner_context_to_binary(Cs0), Cs = bsm_do_an_var(Vname, Cs0), V = bsm_annotate_for_reuse(V0), Vs = core_lib:make_values([V|Vs0]), Case#c_case{arg=Vs,clauses=Cs}; bsm_do_an(_Vs, _Cs, Case) -> Case. +bsm_inner_context_to_binary([#c_clause{body=B}|Cs]) -> + %% Consider: + %% + %% foo(<<Length, Data/binary>>) -> %Line 1 + %% case {Data, Length} of %Line 2 + %% {_, 0} -> Data; %Line 3 + %% {<<...>>, 4} -> ... %Line 4 + %% end. + %% + %% No sub binary will be created for Data in line 1. The match + %% context will be passed on to the `case` in line 2. In line 3, + %% this pass inserts a `bs_context_to_binary` instruction to + %% convert the match context representing Data to a binary before + %% returning it. The problem is that the binary created will be + %% the original binary (including the matched out Length field), + %% not the tail of the binary as it is supposed to be. + %% + %% Here follows a heuristic to disable the binary optimizations + %% for the entire function if this code kind of code is found. + + case cerl_trees:free_variables(B) of + [] -> + %% Since there are no free variables in the body of + %% this clause, there can't be any troublesome + %% bs_context_to_binary instructions. + bsm_inner_context_to_binary(Cs); + [_|_]=Free -> + %% One of the free variables could refer to a match context + %% created by the outer binary match. + F = fun(#c_primop{name=#c_literal{val=bs_context_to_binary}, + args=[#c_var{name=V}]}, _) -> + case member(V, Free) of + true -> + %% This bs_context_to_binary instruction will + %% make a binary of the match context from an + %% outer binary match. It is very likely that + %% the contents of the binary will be wrong + %% (the original binary as opposed to only + %% the tail binary). + throw(unsafe_bs_context_to_binary); + false -> + %% Safe. This bs_context_to_binary instruction + %% will make a binary from a match context + %% defined in the body of the clause. + ok + end; + (_, _) -> + ok + end, + cerl_trees:fold(F, ok, B) + end; +bsm_inner_context_to_binary([]) -> ok. + bsm_do_an_var(V, [#c_clause{pats=[P|_],guard=G,body=B0}=C0|Cs]) -> case P of #c_var{name=VarName} -> diff --git a/lib/compiler/test/bs_match_SUITE.erl b/lib/compiler/test/bs_match_SUITE.erl index 7814738449..e97dbac8a6 100644 --- a/lib/compiler/test/bs_match_SUITE.erl +++ b/lib/compiler/test/bs_match_SUITE.erl @@ -1690,30 +1690,62 @@ non_opt_eq([], <<>>) -> %% ERL-689 -erl_689(Config) -> +erl_689(_Config) -> {{0, 0, 0}, <<>>} = do_erl_689_1(<<0>>, ?MODULE), {{2018, 8, 7}, <<>>} = do_erl_689_1(<<4,2018:16/little,8,7>>, ?MODULE), {{0, 0, 0}, <<>>} = do_erl_689_2(?MODULE, <<0>>), {{2018, 8, 7}, <<>>} = do_erl_689_2(?MODULE, <<4,2018:16/little,8,7>>), ok. -do_erl_689_1(<<Length, Data/binary>>, _) -> +do_erl_689_1(Arg1, Arg2) -> + Res = do_erl_689_1a(Arg1, Arg2), + Res = do_erl_689_1b(Arg1, Arg2). + +do_erl_689_2(Arg1, Arg2) -> + Res = do_erl_689_2a(Arg1, Arg2), + Res = do_erl_689_2b(Arg1, Arg2). + +do_erl_689_1a(<<Length, Data/binary>>, _) -> + case {Data, Length} of + {_, 0} -> + %% bs_context_to_binary would incorrectly set Data to the original + %% binary (before matching in the function head). + {{0, 0, 0}, Data}; + {<<Y:16/little, M, D, Rest/binary>>, 4} -> + {{Y, M, D}, Rest} + end. + +do_erl_689_1b(<<Length, Data/binary>>, _) -> case {Data, Length} of {_, 0} -> %% bs_context_to_binary would incorrectly set Data to the original %% binary (before matching in the function head). + id(0), {{0, 0, 0}, Data}; {<<Y:16/little, M, D, Rest/binary>>, 4} -> + id(1), + {{Y, M, D}, Rest} + end. + +do_erl_689_2a(_, <<Length, Data/binary>>) -> + case {Length, Data} of + {0, _} -> + %% bs_context_to_binary would incorrectly set Data to the original + %% binary (before matching in the function head). + {{0, 0, 0}, Data}; + {4, <<Y:16/little, M, D, Rest/binary>>} -> {{Y, M, D}, Rest} end. -do_erl_689_2(_, <<Length, Data/binary>>) -> +do_erl_689_2b(_, <<Length, Data/binary>>) -> case {Length, Data} of {0, _} -> %% bs_context_to_binary would incorrectly set Data to the original %% binary (before matching in the function head). + id(0), {{0, 0, 0}, Data}; {4, <<Y:16/little, M, D, Rest/binary>>} -> + id(1), {{Y, M, D}, Rest} end. diff --git a/lib/kernel/test/gen_tcp_misc_SUITE.erl b/lib/kernel/test/gen_tcp_misc_SUITE.erl index 358ca872f7..194522c009 100644 --- a/lib/kernel/test/gen_tcp_misc_SUITE.erl +++ b/lib/kernel/test/gen_tcp_misc_SUITE.erl @@ -1981,8 +1981,8 @@ recvtclass(_Config) -> %% pktoptions is not supported for IPv4 recvtos_ok({unix,openbsd}, OSVer) -> not semver_lt(OSVer, {6,4,0}); recvtos_ok({unix,darwin}, OSVer) -> not semver_lt(OSVer, {17,6,0}); -recvtos_ok({unix,freebsd}, OSVer) -> not semver_lt(OSVer, {11,2,0}); %% Using the option returns einval, so it is not implemented. +recvtos_ok({unix,freebsd}, OSVer) -> not semver_lt(OSVer, {11,2,0}); recvtos_ok({unix,sunos}, OSVer) -> not semver_lt(OSVer, {5,12,0}); %% Does not return any value - not implemented for pktoptions recvtos_ok({unix,linux}, OSVer) -> not semver_lt(OSVer, {3,1,0}); @@ -1993,8 +1993,8 @@ recvtos_ok(_, _) -> false. %% pktoptions is not supported for IPv4 recvttl_ok({unix,openbsd}, OSVer) -> not semver_lt(OSVer, {6,4,0}); recvttl_ok({unix,darwin}, OSVer) -> not semver_lt(OSVer, {17,6,0}); -recvttl_ok({unix,freebsd}, OSVer) -> not semver_lt(OSVer, {11,2,0}); %% Using the option returns einval, so it is not implemented. +recvttl_ok({unix,freebsd}, OSVer) -> not semver_lt(OSVer, {11,2,0}); recvttl_ok({unix,sunos}, OSVer) -> not semver_lt(OSVer, {5,12,0}); %% recvttl_ok({unix,linux}, _) -> true; |