From 821bbd818906d5daf4b4f50121a0f1632ca18a5d Mon Sep 17 00:00:00 2001 From: Davide Bettio Date: Sun, 17 Nov 2024 19:23:28 +0100 Subject: [PATCH 1/5] Fix ahttp_client on OTP OTP doesn't like `[{binary, true} | ...]`, replace it with `[binary | ...]`. Signed-off-by: Davide Bettio --- libs/eavmlib/src/ahttp_client.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/eavmlib/src/ahttp_client.erl b/libs/eavmlib/src/ahttp_client.erl index 5b6ebcb3a..e506cd717 100644 --- a/libs/eavmlib/src/ahttp_client.erl +++ b/libs/eavmlib/src/ahttp_client.erl @@ -124,7 +124,7 @@ transform_options(Options) -> undefined -> [{active, true} | Options]; _OtherValue -> Options end, - [{binary, true} | WithActive]. + [binary | WithActive]. take_parse_headers(Options) -> case proplists:get_value(parse_headers, Options) of From 1fc7ffde578071c508311a1da5705b9ea2149eb2 Mon Sep 17 00:00:00 2001 From: Davide Bettio Date: Sun, 17 Nov 2024 19:20:32 +0100 Subject: [PATCH 2/5] Fix: add missing is_number check to fconv opcode term_conv_to_float expects a valid number term. Signed-off-by: Davide Bettio --- CHANGELOG.md | 2 ++ src/libAtomVM/opcodesswitch.h | 3 ++ tests/erlang_tests/CMakeLists.txt | 2 ++ tests/erlang_tests/fconv_fail_invalid.erl | 40 +++++++++++++++++++++++ tests/test.c | 1 + 5 files changed, 48 insertions(+) create mode 100644 tests/erlang_tests/fconv_fail_invalid.erl diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d1700ee0..ff89adcb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed specifications of nifs from `esp_adc` module - ESP32: fix `gpio:init/1` on GPIO >= 32 +- Adding missing check, passing a non numeric argument to a function expecting a floating point +might lead to a crash in certain situations. ## [0.6.5] - 2024-10-15 diff --git a/src/libAtomVM/opcodesswitch.h b/src/libAtomVM/opcodesswitch.h index de7324c2d..f660968bc 100644 --- a/src/libAtomVM/opcodesswitch.h +++ b/src/libAtomVM/opcodesswitch.h @@ -5912,6 +5912,9 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) #ifdef IMPL_EXECUTE_LOOP TRACE("fconv/2 %lx, fp%i\n", src_value, freg); context_ensure_fpregs(ctx); + if (UNLIKELY(!term_is_number(src_value))) { + RAISE_ERROR(BADARITH_ATOM); + } ctx->fr[freg] = term_conv_to_float(src_value); #endif diff --git a/tests/erlang_tests/CMakeLists.txt b/tests/erlang_tests/CMakeLists.txt index 07062c4f3..d7a0eacf9 100644 --- a/tests/erlang_tests/CMakeLists.txt +++ b/tests/erlang_tests/CMakeLists.txt @@ -385,6 +385,7 @@ compile_erlang(floatext) compile_erlang(boxed_is_not_float) compile_erlang(float_is_float) compile_erlang(float_is_number) +compile_erlang(fconv_fail_invalid) compile_erlang(float2list) compile_erlang(float2bin) @@ -857,6 +858,7 @@ add_custom_target(erlang_test_modules DEPENDS boxed_is_not_float.beam float_is_float.beam float_is_number.beam + fconv_fail_invalid.beam float2list.beam float2bin.beam diff --git a/tests/erlang_tests/fconv_fail_invalid.erl b/tests/erlang_tests/fconv_fail_invalid.erl new file mode 100644 index 000000000..cc4a5e41d --- /dev/null +++ b/tests/erlang_tests/fconv_fail_invalid.erl @@ -0,0 +1,40 @@ +% +% This file is part of AtomVM. +% +% Copyright 2024 Davide Bettio +% +% 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(fconv_fail_invalid). + +-export([start/0, deg_min_nsew_to_decimal/1]). + +start() -> + try ?MODULE:deg_min_nsew_to_decimal({5, nil, e}) of + _Any -> -1 + catch + error:badarith -> 0 + end. + +deg_min_nsew_to_decimal(Coord) -> + {Deg, Min, Nsew} = Coord, + DecimalCoord = Deg + Min / 60, + case Nsew of + n -> DecimalCoord; + s -> -DecimalCoord; + e -> DecimalCoord; + w -> -DecimalCoord + end. diff --git a/tests/test.c b/tests/test.c index 29bc258db..9b9f81035 100644 --- a/tests/test.c +++ b/tests/test.c @@ -435,6 +435,7 @@ struct Test tests[] = { TEST_CASE_EXPECTED(boxed_is_not_float, 16), TEST_CASE_EXPECTED(float_is_float, 32), TEST_CASE_EXPECTED(float_is_number, 32), + TEST_CASE(fconv_fail_invalid), TEST_CASE_EXPECTED(float2bin, 31), TEST_CASE_EXPECTED(float2list, 31), From 02fbd145d8dfa86bb9c54f743a8b866498dcbece Mon Sep 17 00:00:00 2001 From: Peter M Date: Fri, 22 Nov 2024 11:56:17 +0100 Subject: [PATCH 3/5] Fix AVMPort compiler warnings Recent changes introduced a spew of compiler warnings: https://github.com/atomvm/AtomVM/pull/1351/files Signed-off-by: Peter M --- libs/exavmlib/lib/AVMPort.ex | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libs/exavmlib/lib/AVMPort.ex b/libs/exavmlib/lib/AVMPort.ex index 12a333e7d..20767bb31 100644 --- a/libs/exavmlib/lib/AVMPort.ex +++ b/libs/exavmlib/lib/AVMPort.ex @@ -22,6 +22,8 @@ defmodule AVMPort do # This avoids crashing the compiler at build time @compile {:autoload, false} + # This avoids compiler warnings + @compile {:no_warn_undefined, [:port]} @moduledoc """ This module provides an interface to communicate with AtomVM port processes. The functionality of AVMPort.call is identical to the eavmlib :port.call/2 and From 652c074a31d8a680a3e60280c95e0fd2c9d162b8 Mon Sep 17 00:00:00 2001 From: Paul Guyot Date: Tue, 19 Nov 2024 22:42:44 +0100 Subject: [PATCH 4/5] Fix issues in http_server Fix a bug where http_server would fail to send a reply with integers. Fix another bug where http_server crashed with badmatch if connection was closed, including because of previous bug. Also fix socket_driver to return `{gen_tcp, closed}` when socket is closed on Linux instead of `{gen_tcp, {recv, 104}}` Fixes #1366 Signed-off-by: Paul Guyot --- CHANGELOG.md | 2 + libs/eavmlib/src/http_server.erl | 16 +++-- .../generic_unix/lib/socket_driver.c | 2 +- tests/libs/eavmlib/CMakeLists.txt | 1 + tests/libs/eavmlib/test_http_server.erl | 60 +++++++++++++++++++ tests/libs/eavmlib/tests.erl | 1 + 6 files changed, 76 insertions(+), 6 deletions(-) create mode 100644 tests/libs/eavmlib/test_http_server.erl diff --git a/CHANGELOG.md b/CHANGELOG.md index ff89adcb4..83a75e5f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - ESP32: fix `gpio:init/1` on GPIO >= 32 - Adding missing check, passing a non numeric argument to a function expecting a floating point might lead to a crash in certain situations. +- Fixed several bugs in `http_server` (#1366) +- Fixed generic\_unix `socket_driver` to return `{gen_tcp, closed}` when socket is closed on Linux instead of `{gen_tcp, {recv, 104}}` ## [0.6.5] - 2024-10-15 diff --git a/libs/eavmlib/src/http_server.erl b/libs/eavmlib/src/http_server.erl index 1d95e7c86..2489a7e1c 100644 --- a/libs/eavmlib/src/http_server.erl +++ b/libs/eavmlib/src/http_server.erl @@ -73,12 +73,16 @@ find_route(Method, Path, [{Target, Mod, _Opts} | T]) -> end. reply(StatusCode, ReplyBody, Conn) -> - {ok, Conn} = reply( + {ok, NewConn} = reply( StatusCode, ReplyBody, [<<"Content-Type: text/html\r\nConnection: close\r\n">>], Conn ), - Socket = proplists:get_value(socket, Conn), + Socket = proplists:get_value(socket, NewConn), gen_tcp:close(Socket), - ClosedConn = [{closed, true} | Conn], + ClosedConn = + case proplists:get_value(closed, NewConn) of + undefined -> [{closed, true} | NewConn]; + true -> NewConn + end, {ok, ClosedConn}. reply(StatusCode, ReplyBody, ReplyHeaders, Conn) -> @@ -107,13 +111,15 @@ reply(StatusCode, ReplyBody, ReplyHeaders, Conn) -> {ok, ClosedConn} end. -send_reply(Socket, [Chunk | Tail]) -> +send_reply(Socket, [Chunk | Tail]) when is_list(Chunk) orelse is_binary(Chunk) -> case gen_tcp:send(Socket, Chunk) of ok -> send_reply(Socket, Tail); {error, _} = ErrorTuple -> ErrorTuple end; send_reply(_Socket, []) -> - ok. + ok; +send_reply(Socket, IOData) -> + gen_tcp:send(Socket, IOData). code_to_status_string(200) -> <<"200 OK">>; diff --git a/src/platforms/generic_unix/lib/socket_driver.c b/src/platforms/generic_unix/lib/socket_driver.c index 3e1792fd0..ebf52e83d 100644 --- a/src/platforms/generic_unix/lib/socket_driver.c +++ b/src/platforms/generic_unix/lib/socket_driver.c @@ -790,7 +790,7 @@ static EventListener *passive_recv_callback(GlobalContext *glb, EventListener *b return NULL; } SocketDriverData *socket_data = (SocketDriverData *) ctx->platform_data; - if (len == 0) { + if (len == 0 || (len < 0 && errno == ECONNRESET)) { // {Ref, {error, closed}} BEGIN_WITH_STACK_HEAP(12, heap); term pid = listener->pid; diff --git a/tests/libs/eavmlib/CMakeLists.txt b/tests/libs/eavmlib/CMakeLists.txt index 758764111..71be02036 100644 --- a/tests/libs/eavmlib/CMakeLists.txt +++ b/tests/libs/eavmlib/CMakeLists.txt @@ -26,6 +26,7 @@ set(ERLANG_MODULES test_dir test_file test_ahttp_client + test_http_server test_port test_timer_manager ) diff --git a/tests/libs/eavmlib/test_http_server.erl b/tests/libs/eavmlib/test_http_server.erl new file mode 100644 index 000000000..b82a487e7 --- /dev/null +++ b/tests/libs/eavmlib/test_http_server.erl @@ -0,0 +1,60 @@ +% +% This file is part of AtomVM. +% +% Copyright 2024 Paul Guyot +% +% 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_http_server). +-export([test/0, handle_req/3]). + +test() -> + ok = test_chunk(). + +test_chunk() -> + Router = [ + {"*", ?MODULE, []} + ], + Pid = http_server:start_server(8080, Router), + {ok, Conn} = connect_client(5), + {ok, Conn2, _Ref} = ahttp_client:request(Conn, <<"GET">>, <<"/">>, [], undefined), + ok = loop_passive(Conn2, []), + exit(Pid, kill), + ok. + +connect_client(Retries) when Retries > 0 -> + ConnectResult = ahttp_client:connect(http, "localhost", 8080, [{active, false}]), + case ConnectResult of + {ok, Conn} -> + {ok, Conn}; + {error, _} = ConnectError -> + io:format("Request failed: ~p~n", [ConnectError]), + connect_client(Retries - 1) + end. + +handle_req("GET", [], Conn) -> + Body = [34, <<"hello">>], + http_server:reply(200, Body, Conn). + +% http_server doesn't send Content-Length, so ahttp_client doesn't know when it's done and reports closed connection +loop_passive(Conn, AccResp) -> + case ahttp_client:recv(Conn, 0) of + {ok, UpdatedConn, Responses} -> + loop_passive(UpdatedConn, lists:reverse(Responses, AccResp)); + {error, {gen_tcp, closed}} -> + [{data, _DataRef, <<"\"hello">>}, {status, _StatusRef, 200}] = AccResp, + ok + end. diff --git a/tests/libs/eavmlib/tests.erl b/tests/libs/eavmlib/tests.erl index c8ceac567..870b48c07 100644 --- a/tests/libs/eavmlib/tests.erl +++ b/tests/libs/eavmlib/tests.erl @@ -26,6 +26,7 @@ start() -> etest:test([ test_dir, test_file, + test_http_server, test_port, test_timer_manager, test_ahttp_client From 2f3c830b010412ff030e923b0c34455074bb598d Mon Sep 17 00:00:00 2001 From: Paul Guyot Date: Thu, 14 Nov 2024 22:29:27 +0100 Subject: [PATCH 5/5] Allow to run beams on CLI on generic_unix Also fix a memory leak where modules were not properly destroyed when the global context is destroyed Signed-off-by: Paul Guyot --- CHANGELOG.md | 5 +++ src/libAtomVM/globalcontext.c | 8 +++++ src/libAtomVM/globalcontext.h | 2 +- src/platforms/emscripten/src/main.c | 1 - src/platforms/generic_unix/main.c | 53 +++++++++++++++-------------- tests/test.c | 1 - 6 files changed, 42 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83a75e5f4..02b588dda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [0.6.6] - Unreleased +### Added + +- Added the ability to run beams from the CLI for Generic Unix platform (it was already possible with nodejs and emscripten). + ### Fixed - Fixed specifications of nifs from `esp_adc` module @@ -14,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 might lead to a crash in certain situations. - Fixed several bugs in `http_server` (#1366) - Fixed generic\_unix `socket_driver` to return `{gen_tcp, closed}` when socket is closed on Linux instead of `{gen_tcp, {recv, 104}}` +- Fixed a memory leak where modules were not properly destroyed when the global context is destroyd ## [0.6.5] - 2024-10-15 diff --git a/src/libAtomVM/globalcontext.c b/src/libAtomVM/globalcontext.c index 54a8d291e..7cdfc020c 100644 --- a/src/libAtomVM/globalcontext.c +++ b/src/libAtomVM/globalcontext.c @@ -194,6 +194,11 @@ COLD_FUNC void globalcontext_destroy(GlobalContext *glb) struct ListHead *item; struct ListHead *tmp; + int module_index = glb->loaded_modules_count; + for (int i = 0; i < module_index; i++) { + module_destroy(glb->modules_by_index[i]); + } + struct ListHead *open_avm_packs = synclist_nolock(&glb->avmpack_data); MUTABLE_LIST_FOR_EACH (item, tmp, open_avm_packs) { struct AVMPackData *avmpack_data = GET_LIST_ENTRY(item, struct AVMPackData, avmpack_head); @@ -622,6 +627,9 @@ Module *globalcontext_get_module(GlobalContext *global, AtomString module_name_a free(module_name); if (UNLIKELY(!loaded_module || (globalcontext_insert_module(global, loaded_module) < 0))) { + if (loaded_module) { + module_destroy(loaded_module); + } return NULL; } diff --git a/src/libAtomVM/globalcontext.h b/src/libAtomVM/globalcontext.h index 71f88b633..617162ab6 100644 --- a/src/libAtomVM/globalcontext.h +++ b/src/libAtomVM/globalcontext.h @@ -435,7 +435,7 @@ term globalcontext_existing_term_from_atom_string(GlobalContext *glb, AtomString /** * @brief Inserts a module to the modules table. * - * @details Inserts an already loaded module to the modules table and assigns and index to it so it can be retrieved later by name or index. + * @details Inserts an already loaded module to the modules table and assigns and index to it so it can be retrieved later by name or index. The module is then owned by the global context that will destroy it when globalcontext_destroy is invoked. * @param global the global context. * @param module the module that will be added to the modules table. * @returns the module index if successful, otherwise -1. diff --git a/src/platforms/emscripten/src/main.c b/src/platforms/emscripten/src/main.c index 3c168960e..fd3f00eef 100644 --- a/src/platforms/emscripten/src/main.c +++ b/src/platforms/emscripten/src/main.c @@ -182,7 +182,6 @@ int main(int argc, char **argv) global = NULL; if (main_module) { - module_destroy(main_module); main_module = NULL; } diff --git a/src/platforms/generic_unix/main.c b/src/platforms/generic_unix/main.c index de22155f2..a5b5fbacb 100644 --- a/src/platforms/generic_unix/main.c +++ b/src/platforms/generic_unix/main.c @@ -46,7 +46,7 @@ void print_help(const char *program_name) "\n" "Syntax:\n" "\n" - " %s [-h] [-v] +\n" + " %s [-h] [-v] +\n" "\n" "Options:\n" "\n" @@ -88,9 +88,7 @@ int main(int argc, char **argv) GlobalContext *glb = globalcontext_new(); - const void *startup_beam = NULL; - uint32_t startup_beam_size; - const char *startup_module_name; + Module *startup_module = NULL; for (int i = 1; i < argc; ++i) { const char *ext = strrchr(argv[i], '.'); @@ -102,50 +100,56 @@ int main(int argc, char **argv) } synclist_append(&glb->avmpack_data, &avmpack_data->avmpack_head); - if (IS_NULL_PTR(startup_beam)) { + if (IS_NULL_PTR(startup_module)) { + const void *startup_beam = NULL; + const char *startup_module_name; + uint32_t startup_beam_size; avmpack_find_section_by_flag(avmpack_data->data, 1, &startup_beam, &startup_beam_size, &startup_module_name); if (startup_beam) { avmpack_data->in_use = true; + startup_module = module_new_from_iff_binary(glb, startup_beam, startup_beam_size); + if (IS_NULL_PTR(startup_module)) { + fprintf(stderr, "Cannot load startup module: %s\n", startup_module_name); + return EXIT_FAILURE; + } + globalcontext_insert_module(glb, startup_module); + startup_module->module_platform_data = NULL; } } - } else if (i == 1 && ext && (strcmp(ext, ".beam") == 0)) { + } else if (ext && (strcmp(ext, ".beam") == 0)) { MappedFile *mapped_file = mapped_file_open_beam(argv[i]); if (!iff_is_valid_beam(mapped_file->mapped)) { - fprintf(stderr, "%s has invalid AVM Pack format.\n", argv[i]); + fprintf(stderr, "%s has invalid beam format.\n", argv[i]); return EXIT_FAILURE; } - startup_module_name = basename(argv[1]); - startup_beam = mapped_file->mapped; - startup_beam_size = mapped_file->size; - - } else if (i == 1) { - fprintf(stderr, "%s is not an AVM or a BEAM file.\n", argv[i]); - return EXIT_FAILURE; + Module *mod = module_new_from_iff_binary(glb, mapped_file->mapped, mapped_file->size); + if (IS_NULL_PTR(mod)) { + fprintf(stderr, "Cannot load module: %s\n", argv[i]); + return EXIT_FAILURE; + } + globalcontext_insert_module(glb, mod); + mod->module_platform_data = NULL; + if (IS_NULL_PTR(startup_module) && module_search_exported_function(mod, ATOM_STR("\5", "start"), 0, glb) != 0) { + startup_module = mod; + } } else { - fprintf(stderr, "%s is not an AVM file.\n", argv[i]); + fprintf(stderr, "%s is not an AVM or a BEAM file.\n", argv[i]); return EXIT_FAILURE; } } - if (IS_NULL_PTR(startup_beam)) { + if (IS_NULL_PTR(startup_module)) { fprintf(stderr, "Unable to locate entrypoint.\n"); return EXIT_FAILURE; } - Module *mod = module_new_from_iff_binary(glb, startup_beam, startup_beam_size); - if (IS_NULL_PTR(mod)) { - fprintf(stderr, "Cannot load startup module: %s\n", startup_module_name); - return EXIT_FAILURE; - } - globalcontext_insert_module(glb, mod); - mod->module_platform_data = NULL; Context *ctx = context_new(glb); ctx->leader = 1; - context_execute_loop(ctx, mod, "start", 0); + context_execute_loop(ctx, startup_module, "start", 0); term ret_value = ctx->x[0]; fprintf(stderr, "Return value: "); @@ -161,7 +165,6 @@ int main(int argc, char **argv) context_destroy(ctx); globalcontext_destroy(glb); - module_destroy(mod); return status; } diff --git a/tests/test.c b/tests/test.c index 9b9f81035..0722f8410 100644 --- a/tests/test.c +++ b/tests/test.c @@ -608,7 +608,6 @@ static int test_atom(struct Test *test) context_destroy(ctx); globalcontext_destroy(glb); - module_destroy(mod); mapped_file_close(beam_file); return result; }