Skip to content

Updated nif erlang process info#2374

Open
bettio wants to merge 18 commits into
atomvm:release-0.7from
bettio:updated-nif-erlang-process_info
Open

Updated nif erlang process info#2374
bettio wants to merge 18 commits into
atomvm:release-0.7from
bettio:updated-nif-erlang-process_info

Conversation

@bettio

@bettio bettio commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

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

Signed-off-by: Mateusz Furga <mateusz.furga@swmansion.com>
@bettio
bettio changed the base branch from main to release-0.7 July 18, 2026 23:15
.. doxygenstruct:: BuiltInAtomRequestSignal
.. doxygenstruct:: ProcessInfoRequestSignal
:allow-dot-graphs:
.. doxygenstruct:: BuiltInAtomSignal

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also fix this one.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Comment thread src/libAtomVM/context.c Outdated
}
} // else: sender died

if (signal->len == 0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make it an assert instead (or two, (signal->mode == PROCESS_INFO_LIST) and len > 0). nif_erlang_process_info doesn't send PROCESS_INFO_LIST if length is 0.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@petermm

petermm commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

AMP, usual caveats:

PR review: process_info/1,2 changes

Range reviewed: 2903b59b19fcd639368772c690d7b8aba4a81acd..ce3e2994e (last 13 commits)

Recommendation: Request changes. The normal behavior and teardown synchronization are well covered, but allocation failures and unchecked list-size arithmetic can still crash, corrupt, or permanently suspend the VM.

Findings

1. High — A reply allocation failure can crash the VM or leave the caller trapped forever

The caller sets Trap after the request has been posted (src/libAtomVM/nifs.c:3287-3292, 3318-3327), so every accepted request must eventually deliver a completion signal.

That guarantee does not hold:

  • mailbox_message_create_from_term() returns NULL when allocation fails (src/libAtomVM/mailbox.c:255-264), but mailbox_send_term_signal() passes the result directly to mailbox_post_message() (mailbox.c:293-297). Both mailbox implementations immediately dereference it (mailbox.c:232-250). This is a native null-pointer crash.
  • mailbox_send_immediate_signal() silently returns when its allocation fails (mailbox.c:299-305). The only exception reply is then lost, and the requester remains trapped indefinitely.
  • All new completion paths use one of these functions, including the teardown undefined reply and list-result replies (src/libAtomVM/context.c:327-397).

Commit c1c79aeb0 correctly makes request allocation failure raise out_of_memory, but response allocation remains fallible after the request has become irrevocable. The unchecked helper predates this range, but the new protocol depends on it for every list and teardown response and therefore does not satisfy its completion/liveness contract.

Suggested fix: reserve a no-allocation fallback completion before posting the request. For example, allocate an ImmediateSignal containing OUT_OF_MEMORY_ATOM together with the ProcessInfoRequestSignal; transfer it to the requester if normal response serialization fails, and free it after a successful response or if the requester has died. Merely adding a NULL check or returning bool from the send helpers would avoid the crash but would still strand the trapped caller, so this is not presented as a small patch.

Add allocator-fault tests for (1) term-response allocation, (2) exception-response allocation, and (3) the teardown undefined response.

2. High — List result-size overflow can under-allocate the heap and corrupt memory

Both list paths sum the required heap size without checking for size_t overflow:

  • Cross-process: src/libAtomVM/context.c:360-368
  • Self-process: src/libAtomVM/nifs.c:3330-3340

This is not limited by the request list's own memory use. A list can repeat a dynamic key such as links or monitored_by, so request length and per-item result size multiply. On a 32-bit target, two individually representable sizes can wrap total_size; memory_init_heap() also performs unchecked multiplication (src/libAtomVM/memory.c:61-68). The subsequent loops build the full result into the undersized heap (context.c:381-396, nifs.c:3348-3359), causing out-of-bounds writes instead of out_of_memory.

Use checked accumulation in both paths:

diff --git a/src/libAtomVM/context.c b/src/libAtomVM/context.c
--- a/src/libAtomVM/context.c
+++ b/src/libAtomVM/context.c
@@
 #include <fenv.h>
 #include <math.h>
+#include <stdint.h>
@@
         for (size_t i = 0; i < signal->len; i++) {
             size_t item_size;
             if (UNLIKELY(!context_get_process_info(ctx, NULL, &item_size, signal->atoms[i], NULL))) {
                 mailbox_send_immediate_signal(target, TrapExceptionSignal, BADARG_ATOM);
                 goto done;
             }
-            total_size += item_size + CONS_SIZE;
+            if (UNLIKELY(item_size > SIZE_MAX - CONS_SIZE)
+                || UNLIKELY(total_size > SIZE_MAX - (item_size + CONS_SIZE))) {
+                mailbox_send_immediate_signal(target, TrapExceptionSignal, OUT_OF_MEMORY_ATOM);
+                goto done;
+            }
+            total_size += item_size + CONS_SIZE;
         }
diff --git a/src/libAtomVM/nifs.c b/src/libAtomVM/nifs.c
--- a/src/libAtomVM/nifs.c
+++ b/src/libAtomVM/nifs.c
@@
     for (size_t i = 0; i < items_len; i++) {
         size_t item_size;
         // NOLINT(allocations-without-ensure-free) called with NULL heap, only computes size
         if (UNLIKELY(!context_get_process_info(ctx, NULL, &item_size, items[i], NULL))) {
             free(items_alloc);
             globalcontext_get_process_unlock(ctx->global, target);
             RAISE_ERROR(BADARG_ATOM);
         }
-        total_size += item_size + CONS_SIZE;
+        if (UNLIKELY(item_size > SIZE_MAX - CONS_SIZE)
+            || UNLIKELY(total_size > SIZE_MAX - (item_size + CONS_SIZE))) {
+            free(items_alloc);
+            globalcontext_get_process_unlock(ctx->global, target);
+            RAISE_ERROR(OUT_OF_MEMORY_ATOM);
+        }
+        total_size += item_size + CONS_SIZE;
     }

The signed reverse loops should also use size_t. This avoids depending on POSIX ssize_t in core/bare-metal builds and avoids implementation-defined conversion when a size_t length exceeds SSIZE_MAX:

diff --git a/src/libAtomVM/context.c b/src/libAtomVM/context.c
--- a/src/libAtomVM/context.c
+++ b/src/libAtomVM/context.c
@@
-        for (ssize_t i = (ssize_t) signal->len - 1; i >= 0; i--) {
+        for (size_t i = signal->len; i-- > 0;) {
diff --git a/src/libAtomVM/nifs.c b/src/libAtomVM/nifs.c
--- a/src/libAtomVM/nifs.c
+++ b/src/libAtomVM/nifs.c
@@
-    for (ssize_t i = (ssize_t) items_len - 1; i >= 0; i--) {
+    for (size_t i = items_len; i-- > 0;) {

The generic memory_init_heap() allocation arithmetic should also be hardened, but that broader pre-existing issue is outside this PR's smallest fix. The asynchronous branch still needs finding 1's guaranteed fallback response.

3. Medium — An asynchronous exception consumes the reply but leaves Trap set

Normal answers clear Trap in context_process_signal_trap_answer() (src/libAtomVM/context.c:406-410). TrapExceptionSignal handling only installs the exception in both execution engines:

  • Interpreter: src/libAtomVM/opcodesswitch.h:824-829
  • JIT: src/libAtomVM/jit.c:919-924

If an asynchronous out_of_memory is caught, exception handling clears the exception state but not the context flag (opcodesswitch.h:3253-3263, jit.c:1414-1422). At the next signal-processing boundary, the still-set Trap flag takes SCHEDULE_WAIT_ANY (opcodesswitch.h:920-928), despite the one expected completion already having been consumed. The process can then sleep forever.

The signal handler predates this range, but the new list allocation path adds another concrete producer at context.c:375-378. Clear the flag exactly as the successful completion path does:

diff --git a/src/libAtomVM/opcodesswitch.h b/src/libAtomVM/opcodesswitch.h
--- a/src/libAtomVM/opcodesswitch.h
+++ b/src/libAtomVM/opcodesswitch.h
@@
                 case TrapExceptionSignal: {                                                     \
                     struct ImmediateSignal *trap_exception                                      \
                         = CONTAINER_OF(signal_message, struct ImmediateSignal, base);           \
+                    context_update_flags(ctx, ~Trap, NoFlags);                                  \
                     SET_ERROR(trap_exception->immediate);                                       \
                     handle_error = true;                                                        \
                     break;                                                                      \
diff --git a/src/libAtomVM/jit.c b/src/libAtomVM/jit.c
--- a/src/libAtomVM/jit.c
+++ b/src/libAtomVM/jit.c
@@
             case TrapExceptionSignal: {
                 struct ImmediateSignal *trap_exception
                     = CONTAINER_OF(signal_message, struct ImmediateSignal, base);
+                context_update_flags(ctx, ~Trap, NoFlags);
                 set_error(ctx, jit_state, 0, trap_exception->immediate);
                 handle_error = true;
                 break;

Add a regression that injects or fault-induces a trap exception, catches it, and proves that the requester continues executing.

4. Low — The process_info/2 spec omits undefined for single-item queries

libs/estdlib/src/erlang.erl:366-376 permits undefined only for the list overload. AtomVM intentionally returns undefined for a dead process for every overload, as asserted throughout tests/erlang_tests/test_process_info.erl (for example lines 109, 125, 146, and 196). OTP has the same process-exit race.

The current spec therefore marks valid handling as unreachable and tells callers that an unconditional tuple match is safe when it is not.

diff --git a/libs/estdlib/src/erlang.erl b/libs/estdlib/src/erlang.erl
--- a/libs/estdlib/src/erlang.erl
+++ b/libs/estdlib/src/erlang.erl
@@
 -spec process_info
-    (Pid :: pid(), heap_size) -> {heap_size, non_neg_integer()};
-    (Pid :: pid(), total_heap_size) -> {total_heap_size, non_neg_integer()};
-    (Pid :: pid(), registered_name) -> {registered_name, term()} | [];
-    (Pid :: pid(), stack_size) -> {stack_size, non_neg_integer()};
-    (Pid :: pid(), message_queue_len) -> {message_queue_len, non_neg_integer()};
-    (Pid :: pid(), memory) -> {memory, non_neg_integer()};
-    (Pid :: pid(), links) -> {links, [pid()]};
-    (Pid :: pid(), monitored_by) -> {monitored_by, [pid() | resource() | port()]};
-    (Pid :: pid(), trap_exit) -> {trap_exit, boolean()};
+    (Pid :: pid(), heap_size) -> {heap_size, non_neg_integer()} | undefined;
+    (Pid :: pid(), total_heap_size) -> {total_heap_size, non_neg_integer()} | undefined;
+    (Pid :: pid(), registered_name) -> {registered_name, term()} | [] | undefined;
+    (Pid :: pid(), stack_size) -> {stack_size, non_neg_integer()} | undefined;
+    (Pid :: pid(), message_queue_len) -> {message_queue_len, non_neg_integer()} | undefined;
+    (Pid :: pid(), memory) -> {memory, non_neg_integer()} | undefined;
+    (Pid :: pid(), links) -> {links, [pid()]} | undefined;
+    (Pid :: pid(), monitored_by) -> {monitored_by, [pid() | resource() | port()]} | undefined;
+    (Pid :: pid(), trap_exit) -> {trap_exit, boolean()} | undefined;
     (Pid :: pid(), [atom()]) -> [{atom(), term()}] | undefined.

Additional suggested fixes

Guard the mailbox request's flexible-array allocation

The sizeof(header) + len * sizeof(term) expression is unchecked in src/libAtomVM/mailbox.c:315-316. Guard it before allocating:

diff --git a/src/libAtomVM/mailbox.c b/src/libAtomVM/mailbox.c
--- a/src/libAtomVM/mailbox.c
+++ b/src/libAtomVM/mailbox.c
@@
 bool mailbox_send_process_info_request_signal(
     Context *c, int32_t sender_pid, process_info_mode_t mode, const term *atoms, size_t len)
 {
+    if (UNLIKELY(len > (SIZE_MAX - sizeof(struct ProcessInfoRequestSignal)) / sizeof(term))) {
+        return false;
+    }
+
     struct ProcessInfoRequestSignal *signal = malloc(
         sizeof(struct ProcessInfoRequestSignal) + len * sizeof(term));

This can reuse the existing false/out_of_memory path and is independent of the larger guaranteed-response change in finding 1.

Make test-process cleanup unconditional

The with_other_pid/1 helper only cleans up after Fun(Pid) returns successfully. If an assertion fails, the child and its monitor message can survive into subsequent test cases. Use try ... after and a bounded wait for the monitored child:

diff --git a/tests/erlang_tests/test_process_info.erl b/tests/erlang_tests/test_process_info.erl
--- a/tests/erlang_tests/test_process_info.erl
+++ b/tests/erlang_tests/test_process_info.erl
@@
-    Fun(Pid),
-    Pid ! quit,
-    normal =
-        receive
-            {'DOWN', Ref, process, Pid, Reason} -> Reason
-        end.
+    try
+        Fun(Pid)
+    after
+        Pid ! quit,
+        normal =
+            receive
+                {'DOWN', Ref, process, Pid, Reason} -> Reason
+            after 1000 ->
+                erlang:error(timeout_waiting_for_child_exit)
+            end
+    end.

Apply the same fixture discipline to the estdlib process-info test where practical.

What was checked

  • Full diff and commit sequence for HEAD~13..HEAD, with surrounding process table, mailbox ownership/disposal, scheduler, interpreter, JIT, NIF, and estdlib code.
  • Request validation, external/dead PID handling, teardown locking, list order/duplicates, registered_name, and signal exclusion from message_queue_len.
  • Independent Oracle review, followed by validation of each reported issue against the current source and the pre-range implementation.
  • git diff --check HEAD~13..HEAD — clean.
  • Current AtomVM, test_estdlib, and tests/erlang_tests/test_process_info.beam targets build successfully.
  • build/src/AtomVM build/tests/erlang_tests/test_process_info.beam build/libs/atomvmlib.avm build/libs/estdlib/src/estdlib.avm — returns 0.
  • Rebuilt build/tests/libs/estdlib/test_estdlib.avm — full suite returns ok, including {test_process_info,ok}.

Allocator-fault and 32-bit overflow tests were not available, so findings 1–3 are source-verified rather than dynamically reproduced.

bettio added 17 commits July 19, 2026 16:53
The with_other_pid(Check) assertion was commented out in the original
submission with no recorded reason; enabled it passes on both AtomVM
and OTP (50/50 repeat runs) and it ran enabled with green CI in the
FissionVM fork's merged copy of this feature.

Temporary fixup commit: to be squashed into the process_info commit.

Signed-off-by: Davide Bettio <davide@uninstall.it>
process_info/1 is by OTP definition one process_info/2 list call plus
a presentation rule, so implement it in Erlang (maintainer preference:
Erlang over C when OTP-compliant semantics allow). This deletes from C
the default_items table, the argc == 1 branch, the third signal mode
PROCESS_INFO_LIST_OMIT_UNREGISTERED and both registered_name filter
sites; the signal protocol keeps a clean single/list request. The /1
default item set (OTP's default set intersected with AtomVM-supported
items, in OTP's relative order) moves to erlang.erl, where extending
it no longer requires a VM release.

The erlang_tests runner resolves modules from its own directory and
does not load atomvmlib, so the built estdlib erlang.beam (and its
JIT-precompiled variant) is now copied next to the test beams;
otherwise process_info/1 cannot be resolved by the test suite.

Behaviorally verified equivalent on all probes, including cross-process
/1, death races and 2000-item lists (found-issues.md section E).

Note: inside the module named erlang, guard BIFs must be written
qualified (erlang:is_pid/1), as unqualified calls resolve locally.

Temporary fixup commit: to be squashed into the process_info commit.

Signed-off-by: Davide Bettio <davide@uninstall.it>
term_is_pid also accepts external pids, whose boxed term was fed to
term_to_local_process_id: an arbitrary process table lookup in release
builds and an assert abort in debug builds. Match OTP behavior instead
(verified on OTP 29): badarg for another node's pid, undefined for an
external pid naming this node (a previous incarnation, so the process
cannot be alive). Local pids are validated with VALIDATE_VALUE as
process_flag/3 already does.

Temporary fixup commit: to be squashed into the process_info commit.

Signed-off-by: Davide Bettio <davide@uninstall.it>
OTP validates the info request argument before considering whether the
target process is alive, so a dead pid combined with an invalid item,
an invalid list element or an improper list is a badarg, not undefined
(five empirically-diffed divergences vs OTP 29, one of them a
regression vs the old AtomVM code, which checked term_is_atom before
the process lookup). Item validity is probed with
context_get_process_info(ctx, NULL, NULL, item, NULL), which returns
pure membership and cannot drift from the supported item set. The list
walk also counts the length, so the later copy loop no longer
revalidates. The stale own-node external pid undefined answer moves
after the argument validation for the same reason.

Temporary fixup commit: to be squashed into the process_info commit.

Signed-off-by: Davide Bettio <davide@uninstall.it>
The custom allocations-without-ensure-free CodeQL query (severity
error) flags every context_get_process_info call in a NIF that is
reachable before a memory_ensure_free barrier; the PR deleted the
existing NOLINT suppression and the real CI raised two alerts on the
size-pass call sites. Re-add the suppressions on the two size-pass
sites and on the two new argument-validity probes, which the query
reaches the same way. All four calls pass a NULL heap and cannot
allocate.

Temporary fixup commit: to be squashed into the process_info commit.

Signed-off-by: Davide Bettio <davide@uninstall.it>
mailbox_send_process_info_request_signal returned void and silently
dropped the request on malloc failure, while the NIF unconditionally
set the Trap flag afterwards, leaving the caller waiting forever for
an answer that would never come. The allocation is now caller-sized
(list length), so this is easier to hit on memory-constrained targets.
Return bool from the send function and raise out_of_memory
synchronously instead of trapping; the list path also frees the items
array on that branch.

Temporary fixup commit: to be squashed into the process_info commit.

Signed-off-by: Davide Bettio <davide@uninstall.it>
The get_item/2 helper asserted that the second process_info read
returns exactly the value bound by the first, which for heap_size,
total_heap_size and memory holds only while no GC lands between the
two calls: on AtomVM each call runs memory_ensure_free_opt with
MEMORY_CAN_SHRINK and on BEAM the first result allocation can trigger
a minor GC, so the suite would pass almost always and then flake in
CI. Volatile metrics now assert shape and invariants on both reads
instead; exact double-read equality stays for stable items. The
heap_size =< total_heap_size invariant is taken from one list request,
which is an atomic snapshot.

Temporary fixup commit: to be squashed into the process_info commit.

Signed-off-by: Davide Bettio <davide@uninstall.it>
The old test asserted that the memory item grows when messages are
queued (strictly on AtomVM, non-strictly on BEAM); the rewritten
test_memory only checked is_integer, dropping the only functional
check that memory responds to actual usage. Restore it inside the
message_queue_len choreography, which already queues messages against
a blocked helper process. Also add alive-cross-process invalid
argument coverage to test_badargs: previously only self and dead pids
were exercised, so error delivery for another live process was
untested.

Temporary fixup commit: to be squashed into the process_info commit.

Signed-off-by: Davide Bettio <davide@uninstall.it>
Give process_info/1 its own edoc block (it previously sat under the
process_info/2 doc comment), documenting the default key subset and
the registered_name omission rule; extend the process_info/2 spec
with the trap_exit item and the list-of-keys clause, and document
both in the key list. Add a CHANGELOG note for the behavior change
that ports are no longer accepted as process pids.

Temporary fixup commit: to be squashed into the process_info commit.

Signed-off-by: Davide Bettio <davide@uninstall.it>
A ProcessInfoRequestSignal processed from context_destroy was answered
with a mid-destruction snapshot: at that point the process has already
been removed from the process table and unregistered, but links and
monitors are still attached, so the requester could observe a cleared
registered_name for a process that is no longer lookupable. Erlang/OTP
answers undefined for an exiting/released process.

Reply undefined instead of gathering info when the request is processed
from the destroy path, which is uniquely identified by
process_table_locked: context_destroy is the only caller that holds the
process table write lock. Requests sent after the requester observed a
DOWN or an exit already returned undefined from the process table
lookup, so this only changes the answer for requests racing with the
teardown itself.

Verified with a 2000-iteration spawn/die race in both single-item and
list mode: no hangs, answers are either complete results or undefined.

Signed-off-by: Davide Bettio <davide@uninstall.it>
context_message_queue_len used mailbox_len, which counts every node of
both mailbox lists: unprocessed system signals sitting in the outer
list (monitor and demonitor requests, unlink and group leader signals,
process_info requests, ...) were reported as queued messages. A process
whose true message queue was always empty could observe
message_queue_len values in the thousands under signal load, while
Erlang/OTP counts only ordinary messages; queue-length based logic such
as load shedding or the proc_lib crash report code can act on wildly
wrong values.

Add mailbox_normal_message_len, which filters the outer list on
NormalMessage exactly like mailbox_size already does (the inner list
only holds normal messages), and use it for message_queue_len. The new
monitor/demonitor flood test fails on the previous code within a few
samples and must always report zero with the fix, on AtomVM and BEAM
alike.

The bug predates the process_info list support; it is fixed here
because process_info/1 now bundles message_queue_len into every result,
widening the exposure.

Signed-off-by: Davide Bettio <davide@uninstall.it>
tests/erlang_tests runs against the bare VM with no library archive,
by design; functions implemented in estdlib belong in tests/libs/
estdlib, which is exactly how the estdlib-implemented apply/2,3 are
already covered there. Move the process_info/1 tests (default item
set, cross-process, dead pid undefined, non-pid and foreign external
pid badarg) into a new tests/libs/estdlib/test_process_info.erl - run
on AtomVM via test_estdlib.avm and on BEAM by the estdlib-with-BEAM CI
step - and revert the copying of the built estdlib erlang.beam into
the erlang_tests runner directory, which worked against that design.
The erlang_tests module keeps every native process_info/2 path; the
new estdlib module additionally checks that a registered process
reports registered_name first.

Temporary fixup commit: to be squashed into the process_info commit.

Signed-off-by: Davide Bettio <davide@uninstall.it>
nif_erlang_process_info answers [] before sending the signal, so a
PROCESS_INFO_LIST request never carries an empty item list. Assert the
two invariants the handler relies on instead of handling a length that
cannot reach it.

Signed-off-by: Davide Bettio <davide@uninstall.it>
ssize_t is POSIX and neither context.c nor nifs.c includes sys/types.h:
it only resolves through transitive includes today. A size_t countdown
loop needs no such type.

Signed-off-by: Davide Bettio <davide@uninstall.it>
A request may repeat a dynamically sized item such as links or
monitored_by, so the accumulated size is the request length times the
per-item size and is not bounded by the item list itself. Allocating
the heap multiplies that sum by sizeof(term), so the bound divides
SIZE_MAX by it rather than comparing against SIZE_MAX.

Cover the accumulation with a long repeated request, self and
cross-process.

Signed-off-by: Davide Bettio <davide@uninstall.it>
BuiltInAtomSignal was renamed to ImmediateSignal in a639cbc and the
page was never updated, so the directive resolved to nothing. Restore
the alphabetical order the process_info rename broke while here.

Signed-off-by: Davide Bettio <davide@uninstall.it>
AtomsHashTable was removed in 2ab1acb ("Use valueshashtable for
modules and remove atomshashtable"), but the libAtomVM API
documentation still listed it, leaving a directive that resolves to
nothing: a doxygen run over src/libAtomVM emits no such compound.

Unrelated to process_info: found while fixing the neighbouring stale
BuiltInAtomSignal entry pointed out in review.

Signed-off-by: Davide Bettio <davide@uninstall.it>
@bettio
bettio force-pushed the updated-nif-erlang-process_info branch from ce3e299 to 712dd87 Compare July 19, 2026 15:00
@petermm

petermm commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

AMP keeps insisting on these, your call..

Follow-up review: process_info/1,2 fixes

Head verified: 712dd8776
Fix range: 9900c9a0f..712dd8776

Recommendation: Changes still requested. The contributor fixed the list-size accumulation, portable reverse iteration, and single-item specifications. Reply delivery under allocation failure and asynchronous trap-exception handling remain correctness blockers.

Resolved findings

List result-size accumulation is now bounded

Both the self and cross-process paths now check each item and the running total against MEMORY_HEAP_MAX_TERMS before addition (src/libAtomVM/context.c:363-372, src/libAtomVM/nifs.c:3330-3348). This prevents total_size wrapping and makes the immediate memory_init_heap(total_size) multiplication safe.

The new 100-item self/remote test exercises repeated-key result construction and ordering. It is not an allocation-boundary test, but it provides useful coverage of the changed loops.

Reverse list construction is portable

Both loops now use:

for (size_t i = len; i-- > 0;)

This removes the dependency on POSIX ssize_t in core code and avoids implementation-defined conversion from a large size_t.

Single-item specs now include undefined

Every single-item process_info/2 overload in libs/estdlib/src/erlang.erl:366-376 now includes | undefined, matching the dead-process behavior and tests.

Remaining findings

1. High — Reply allocation can crash the VM or leave the requester trapped forever

An accepted cross-process request sets Trap, so it must receive exactly one completion. That guarantee is still absent:

  • mailbox_message_create_from_term() returns NULL on allocation failure (src/libAtomVM/mailbox.c:255-264).
  • mailbox_send_term_signal() passes that pointer directly to mailbox_post_message() (mailbox.c:293-297), whose enqueue paths dereference it. A failed reply allocation therefore causes a native null-pointer crash.
  • mailbox_send_immediate_signal() silently returns when its allocation fails (mailbox.c:299-305). The exception reply is lost and the requester remains trapped indefinitely.

The new list-size check does not close this path. A cross-process result is first built on a temporary heap and then copied into a separately allocated TermSignal; that second allocation still has unchecked size arithmetic and unchecked failure handling.

Required direction: reserve a no-allocation fallback completion before posting the request. One contained design is to allocate an ImmediateSignal carrying OUT_OF_MEMORY_ATOM with the ProcessInfoRequestSignal, transfer it to the requester if normal reply construction or serialization fails, and free it after successful completion or if the requester has died.

Merely making mailbox_send_term_signal() return false is insufficient: once the requester is trapped, the failure still needs a guaranteed completion path. For that reason, no small patch is suggested for this finding.

Add allocator-fault tests for:

  1. TrapAnswerSignal allocation failure;
  2. TrapExceptionSignal allocation failure;
  3. teardown's undefined response allocation failure.

2. Medium — TrapExceptionSignal still leaves Trap set

Normal answers clear Trap through context_process_signal_trap_answer(). Exception answers only install the exception:

  • Interpreter: src/libAtomVM/opcodesswitch.h:824-829
  • JIT: src/libAtomVM/jit.c:919-924

If the asynchronous exception is caught, exception state is cleared but the context remains marked Trap. At the next scheduling boundary it can enter SCHEDULE_WAIT_ANY even though its one expected completion was already consumed.

Clear Trap before raising the asynchronous exception in both engines:

diff --git a/src/libAtomVM/opcodesswitch.h b/src/libAtomVM/opcodesswitch.h
--- a/src/libAtomVM/opcodesswitch.h
+++ b/src/libAtomVM/opcodesswitch.h
@@
                 case TrapExceptionSignal: {                                                     \
                     struct ImmediateSignal *trap_exception                                      \
                         = CONTAINER_OF(signal_message, struct ImmediateSignal, base);           \
+                    context_update_flags(ctx, ~Trap, NoFlags);                                  \
                     SET_ERROR(trap_exception->immediate);                                       \
                     handle_error = true;                                                        \
                     break;                                                                      \
diff --git a/src/libAtomVM/jit.c b/src/libAtomVM/jit.c
--- a/src/libAtomVM/jit.c
+++ b/src/libAtomVM/jit.c
@@
             case TrapExceptionSignal: {
                 struct ImmediateSignal *trap_exception
                     = CONTAINER_OF(signal_message, struct ImmediateSignal, base);
+                context_update_flags(ctx, ~Trap, NoFlags);
                 set_error(ctx, jit_state, 0, trap_exception->immediate);
                 handle_error = true;
                 break;

Add a regression that fault-injects or directly delivers a trap exception, catches it, and proves the process continues executing.

Additional small suggestions

Guard the request signal's flexible-array allocation

The mailbox helper still evaluates sizeof(header) + len * sizeof(term) without checking for overflow (src/libAtomVM/mailbox.c:312-326). Guard the allocation at its API boundary and reuse the existing false/out_of_memory caller path:

diff --git a/src/libAtomVM/mailbox.c b/src/libAtomVM/mailbox.c
--- a/src/libAtomVM/mailbox.c
+++ b/src/libAtomVM/mailbox.c
@@
 bool mailbox_send_process_info_request_signal(
     Context *c, int32_t sender_pid, process_info_mode_t mode, const term *atoms, size_t len)
 {
+    if (UNLIKELY(len > (SIZE_MAX - sizeof(struct ProcessInfoRequestSignal)) / sizeof(term))) {
+        return false;
+    }
+
     struct ProcessInfoRequestSignal *signal = malloc(
         sizeof(struct ProcessInfoRequestSignal) + len * sizeof(term));

Make child cleanup unconditional in process-info tests

Both with_other_pid/1 helpers only clean up after Fun(Pid) succeeds. An assertion failure can leave the child and its monitor message alive. Use try ... after and a bounded wait in tests/erlang_tests/test_process_info.erl and tests/libs/estdlib/test_process_info.erl:

diff --git a/tests/erlang_tests/test_process_info.erl b/tests/erlang_tests/test_process_info.erl
--- a/tests/erlang_tests/test_process_info.erl
+++ b/tests/erlang_tests/test_process_info.erl
@@
-    Fun(Pid),
-    Pid ! quit,
-    normal =
-        receive
-            {'DOWN', Ref, process, Pid, Reason} -> Reason
-        end.
+    try
+        Fun(Pid)
+    after
+        Pid ! quit,
+        normal =
+            receive
+                {'DOWN', Ref, process, Pid, Reason} -> Reason
+            after 1000 ->
+                erlang:error(timeout_waiting_for_child_exit)
+            end
+    end.

Apply the same hunk to the estdlib helper.

Verification

  • git diff --check 9900c9a0f..712dd8776 — clean.
  • AtomVM, test_estdlib, and tests/erlang_tests/test_process_info.beam rebuilt successfully.
  • Focused native test_process_info — returned 0.
  • Full estdlib suite — returned ok, including {test_process_info,ok}.
  • Independent Oracle follow-up review confirmed which original findings are resolved and which remain.
  • Sphinx generation reached the estdlib documentation, then failed in unrelated alisp edoc generation because edoc scanned an erlfmt plugin source containing an unsupported declaration. No failure was observed in the changed data-structure documentation.

No ordinary-input behavioral regression was found in the pushed fixes. Allocator-fault and allocation-boundary behavior remains source-verified rather than dynamically reproduced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants