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

Signed-off-by: Paul Guyot <pguyot@kallisys.net>

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),