Possible problem with safe destructive update of tuples

[badlop]

Describe the bug

I found unexpected behavior when testing some old source code with Erlang 27.

The problem may be related to this improvement introduced in OTP 27.0 (I'm just guessing):

Safe destructive update of tuples has been implemented in the compiler and runtime system. This allows the VM to update tuples in-place when it is safe to do so, thus improving performance by doing less copying but also by producing less garbage.

To Reproduce

The problem is 100% reproducible with the example code. I tried to reduce the code as much as possible, but it still includes calls to ets, maps... that probably can be simplified once the problem is better delimited.

Expected behavior

test1/0 finishes correctly, test2 and test3 fail, even if they are almost identical to test1.

Affected versions

Tested and affected: Erlang/OTP 27.0.1 and 27.1.1

Not tested with older 27 versions or git master branch.

Tested and not affected: Erlang 26.2.5.

Additional context

Example module to reproduce the problem:

-module(test).

-export([test1/0, test2/0, test3/0]).

%%%
%%% test1 succeeds
%%%

test1() ->
    {Find, Details} = setup(),
    {ok1, _} = Find,
    {ok2, _, _} = Details,
    ok.

setup() ->
    TableName = table_name,
    catch ets:new(TableName, [named_table, public]),

    ElementName = <<"element-name">>,
    ContentName = <<"content-name">>,
    Element = {ElementName, maps:put(ContentName, [], maps:new())},

    ets:insert(TableName, Element),

    Content =
        try
            ets:lookup_element(TableName, ElementName, 2)
        catch
            _:badarg ->
                #{}
        end,

    Find =
        case maps:find(ContentName, Content) of
            {ok, _} ->
                {ok1, aa};
            error ->
                ok
        end,

    Details =
        try function1() of
            good ->
                {ok2, bb, cc}
        catch
            _:_ ->
                io:format("~nsome error was catched!~n", []),
                {error, some_explanation}
        end,
    {Find, Details}.

function1() ->
    good.

%%%
%%% test2 fails with Erlang/OTP 27
%%%
%%% ** exception error: no match of right hand side value {error,some_explanation}
%%% in function  test:test2/0 (test.erl, line 99)
%%%

test2() ->
    TableName = table_name,
    catch ets:new(TableName, [named_table, public]),

    ElementName = <<"element-name">>,
    ContentName = <<"content-name">>,
    Element = {ElementName, maps:put(ContentName, [], maps:new())},

    ets:insert(TableName, Element),

    Content =
        try
            ets:lookup_element(TableName, ElementName, 2)
        catch
            _:badarg ->
                #{}
        end,

    Find =
        case maps:find(ContentName, Content) of
            {ok, _} ->
                {ok1, aa};
            error ->
                ok
        end,

    Details =
        try function2() of
            good ->
                {ok2, bb, cc}
        catch
            _:_ ->
                io:format("~nsome error was catched!~n", []),
                {error, some_explanation}
        end,

    {ok1, _} = Find,
    {ok2, _, _} = Details,
    ok.

function2() ->
    good.

%%%
%%% test3 fails with Erlang/OTP 27
%%%
%%% ** exception error: no match of right hand side value {{ok1,aa},{ok2,bb,cc}}
%%% in function  test:test3/0 (test.erl, line 147)
%%%

test3() ->
    TableName = table_name,
    catch ets:new(TableName, [named_table, public]),

    ElementName = <<"element-name">>,
    ContentName = <<"content-name">>,
    Element = {ElementName, maps:put(ContentName, [], maps:new())},

    ets:insert(TableName, Element),

    Content =
        try
            ets:lookup_element(TableName, ElementName, 2)
        catch
            _:badarg ->
                #{}
        end,

    Find =
        case maps:find(ContentName, Content) of
            {ok, _} ->
                {ok1, aa};
            error ->
                ok
        end,

    Details =
        try function3() of
            good ->
                {ok2, bb, cc}
        catch
            _:_ ->
                io:format("~nsome error was catched!~n", []),
                {error, some_explanation}
        end,
    {{ok1, _}, {ok2, _, _}} = {Find, Details},
    ok.

function3() ->
    good.

[frej]

I'm taking a look.

[frej]

This appears to be unrelated to the destructive update, if we just look at the first failing function

test.erl

-module(test).

-export([test2/0]).

test2() ->
    TableName = table_name,
    catch ets:new(TableName, [named_table, public]),

    ElementName = <<"element-name">>,
    ContentName = <<"content-name">>,
    Element = {ElementName, maps:put(ContentName, [], maps:new())},

    ets:insert(TableName, Element),

    Content =
        try
            ets:lookup_element(TableName, ElementName, 2)
        catch
            _:badarg ->
                #{}
        end,

    Find =
        case maps:find(ContentName, Content) of
            {ok, _} ->
                {ok1, aa};
            error ->
                ok
        end,

    Details =
        try function2() of
            good ->
		io:format("function2 returned good~n"),
                {ok2, bb, cc}
        catch
            _:_ ->
                io:format("~nsome error was catched!~n", []),
                {error, some_explanation}
        end,

    {ok1, _} = Find,
    {ok2, _, _} = Details,
    ok.

function2() ->
    good.

For the call to function2/0 and the following pattern matching, we end up with following beam-assembly which, to me, looks perfectly fine:

  {label,8}.
    {'try',{y,1},{f,9}}.
    {line,[{scope,[3]},{location,"test.erl",39}]}.
    {call,0,{f,15}}. % function2/0
    {'%',{var_info,{x,0},[{type,{t_atom,[good]}}]}}.
    {try_end,{y,1}}.
    {move,{literal,"function2 returned good~n"},{x,0}}.
    {line,[{scope,[3]},{location,"test.erl",41}]}.
    {call_ext,1,{extfunc,io,format,1}}.
    {move,{literal,{ok2,bb,cc}},{x,0}}.
    {jump,{f,10}}.
  {label,9}.
    {try_case,{y,1}}.
    {move,nil,{x,1}}.
    {move,{literal,"~nsome error was catched!~n"},{x,0}}.
    {line,[{scope,[3]},{location,"test.erl",45}]}.
    {call_ext,2,{extfunc,io,format,2}}.
    {move,{literal,{error,some_explanation}},{x,0}}.
  {label,10}.
    {test,is_tuple,
          {f,13},
          [{tr,{y,0},
               {t_union,{t_atom,[ok]},
                        none,none,
                        [{{2,{t_atom,[ok1]}},
                          {t_tuple,2,true,
                                   #{1 => {t_atom,[ok1]},
                                     2 => {t_atom,[aa]}}}}],
                        none}}]}.
    {test,test_arity,{f,12},[{x,0},3]}.
    {move,{atom,ok},{x,0}}.
    {deallocate,2}.
    return.
  {label,11}.
    {bif,raise,{f,0},[{x,1},{x,0}],{x,0}}.
  {label,12}.
    {line,[{scope,[0]},{location,"test.erl",50}]}.
    {badmatch,{literal,{error,some_explanation}}}.
  {label,13}.
    {line,[{scope,[0]},{location,"test.erl",49}]}.
    {badmatch,{atom,ok}}.

If we load this module into the x86 JIT with +JDdump true and look at the x86-assembly we can se that {ok2,bb,cc}} is put in {x,0} which corresponds to $rdi and Details. Find is in {y,0} which is stored in [rsp]. For block 10, the JIT has produced:

label_10:
# i_is_tuple_of_arity_ff_ffsA
    mov rsi, qword ptr [rsp]
# simplified tuple test since the source is always a tuple when boxed
    rex test sil, 1
    jne label_13
    cmp dword ptr [rsi-2], 192
    jne label_12
# i_move_sd
    mov qword ptr [rbx], 31563
# deallocate_t
    add rsp, 16
# return
    dec r14d
    jl L51
    ret

If we place a breakpoint following the first instruction of block 10 and dump $rsi and $rdi using etp we can see that they are {ok1,aa} and {ok2,bb,cc} respectively. The cmp dword ptr [rsi-2], 192 fails and we end up at block 12, which reports the match error at line 50. To me this looks like that the JIT has messed up the second test in block 10. @bjorng can you take a look?

[bjorng]

@badlop, thanks for reporting this issue. The linked pull request fixes the issue. It will be included in the next emergency patch.

@frej, thanks for investigating.