Merge pull request #518 from pguyot/w16/fix-call_ext_last-throw-stack…

…_base

Add workaround for crash related to `call_ext_last/3`

BEAM's compiler and especially beam_trim optimization pass can generate an
incorrect deallocation `n_words` parameter for `call_ext_last/3`.
As a workaround, deallocate after the nif call, and before any error handling
that may need the stack to find catch handlers.

See:  erlang/otp#7152

Add test that currently crashes on master without this change

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

src/libAtomVM/opcodesswitch.h

@@ -1595,9 +1595,6 @@ static bool maybe_call_native(Context *ctx, AtomString module_name, AtomString f
 
                     TRACE_CALL_EXT(ctx, mod, "call_ext_last", index, arity);
 
-                    ctx->cp = ctx->e[n_words];
-                    ctx->e += (n_words + 1);
-
                     const struct ExportedFunction *func = mod->imported_funcs[index].func;
 
                     if (func->type == UnresolvedFunctionCall) {
@@ -1617,11 +1614,25 @@ static bool maybe_call_native(Context *ctx, AtomString module_name, AtomString f
                             }
                             ctx->x[0] = return_value;
 
+                            // We deallocate after (instead of before) as a
+                            // workaround for issue
+                            // https://github.com/erlang/otp/issues/7152
+
+                            ctx->cp = ctx->e[n_words];
+                            ctx->e += (n_words + 1);
+
                             DO_RETURN();
 
                             break;
                         }
                         case ModuleFunction: {
+                            // In the non-nif case, we can deallocate before
+                            // (and it doesn't matter as the code below does
+                            // not access ctx->e or ctx->cp)
+
+                            ctx->cp = ctx->e[n_words];
+                            ctx->e += (n_words + 1);
+
                             const struct ModuleFunction *jump = EXPORTED_FUNCTION_TO_MODULE_FUNCTION(func);
 
                             mod = jump->target;

tests/erlang_tests/CMakeLists.txt

@@ -425,6 +425,7 @@ compile_erlang(bs_context_to_binary_with_offset)
 compile_erlang(bs_restore2_start_offset)
 compile_erlang(test_refc_binaries)
 compile_erlang(test_sub_binaries)
+compile_erlang(test_throw_call_ext_last)
 compile_erlang(bs_append_extra_words)
 
 compile_erlang(test_monotonic_time)
@@ -839,6 +840,7 @@ add_custom_target(erlang_test_modules DEPENDS
 
     test_refc_binaries.beam
     test_sub_binaries.beam
+    test_throw_call_ext_last.beam
     test_function_exported.beam
     test_list_to_tuple.beam
 

tests/erlang_tests/test_throw_call_ext_last.erl

@@ -0,0 +1,133 @@
+%
+% This file is part of AtomVM.
+%
+% Copyright 2023 Paul Guyot <pguyot@kallisys.net>
+%
+% Licensed under the Apache License, Version 2.0 (the "License");
+% you may not use this file except in compliance with the License.
+% You may obtain a copy of the License at
+%
+%    http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS,
+% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+% See the License for the specific language governing permissions and
+% limitations under the License.
+%
+% SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later
+%
+
+-module(test_throw_call_ext_last).
+
+-export([start/0, loop/1]).
+
+-record(state, {
+    bin
+}).
+
+start() ->
+    ok = run_test(fun() -> test_count_binary() end),
+    {error, {heap_delta, _Delta}} = run_test(fun() -> test_spawn_fun_sub_binary() end),
+    0.
+
+test_spawn_fun_sub_binary() ->
+    Bin = create_binary(1024),
+    BinarySize = erlang:byte_size(Bin),
+    %%
+    %% Spawn a function, passing a refc binary through the args
+    %%
+    LargeSubBin = binary:part(Bin, 1, BinarySize - 1),
+    Pid = erlang:spawn(fun() -> loop(#state{bin = LargeSubBin}) end),
+    PidHeapSize0 = get_heap_size(Pid),
+    %%
+    %% Make sure we can get what we spawned
+    %%
+    LargeSubBin = send(Pid, get),
+    %%
+    %% Free the refc binary; heap should decrease
+    %%
+    ok = send(Pid, free),
+    PidHeapSize2 = get_heap_size(Pid),
+    case PidHeapSize2 - PidHeapSize0 of
+        0 -> ok;
+        % should be call_ext_last
+        Delta -> throw({heap_delta, Delta})
+    end,
+    ok = send(Pid, halt),
+    ok.
+
+test_count_binary() ->
+    _ = create_binary(1024),
+    ok.
+
+%%
+%% helper functions
+%%
+
+get_heap_size() ->
+    erlang:garbage_collect(),
+    {heap_size, Size} = erlang:process_info(self(), heap_size),
+    Size * erlang:system_info(wordsize).
+
+get_heap_size(Pid) ->
+    send(Pid, get_heap_size).
+
+send(Pid, Msg) ->
+    Ref = erlang:make_ref(),
+    Pid ! {self(), Ref, Msg},
+    receive
+        {Ref, Reply} -> Reply
+    end.
+
+loop(State) ->
+    erlang:garbage_collect(),
+    receive
+        {Pid, Ref, get} ->
+            Pid ! {Ref, State#state.bin},
+            loop(State);
+        {Pid, Ref, free} ->
+            Pid ! {Ref, ok},
+            loop(State#state{bin = undefined});
+        {Pid, Ref, get_heap_size} ->
+            Pid ! {Ref, get_heap_size()},
+            loop(State);
+        {Pid, Ref, {ref, Bin}} ->
+            Pid ! {Ref, ok},
+            loop(State#state{bin = Bin});
+        {Pid, Ref, halt} ->
+            Pid ! {Ref, ok}
+    end.
+
+create_binary(N) when is_integer(N) ->
+    S = create_string(N, []),
+    R = erlang:list_to_binary(S),
+    R;
+create_binary(S) when is_list(S) ->
+    list_to_binary(S).
+
+create_string(0, Accum) ->
+    Accum;
+create_string(N, Accum) ->
+    create_string(N - 1, [N rem 256 | Accum]).
+
+run_test(Fun) ->
+    Self = self(),
+    _Pid = spawn(fun() -> execute(Self, Fun) end),
+    receive
+        ok ->
+            ok;
+        Error ->
+            Error
+    end.
+
+execute(Pid, Fun) ->
+    Result =
+        try
+            Fun(),
+            ok
+        catch
+            _:Error ->
+                {error, Error}
+        end,
+    Pid ! Result.

tests/test.c

@@ -354,6 +354,7 @@ struct Test tests[] = {
     TEST_CASE(test_map),
     TEST_CASE_ATOMVM_ONLY(test_refc_binaries, 0),
     TEST_CASE(test_sub_binaries),
+    TEST_CASE_ATOMVM_ONLY(test_throw_call_ext_last, 0),
 
     TEST_CASE_EXPECTED(ceilint, 1),
     TEST_CASE_EXPECTED(ceilbadarg, -1),