From 2a43c6dae99bc947930f36710aff8f8e3e3d16a5 Mon Sep 17 00:00:00 2001 From: Marco Casaroli Date: Fri, 7 Aug 2026 13:03:08 +0200 Subject: [PATCH] testing/ostest: Split the fork test into vfork and fork. ostest's "vfork" test was never testing vfork(). It has the child write a global and the parent observe the write -- the defining property of *sharing*, not of vfork(), whose defining property is that the parent is suspended and whose contract forbids the child to write anything at all. It passed because NuttX implemented fork() and vfork() as the same sharing primitive, which apache/nuttx#19562 separates. vfork.c is rewritten to test what vfork() promises. The child does only what POSIX permits -- it calls _exit(42) and nothing else, not even exit(), which would run atexit handlers and flush stdio in the parent's address space. Since the child may not write memory and the parent cannot run while the child lives, the observable is the child's exit status: had the parent not been suspended, it would have reached waitpid() while the child was still alive. Where child status is not retained -- ostest_main() sets SA_NOCLDWAIT for the whole run, deliberately -- ECHILD is accepted as equally good evidence, since it says the child was already gone when the parent asked. fork.c is new and tests POSIX fork(): the child's writes to .data, .bss and the heap are invisible to the parent and vice versa, a pointer to a stack local taken before the fork names the same object in both, and the child does everything a vfork() child may not -- calls malloc() and printf(), and returns from the function that called fork(). Both run at the top of user_main(). They exercise the lowest-level machinery in the suite -- address environments, stack setup, the architecture's register context -- so a fault in one takes the process down instead of reporting a failure. Learning that in seconds rather than after everything else has passed matters when a port is being brought up. Each test gates on the one primitive it tests, ARCH_HAVE_VFORK and ARCH_HAVE_FORK respectively. There is no compatibility layer and no mapping between symbols. vfork.c no longer requires SCHED_WAITPID: the suspension is in the kernel primitive now, so the test's core assertion holds without it and only the status check is conditional. The other in-tree callers are audited for which primitive they actually meant: * interpreters/python's _posixsubprocess and netutils/libwebsockets' LWS_HAVE_WORKING_VFORK want the fork-then-exec path -- ARCH_HAVE_VFORK. * python's os.fork() and libwebsockets' LWS_HAVE_FORK mean real fork() and stay on ARCH_HAVE_FORK, so they become *absent* rather than silently wrong. * testing/fs/fdsantest's vfork case follows ARCH_HAVE_VFORK. interpreters/bas is deliberately left alone. Its SHELL and EDIT statements reach for vfork() under an ARCH_HAVE_FORK guard and want the same treatment, but checkpatch.sh checks the whole of any file a patch touches and bas_statement.c produces 1681 pre-existing findings against master, so a one-line change there fails CI on its own. The consequence is small: EXAMPLES_BAS_SHELL is EXPERIMENTAL and already depends on ARCH_HAVE_FORK, so it becomes unselectable rather than misbehaving. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Marco Casaroli --- interpreters/python/Makefile | 10 +- netutils/libwebsockets/lws_config_private.h | 7 + testing/fs/fdsantest/fdsantest_simple.c | 4 + testing/ltp/Makefile | 2 +- testing/ostest/CMakeLists.txt | 8 +- testing/ostest/Makefile | 8 +- testing/ostest/fork.c | 276 ++++++++++++++++++++ testing/ostest/ostest.h | 8 +- testing/ostest/ostest_main.c | 23 +- testing/ostest/vfork.c | 66 +++-- 10 files changed, 377 insertions(+), 35 deletions(-) create mode 100644 testing/ostest/fork.c diff --git a/interpreters/python/Makefile b/interpreters/python/Makefile index bac4013f886..8909586cc30 100644 --- a/interpreters/python/Makefile +++ b/interpreters/python/Makefile @@ -114,6 +114,11 @@ ifeq ($(CONFIG_ARCH_HAVE_FORK),y) else @echo "export ac_cv_func_fork=\"no\"" >> $@ endif +ifneq ($(CONFIG_ARCH_HAVE_VFORK),) + @echo "export ac_cv_func_vfork=\"yes\"" >> $@ +else + @echo "export ac_cv_func_vfork=\"no\"" >> $@ +endif ifeq ($(CONFIG_SYSTEM_SYSTEM),y) @echo "export ac_cv_func_system=\"yes\"" >> $@ else @@ -135,7 +140,10 @@ endif $(SETUP_LOCAL): $(Q) ( cp $(SETUP_LOCAL).in $(SETUP_LOCAL)) -ifneq ($(CONFIG_ARCH_HAVE_FORK),y) +# _posixsubprocess is the fork-then-exec path, so vfork() is enough for it; +# os.fork() itself needs a real fork() and is governed by ac_cv_func_fork +# above. +ifeq ($(CONFIG_ARCH_HAVE_FORK)$(CONFIG_ARCH_HAVE_VFORK),) @echo "_posixsubprocess" >> $@ endif ifneq ($(CONFIG_LIBC_DLFCN),y) diff --git a/netutils/libwebsockets/lws_config_private.h b/netutils/libwebsockets/lws_config_private.h index 8f5170faff9..057baf73bb6 100644 --- a/netutils/libwebsockets/lws_config_private.h +++ b/netutils/libwebsockets/lws_config_private.h @@ -43,7 +43,10 @@ /* #undef USE_CYASSL */ /* Define to 1 if you have the `fork' function. */ + +#ifdef CONFIG_ARCH_HAVE_FORK #define LWS_HAVE_FORK +#endif #ifndef CONFIG_DISABLE_ENVIRON /* Define to 1 if you have the `getenv' function. */ @@ -111,10 +114,14 @@ /* #undef LWS_HAVE_VFORK_H */ /* Define to 1 if `fork' works. */ +#ifdef CONFIG_ARCH_HAVE_FORK #define LWS_HAVE_WORKING_FORK +#endif /* Define to 1 if `vfork' works. */ +#ifdef CONFIG_ARCH_HAVE_VFORK #define LWS_HAVE_WORKING_VFORK +#endif /* Define to 1 if execvpe() exists */ #define LWS_HAVE_EXECVPE diff --git a/testing/fs/fdsantest/fdsantest_simple.c b/testing/fs/fdsantest/fdsantest_simple.c index 128e2878bff..114f0aeeb82 100644 --- a/testing/fs/fdsantest/fdsantest_simple.c +++ b/testing/fs/fdsantest/fdsantest_simple.c @@ -96,6 +96,7 @@ static void test_case_overflow(void **state) assert_int_equal(open_count, close_count); } +#ifdef CONFIG_ARCH_HAVE_VFORK static void test_case_vfork(void **state) { int fd = open("/dev/null", O_RDONLY); @@ -112,6 +113,7 @@ static void test_case_vfork(void **state) android_fdsan_close_with_tag(fd, 0xbadc0de); } +#endif /**************************************************************************** * Public Functions @@ -129,7 +131,9 @@ int main(int argc, FAR char *argv[]) cmocka_unit_test(test_case_unowned_tagged_close), cmocka_unit_test(test_case_owned_tagged_close), cmocka_unit_test(test_case_overflow), +#ifdef CONFIG_ARCH_HAVE_VFORK cmocka_unit_test(test_case_vfork), +#endif }; return cmocka_run_group_tests(tests, NULL, NULL); diff --git a/testing/ltp/Makefile b/testing/ltp/Makefile index cee750cd0a1..9a2513336d6 100644 --- a/testing/ltp/Makefile +++ b/testing/ltp/Makefile @@ -45,7 +45,7 @@ BLACKWORDS += "pthread_spin_trylock" endif # Where NuttX does not declare fork(), a test that calls it cannot be built. -# The pattern spares vfork() and task_fork(), which remain available. +# The pattern spares vfork(), and any identifier that ends in _fork(). ifeq ($(CONFIG_ARCH_HAVE_FORK),) BLACKWORDS += "[^v_]fork(" diff --git a/testing/ostest/CMakeLists.txt b/testing/ostest/CMakeLists.txt index a76b2447c00..8eb6516790e 100644 --- a/testing/ostest/CMakeLists.txt +++ b/testing/ostest/CMakeLists.txt @@ -143,10 +143,12 @@ if(CONFIG_TESTING_OSTEST) endif() endif() + if(CONFIG_ARCH_HAVE_VFORK) + list(APPEND SRCS vfork.c) + endif() + if(CONFIG_ARCH_HAVE_FORK) - if(CONFIG_SCHED_WAITPID) - list(APPEND SRCS vfork.c) - endif() + list(APPEND SRCS fork.c) endif() if(CONFIG_ARCH_SETJMP_H) diff --git a/testing/ostest/Makefile b/testing/ostest/Makefile index 4919a6204f9..c89b2466f4f 100644 --- a/testing/ostest/Makefile +++ b/testing/ostest/Makefile @@ -144,10 +144,14 @@ CSRCS += sigev_thread.c endif endif -ifeq ($(CONFIG_ARCH_HAVE_FORK),y) -ifeq ($(CONFIG_SCHED_WAITPID),y) +# Each test is built where the primitive it tests exists. + +ifeq ($(CONFIG_ARCH_HAVE_VFORK),y) CSRCS += vfork.c endif + +ifeq ($(CONFIG_ARCH_HAVE_FORK),y) +CSRCS += fork.c endif ifeq ($(CONFIG_ARCH_SETJMP_H),y) diff --git a/testing/ostest/fork.c b/testing/ostest/fork.c new file mode 100644 index 00000000000..9d2c3ae9137 --- /dev/null +++ b/testing/ostest/fork.c @@ -0,0 +1,276 @@ +/**************************************************************************** + * apps/testing/ostest/fork.c + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you 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. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "ostest.h" + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define FORK_HEAPSIZE 256 +#define FORK_PARENTMARK 0x5a +#define FORK_CHILDMARK 0xa5 + +/* Distinct values written through a pointer to a stack local, to check that + * the child's stack is at the same virtual address as the parent's. + */ + +#define FORK_STACKPARENT 0x1234 +#define FORK_STACKCHILD 0x5678 + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +/* .data and .bss, which a fork() child must get its own copy of */ + +static volatile int g_forkdata = 1; +static volatile int g_forkbss; +static FAR unsigned char *g_forkheap; + +/* Pointer to a local in fork_test()'s frame, taken before fork(). In .data + * rather than on the stack: the compiler could rematerialise a local + * pointer from the current stack pointer, which would test nothing. + */ + +static FAR volatile int *g_forkstackptr; + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: fork_child + * + * Description: + * Everything here is forbidden to a vfork() child and permitted to a + * fork() child. Returns the child's exit status. + * + ****************************************************************************/ + +static int fork_child(void) +{ + FAR char *scratch; + + /* The parent must see none of this. */ + + g_forkdata = 2; + g_forkbss = 2; + memset(g_forkheap, FORK_CHILDMARK, FORK_HEAPSIZE); + + /* A vfork() child may not call these; a fork() child may. */ + + scratch = malloc(64); + if (scratch == NULL) + { + printf("fork_test: ERROR Child could not malloc()\n"); + return 1; + } + + strlcpy(scratch, "child", 64); + printf("fork_test: Child running independently (%s)\n", scratch); + free(scratch); + + /* Give the parent time to make its own writes, so that if the two shared + * memory we would see the parent's values below rather than our own. + */ + + usleep(200 * 1000); + + if (g_forkdata != 2 || g_forkbss != 2) + { + printf("fork_test: ERROR Child saw the parent's writes: " + "data=%d bss=%d\n", g_forkdata, g_forkbss); + return 1; + } + + if (g_forkheap[0] != FORK_CHILDMARK || + g_forkheap[FORK_HEAPSIZE - 1] != FORK_CHILDMARK) + { + printf("fork_test: ERROR Child saw the parent's heap writes\n"); + return 1; + } + + return 0; +} + +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: fork_test + * + * Description: + * Verify the defining property of POSIX fork(): the child gets its own + * copy of the parent's memory, in both directions. That is the exact + * opposite of the sharing vfork() gives. Also verifies that none of + * vfork()'s restrictions apply. + * + ****************************************************************************/ + +int fork_test(void) +{ + pid_t pid; + int status = 0; + int ret = 0; + + /* volatile so it is really in memory and the compare is not folded. */ + + volatile int stackvar = FORK_STACKPARENT; + + printf("fork_test: Started\n"); + + /* Publish its address before forking. See g_forkstackptr. */ + + g_forkstackptr = &stackvar; + + g_forkdata = 1; + g_forkbss = 1; + + g_forkheap = malloc(FORK_HEAPSIZE); + if (g_forkheap == NULL) + { + printf("fork_test: ERROR Failed to allocate the heap probe\n"); + ASSERT(false); + return -1; + } + + memset(g_forkheap, FORK_PARENTMARK, FORK_HEAPSIZE); + + pid = fork(); + if (pid == 0) + { + /* The child's stack is at the parent's virtual addresses, so this + * pointer names the child's own live local. A relocated stack would + * write into the copy of the parent's instead. + */ + + *g_forkstackptr = FORK_STACKCHILD; + if (stackvar != FORK_STACKCHILD) + { + printf("fork_test: ERROR Child stack was relocated: wrote %d " + "through %p, local at %p reads %d\n", + FORK_STACKCHILD, g_forkstackptr, &stackvar, stackvar); + _exit(1); + } + + /* Returns from fork_child() and from this branch -- both illegal + * for a vfork() child. + */ + + _exit(fork_child()); + } + else if (pid < 0) + { + printf("fork_test: ERROR fork() failed: %d\n", errno); + free(g_forkheap); + ASSERT(false); + return -1; + } + + /* Parent runs concurrently: write now, check isolation afterwards. */ + + g_forkdata = 3; + g_forkbss = 3; + memset(g_forkheap, FORK_PARENTMARK, FORK_HEAPSIZE); + +#ifdef CONFIG_SCHED_WAITPID + /* Wait for the child to be done before comparing memory. waitpid() blocks + * on a child that is still alive whether or not its exit status will be + * retained, so this synchronises either way; ECHILD simply means the child + * had already finished, which is just as good. It is not a failure: + * ostest_main() sets SA_NOCLDWAIT on SIGCHLD for the whole run, so an + * exited child's status is not kept even where CONFIG_SCHED_CHILD_STATUS + * is enabled. + */ + + if (waitpid(pid, &status, 0) != pid && errno != ECHILD) + { + printf("fork_test: ERROR waitpid() failed: %d\n", errno); + free(g_forkheap); + ASSERT(false); + return -1; + } +#else + sleep(1); +#endif + + if (g_forkdata != 3 || g_forkbss != 3) + { + printf("fork_test: ERROR Parent saw the child's writes: " + "data=%d bss=%d (expected 3, 3)\n", g_forkdata, g_forkbss); + ret = -1; + } + + if (g_forkheap[0] != FORK_PARENTMARK || + g_forkheap[FORK_HEAPSIZE - 1] != FORK_PARENTMARK) + { + printf("fork_test: ERROR Parent saw the child's heap writes\n"); + ret = -1; + } + + /* The child wrote to the same stack address; the parent must not see it */ + + if (stackvar != FORK_STACKPARENT) + { + printf("fork_test: ERROR Parent saw the child's stack write: " + "%d (expected %d)\n", stackvar, FORK_STACKPARENT); + ret = -1; + } + +#ifdef CONFIG_SCHED_WAITPID + if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) + { + printf("fork_test: ERROR Child reported failure, status 0x%04x\n", + status); + ret = -1; + } +#endif + + free(g_forkheap); + g_forkheap = NULL; + + if (ret < 0) + { + ASSERT(false); + return ret; + } + + printf("fork_test: Parent and child had independent memory\n"); + return 0; +} diff --git a/testing/ostest/ostest.h b/testing/ostest/ostest.h index 2be54de5a68..8fa6d64d0a9 100644 --- a/testing/ostest/ostest.h +++ b/testing/ostest/ostest.h @@ -284,10 +284,16 @@ void sched_lock_test(void); /* vfork.c ******************************************************************/ -#if defined(CONFIG_ARCH_HAVE_FORK) && defined(CONFIG_SCHED_WAITPID) +#ifdef CONFIG_ARCH_HAVE_VFORK int vfork_test(void); #endif +/* fork.c *******************************************************************/ + +#ifdef CONFIG_ARCH_HAVE_FORK +int fork_test(void); +#endif + /* setjmp.c *****************************************************************/ void setjmp_test(void); diff --git a/testing/ostest/ostest_main.c b/testing/ostest/ostest_main.c index 964771d907c..77ebe95acc7 100644 --- a/testing/ostest/ostest_main.c +++ b/testing/ostest/ostest_main.c @@ -224,6 +224,23 @@ static int user_main(int argc, char *argv[]) g_mmbefore = mallinfo(); g_mmprevious = g_mmbefore; + /* Run the two fork primitives first. They exercise the lowest-level + * machinery in the suite, so a fault takes the process down rather than + * reporting a failure -- better to learn that in seconds. + */ + +#ifdef CONFIG_ARCH_HAVE_VFORK + printf("\nuser_main: vfork() test\n"); + vfork_test(); + check_test_memory_usage(); +#endif + +#ifdef CONFIG_ARCH_HAVE_FORK + printf("\nuser_main: fork() test\n"); + fork_test(); + check_test_memory_usage(); +#endif + printf("\nuser_main: Begin argument test\n"); printf("user_main: Started with argc=%d\n", argc); @@ -637,12 +654,6 @@ static int user_main(int argc, char *argv[]) check_test_memory_usage(); #endif -#if defined(CONFIG_ARCH_HAVE_FORK) && defined(CONFIG_SCHED_WAITPID) && \ - !defined(CONFIG_ARCH_SIM) - printf("\nuser_main: vfork() test\n"); - vfork_test(); -#endif - #if defined(CONFIG_SMP) && defined(CONFIG_BUILD_FLAT) printf("\nuser_main: smp call test\n"); smp_call_test(); diff --git a/testing/ostest/vfork.c b/testing/ostest/vfork.c index bd6cfabadb9..473b19222bd 100644 --- a/testing/ostest/vfork.c +++ b/testing/ostest/vfork.c @@ -28,39 +28,41 @@ #include #include -#include #include #include +#include #include #include "ostest.h" -#if defined(CONFIG_ARCH_HAVE_FORK) && defined(CONFIG_SCHED_WAITPID) - /**************************************************************************** - * Private Data + * Public Functions ****************************************************************************/ -static volatile bool g_vforkchild; - /**************************************************************************** - * Public Functions + * Name: vfork_test + * + * Description: + * Verify the defining property of vfork(): the parent is suspended until + * the child _exit()s or exec()s. The child does only what POSIX + * permits -- _exit(), not exit(), which would flush stdio in the parent's + * address space. Since the child may not write memory, the observable is + * its exit status: an unsuspended parent would reach waitpid() first. + * ****************************************************************************/ int vfork_test(void) { pid_t pid; - g_vforkchild = false; + printf("vfork_test: Started\n"); + pid = vfork(); if (pid == 0) { - /* There is not very much that the child is permitted to do. Perhaps - * it can just set g_vforkchild. - */ + /* The only thing a vfork() child may do is leave. */ - g_vforkchild = true; - exit(0); + _exit(42); } else if (pid < 0) { @@ -68,22 +70,44 @@ int vfork_test(void) ASSERT(false); return -1; } - else + + /* Reached only once the child has exited or exec'ed. */ + +#ifdef CONFIG_SCHED_WAITPID { - sleep(1); - if (g_vforkchild) + int status = 0; + pid_t ret; + + ret = waitpid(pid, &status, 0); + + /* Two answers are correct, and which one comes back is a property of + * the configuration: a retained status must be exit(42), and ECHILD + * is equally good evidence -- it says the child was already gone when + * we asked. ostest_main() sets SA_NOCLDWAIT for the whole run, so + * testing CONFIG_SCHED_CHILD_STATUS alone is not enough. + */ + + if (ret == pid) { - printf("vfork_test: Child %d ran successfully\n", pid); + if (!WIFEXITED(status) || WEXITSTATUS(status) != 42) + { + printf("vfork_test: ERROR Child %d status 0x%04x, expected " + "exit(42)\n", pid, status); + ASSERT(false); + return -1; + } } - else + else if (ret >= 0 || errno != ECHILD) { - printf("vfork_test: ERROR Child %d did not run\n", pid); + printf("vfork_test: ERROR waitpid() returned %d (%d), expected " + "the child's status or ECHILD\n", ret, errno); ASSERT(false); return -1; } } +#endif + printf("vfork_test: Child %d ran and exited before the parent resumed\n", + pid); return 0; } - -#endif /* CONFIG_ARCH_HAVE_FORK && CONFIG_SCHED_WAITPID */