- Notifications
You must be signed in to change notification settings - Fork 128
Merge #1814 after JIT changes #1864
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Franciszek Kubis <franciszek.kubis@swmansion.com> # Conflicts: # src/libAtomVM/module.c # src/libAtomVM/module.h # src/libAtomVM/nifs.c # src/libAtomVM/nifs.gperf
…et-object-code Implement code:get_object_code/1 nif 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
| ||
TEST_CASE(test_code_all_available_loaded), | ||
TEST_CASE_EXPECTED(test_code_load_binary, 24), | ||
#ifndef AVM_NO_JIT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pguyot This is my best try at fixing build error with build-and-test (macos-15-intel, 28, -DAVM_DISABLE_JIT=OFF). Can we do anything better than just skipping the test?
The relevant error is:
test_code_get_object_code: Unable to open export_test_module.beam Unable to open export_test_module.beam CRASH ====== pid: <0.1.0> Stacktrace: [{test_code_get_object_code,undefined,0,[]},{test_code_get_object_code,start,0,[{file,"/Users/runner/work/AtomVM/AtomVM/tests/erlang_tests/test_code_get_object_code.erl"},{line,28}]}] cp: #CP<module: 0, label: 13, offset: 756> x[0]: error x[1]: undef x[2]: {2,1,81,1,[{0,268},{0,0}],error}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test should follow the same pattern as test_code_load_binary to make sure we load the binary with precompiled native code. We will need to update it for every jit target, so maybe we should factorize the hrl at some point.
diff --git a/tests/erlang_tests/test_code_get_object_code.erl b/tests/erlang_tests/test_code_get_object_code.erl index f411441b7..80f68711f 100644 --- a/tests/erlang_tests/test_code_get_object_code.erl +++ b/tests/erlang_tests/test_code_get_object_code.erl @@ -22,8 +22,18 @@ -export([start/0, get_object_wrong_argument/1]). +-ifdef(AVM_DISABLE_JIT). -include("code_load/export_test_module_data.hrl"). +export_test_module_data() -> + ?EXPORT_TEST_MODULE_DATA. +-else. +-include("code_load/export_test_module_data_x86_64.hrl"). + +export_test_module_data() -> + ?EXPORT_TEST_MODULE_DATA_x86_64. +-endif. + start() -> ok = get_object_from_export_test_module(), ok = get_object_from_already_loaded_test_module(), @@ -43,7 +53,7 @@ get_object_from_already_loaded_test_module() -> ok. get_object_from_export_test_module() -> - Bin = ?EXPORT_TEST_MODULE_DATA, + Bin = export_test_module_data(), error = code:get_object_code(export_test_module), {module, export_test_module} = code:load_binary( export_test_module, "export_test_module.beam", Bin
However, when I do that, I have a crash with the external calls:
ok = ?MODULE:get_object_wrong_argument("a string"),
There are other external calls in these tests and they don't crash. I don't know why this one does crash, though. I have yet to investigate this.
The following two versions don't crash.
start() -> hello = ?MODULE:id(hello), ok = ?MODULE:get_object_wrong_argument("a string"), ok = ?MODULE:get_object_wrong_argument(123), ok = ?MODULE:get_object_wrong_argument({1, "a"}), ok = ?MODULE:get_object_wrong_argument([1, b, 3]), ok = get_object_from_export_test_module(), ok = get_object_from_already_loaded_test_module(), ok = get_object_from_non_existing_module(), ok = get_object_wrong_argument("a string"), ok = get_object_wrong_argument(123), ok = get_object_wrong_argument({1, "a"}), ok = get_object_wrong_argument([1, b, 3]), 0.
start() -> hello = ?MODULE:id(hello), ok = ?MODULE:get_object_wrong_argument("a string"), ok = ?MODULE:get_object_wrong_argument(123), ok = ?MODULE:get_object_wrong_argument({1, "a"}), ok = ?MODULE:get_object_wrong_argument([1, b, 3]), ok = get_object_from_export_test_module(), ok = get_object_from_already_loaded_test_module(), ok = get_object_from_non_existing_module(), ok = ?MODULE:get_object_wrong_argument("a string"), ok = ?MODULE:get_object_wrong_argument(123), ok = ?MODULE:get_object_wrong_argument({1, "a"}), ok = ?MODULE:get_object_wrong_argument([1, b, 3]), 0.
I don't know if it's related to this nif.
Also I don't like this warning:
Warning, dangling refc binary, ref_count = 1 test_code_get_object_code: OK Success.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the bug: https://github.com/atomvm/AtomVM/pull/1814/files#r2399909494
Additionally, maybe JIT is not very tolerant to hot code reloading on macOS (the mmap didn't have the proper flags), but it's not like we support hot code reloading yet.
PR #1814 fails after JIT changes, apply fix before merging it to main.
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