You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "casaroli (via GitHub)" <gi...@apache.org> on 2026/10/07 17:35:10 UTC
[PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
casaroli opened a new pull request, #20492:
URL: https://github.com/apache/nuttx/pull/20492
## Summary
Each task group now has its own registry of the ELF objects that `dlopen()` loads, as suggested in #20132. The registry is in `struct task_info_s` and not in `struct tls_info_s`, because POSIX makes a `dlopen()` handle valid in the whole process, not only in the thread that opened it.
Kernel modules from `insmod()` stay on one registry for the system, and `/proc/modules` shows only them.
A library that a module names in `DT_NEEDED` goes on the registry of that module. In a flat build, `exec()` gives the libraries of the program to the new task group, so `dlopen()` in the program finds the same copy.
When a task group exits, the libraries that it did not close are closed. A flat build does this in `group_release()`, after the last thread. A protected build does it in `exit()`, because the kernel cannot reach the user registry.
## Impact
Two processes that use one library now each have their own copy of its data, and its constructors and destructors run once per process. An FDPIC library executed in place from xipfs still shares its text.
A library that is not executed in place is now loaded once per process, so its text uses RAM in each process.
A program loaded by `exec()` no longer resolves symbols from libraries that other processes loaded, or from `insmod()` modules.
`dlopen()` handles that a process does not close are now freed when it exits. Before, they stayed loaded.
The xipfs C++ test in apps checks the old shared behaviour. apache/nuttx-apps#APPS changes it.
## Testing
Arm GNU Toolchain 13.2.rel1, QEMU `mps2-an500`.
| test | master | this PR |
|---|---|---|
| `xipfs_test fdpic` (FDPIC, `mps2-an500:xipfs`) | 32 passed, 2 failed | 32 passed, 2 failed, with the apps PR |
| `fdpicxip solib` | library pinned once, totals 9 and 9 | pinned twice, totals 3 and 6, 0 pins after exit |
| `fdpicxip cxx` | one library destructor | one destructor per process |
| `sotest` (plain ELF) | same output | same output |
| `sotest` that exits with its libraries open | the library destructor never runs | it runs at exit |
| `mps2-an500:knsh` (protected) with ELF and dlfcn: `ostest` | exits 0 | exits 0 |
| `qemu-armv7a:knsh` (kernel) with dlfcn | builds | builds |
The 2 `xipfs_test` failures also occur on master. `tools/checkpatch.sh -c -u -m -g` passes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#issuecomment-6102413582
<!-- nuttx-depends-on-bot run-38088049757 -->
### ❌ Cross-repo dependency could not be applied
The Build report says the declared dependency PR(s) could not be applied, so CI did **not** run against the combined code:
- https://github.com/apache/nuttx/pull/20554
Reason: cherry-pick failed (if your PR has merge commits, rebase instead)
CI run: https://github.com/apache/nuttx/actions/runs/38088049757
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#discussion_r4250864451
##########
libs/libc/elf/elf_registry.c:
##########
@@ -46,13 +47,35 @@
* Private Data
****************************************************************************/
+/* One lock for every registry. g_mod_registry holds the kernel modules of
+ * insmod(); what dlopen() loads is on the registry of the task group.
+ */
+
static rmutex_t g_modlock = NXRMUTEX_INITIALIZER;
static FAR struct module_s *g_mod_registry;
/****************************************************************************
* Public Functions
****************************************************************************/
+/****************************************************************************
+ * Name: libelf_registry
+ ****************************************************************************/
+
+FAR struct module_s **libelf_registry(void)
+{
+ return &task_get_info()->ta_modules;
+}
+
+/****************************************************************************
+ * Name: libelf_registry_kernel
+ ****************************************************************************/
+
+FAR struct module_s **libelf_registry_kernel(void)
Review Comment:
move to module
##########
include/nuttx/lib/elf.h:
##########
@@ -175,6 +175,10 @@ struct fdpic_desc_s;
struct module_s
{
FAR struct module_s *flink; /* Supports a singly linked list */
+ FAR struct module_s **registry; /* The registry this module is on */
+ FAR struct module_s *libraries; /* exec(): its own registry, for
Review Comment:
why need add libraries/registry field? we just need a link list and hold the head in tls
##########
libs/libc/elf/elf_bind.c:
##########
@@ -1040,7 +984,8 @@ static int libelf_relocatedyn(FAR struct module_s *modp,
return ret;
}
- handle = libelf_insertneeded((FAR const char *)loadinfo->iobuffer);
+ handle = libelf_insertneeded(modp->registry,
Review Comment:
why not insert to ta_modules
##########
libs/libc/elf/elf_registry.c:
##########
@@ -46,13 +47,35 @@
* Private Data
****************************************************************************/
+/* One lock for every registry. g_mod_registry holds the kernel modules of
+ * insmod(); what dlopen() loads is on the registry of the task group.
+ */
+
static rmutex_t g_modlock = NXRMUTEX_INITIALIZER;
static FAR struct module_s *g_mod_registry;
/****************************************************************************
* Public Functions
****************************************************************************/
+/****************************************************************************
+ * Name: libelf_registry
+ ****************************************************************************/
+
+FAR struct module_s **libelf_registry(void)
Review Comment:
move to dlfnc
##########
libs/libc/elf/elf_remove.c:
##########
@@ -289,3 +290,42 @@ int libelf_remove(FAR void *handle)
set_errno(-ret);
return ERROR;
}
+
+/****************************************************************************
+ * Name: libelf_closeall
+ *
+ * Description:
+ * Give back every reference that dlopen() took in a registry. A module
+ * that another one still depends on goes with its last dependent.
+ *
+ ****************************************************************************/
+
+void libelf_closeall(FAR struct module_s **registry)
Review Comment:
libelf_removeall
##########
Documentation/os/binfmt/kernel_modules_vs_shared_libraries.rst:
##########
@@ -139,40 +139,52 @@ FLAT build.
Opening a Library More Than Once
================================
-In the FLAT and PROTECTED builds, ``dlopen()`` of a library that is already
+In the FLAT and PROTECTED builds, each task group has its own list of
+loaded libraries. ``dlopen()`` of a library that the task group already
loaded does not load a second copy and does not fail. It returns a handle to
the library that is already loaded and takes an additional reference on it.
Each successful ``dlopen()`` must be matched by a ``dlclose()``; the library
is unloaded only when the last handle is closed. Up to 255 handles may be
outstanding on one library; beyond that ``dlopen()`` fails with ``EMFILE``.
+The libraries that a task group did not close are closed when it exits.
+In the PROTECTED build only ``exit()`` closes them, so a task group that
+ends in another way leaves them loaded.
Some consequences worth keeping in mind:
* A library is identified by the *basename* of the path passed to
``dlopen()``. Two files with the same basename in different directories
are treated as the same library, and the second ``dlopen()`` will return
the first one.
-* There is only one instance of the library's ``.data`` and ``.bss``. Global
- and static data are shared by every user of the library, and by every task
- group in the system.
-* Constructors in ``.init_array`` run once, when the library is first loaded,
- and destructors in ``.fini_array`` run once, when the last handle is
- closed. They do not run per ``dlopen()``/``dlclose()`` pair.
+* Each task group that opens a library loads its own copy. Global and static
+ data are shared by the threads of one task group, not between task groups.
+ Only an FDPIC library executed in place from xipfs shares its ``.text``
+ between task groups.
+* Constructors in ``.init_array`` run once per task group, when the library
+ is first loaded in it, and destructors in ``.fini_array`` run once, when
+ its last handle in the task group is closed. They do not run per
+ ``dlopen()``/``dlclose()`` pair.
+* The libraries that a program loaded by ``exec()`` names in ``DT_NEEDED``
+ belong to its task group in the FLAT build, so ``dlopen()`` of one of them
Review Comment:
does DT_NEEDED work in protected/kernel build now?
##########
libs/libc/elf/elf_remove.c:
##########
@@ -289,3 +290,42 @@ int libelf_remove(FAR void *handle)
set_errno(-ret);
return ERROR;
}
+
+/****************************************************************************
+ * Name: libelf_closeall
+ *
+ * Description:
+ * Give back every reference that dlopen() took in a registry. A module
+ * that another one still depends on goes with its last dependent.
+ *
+ ****************************************************************************/
+
+void libelf_closeall(FAR struct module_s **registry)
+{
+ FAR struct module_s *modp;
+
+ libelf_registry_lock();
Review Comment:
should use per task lock from tls
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#issuecomment-6234535134
<!-- nuttx-depends-on-bot run-39019425152 -->
### 🔗 Cross-repo PR dependencies
The read-only Build run reported the following dependent PR(s) and fetched head SHA(s):
- https://github.com/apache/nuttx-apps/pull/3857 @ ecbb8763ed
CI run: https://github.com/apache/nuttx/actions/runs/39019425152
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis merged PR #20492:
URL: https://github.com/apache/nuttx/pull/20492
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "casaroli (via GitHub)" <gi...@apache.org>.
casaroli closed pull request #20492: libs/libc/elf: Keep a module registry per task group.
URL: https://github.com/apache/nuttx/pull/20492
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "casaroli (via GitHub)" <gi...@apache.org>.
casaroli closed pull request #20492: libs/libc/elf: Keep a module registry per task group.
URL: https://github.com/apache/nuttx/pull/20492
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#discussion_r4306065193
##########
sched/module/mod_modhandle.c:
##########
@@ -29,6 +29,8 @@
#include <nuttx/module.h>
#include <nuttx/lib/elf.h>
+#include "module/module.h"
+
#ifdef CONFIG_MODULE
Review Comment:
let's remove ALL unnecessary `#ifdef CONFIG_MODULE`
##########
sched/module/mod_procfs.c:
##########
@@ -42,6 +42,8 @@
#include <nuttx/fs/fs.h>
#include <nuttx/fs/procfs.h>
+#include "module/module.h"
+
#if !defined(CONFIG_DISABLE_MOUNTPOINT) && defined(CONFIG_FS_PROCFS) && \
!defined(CONFIG_FS_PROCFS_EXCLUDE_MODULE)
Review Comment:
remove the check
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#issuecomment-6052049508
> @xiaoxiang781216 i think this is breaking change. Should I mark it and bring a vote to the list?
the old behavior doesn't compliant to POSIX, this patch fix the problem. So, I think it's a bug fix.:)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "casaroli (via GitHub)" <gi...@apache.org>.
casaroli commented on PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#issuecomment-6045337558
@xiaoxiang781216 i think this is breaking change. Should I mark it and bring a vote to the list?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "casaroli (via GitHub)" <gi...@apache.org>.
casaroli closed pull request #20492: libs/libc/elf: Keep a module registry per task group.
URL: https://github.com/apache/nuttx/pull/20492
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#discussion_r4303412709
##########
binfmt/binfmt_loadmodule.c:
##########
@@ -184,6 +184,12 @@ int load_module(FAR struct binary_s *bin, FAR const char *filename,
if (bin && filename)
#endif
{
+#ifdef CONFIG_LIBC_ELF
+ /* The DT_NEEDED libraries of the program go here */
+
+ list_initialize(&bin->libraries);
Review Comment:
libraries may need for other format too, remove the check CONFIG_LIBC_ELF
##########
binfmt/binfmt_execmodule.c:
##########
@@ -45,6 +45,7 @@
#include <nuttx/spawn.h>
#include <nuttx/binfmt/binfmt.h>
#include <nuttx/environ.h>
+#include <nuttx/lib/elf.h>
Review Comment:
shouldn't include elf.h in the common loader code
##########
libs/libc/elf/elf_getsymbol.c:
##########
@@ -73,13 +78,12 @@ FAR const void *libelf_getsymbol(FAR void *handle, FAR const char *name)
/* Verify that the module is in the registry */
- libelf_registry_lock();
- ret = libelf_registry_verify(modp);
+ ret = libelf_registry_verify(registry, modp);
Review Comment:
remove libelf_registry_verify to avoid adding registry argument
##########
libs/libc/dlfcn/lib_dlfcn.h:
##########
@@ -0,0 +1,84 @@
+/****************************************************************************
+ * libs/libc/dlfcn/lib_dlfcn.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __LIBS_LIBC_DLFCN_LIB_DLFCN_H
+#define __LIBS_LIBC_DLFCN_LIB_DLFCN_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <errno.h>
+
+#include <nuttx/list.h>
+#include <nuttx/mutex.h>
+#include <nuttx/tls.h>
+
+#ifdef CONFIG_LIBC_ELF
+
+/****************************************************************************
+ * Inline Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: dl_registry_lock
+ *
+ * Description:
+ * Lock the registry of the calling task group, which holds what dlopen()
+ * loads, and return it. The lock is recursive: the constructors of a
+ * library run with it held, and they can call dlopen().
+ *
+ ****************************************************************************/
+
+static inline FAR struct list_node *dl_registry_lock(void)
Review Comment:
should we move to tls.h?
##########
sched/group/group_leave.c:
##########
@@ -80,6 +81,30 @@ static inline void group_release(FAR struct task_group_s *group)
nxrmutex_destroy(&group->tg_mutex);
+#if defined(CONFIG_LIBC_ELF) && defined(CONFIG_BUILD_FLAT)
+ /* Close what dlopen() left open. No thread of the group runs now, so
+ * the lock is not needed.
+ */
+
+ libelf_removeall(&group->tg_info->ta_modules);
+
+# ifdef CONFIG_BINFMT_LOADABLE
+ /* The rest is used by the program. Give it back to the program before
+ * the list goes with the task information: binfmt_exit() unloads it.
+ */
+
+ if (group->tg_bininfo != NULL)
+ {
+ FAR struct list_node *node;
+
+ while ((node = list_remove_head(&group->tg_info->ta_modules)) != NULL)
+ {
+ list_add_tail(&group->tg_bininfo->libraries, node);
Review Comment:
why need
##########
binfmt/binfmt_execmodule.c:
##########
@@ -190,6 +191,10 @@ int exec_module(FAR struct binary_s *binp,
char name[PATH_MAX];
#endif
FAR void *stackaddr = NULL;
+#if defined(CONFIG_BINFMT_LOADABLE) && defined(CONFIG_LIBC_ELF) && \
Review Comment:
why check CONFIG_LIBC_ELF in the common loader
##########
binfmt/binfmt_execmodule.c:
##########
@@ -190,6 +191,10 @@ int exec_module(FAR struct binary_s *binp,
char name[PATH_MAX];
#endif
FAR void *stackaddr = NULL;
+#if defined(CONFIG_BINFMT_LOADABLE) && defined(CONFIG_LIBC_ELF) && \
+ defined(CONFIG_BUILD_FLAT)
Review Comment:
why need check flat mode? the same code should be used for other mode too
##########
include/nuttx/lib/elf.h:
##########
@@ -776,9 +764,10 @@ FAR void *libelf_insert(FAR const char *filename, FAR const char *modname);
* called.
*
* Input Parameters:
- * handle - The opaque, non-NULL value returned by a previous successful
- * call to libelf_insert().
- * name - A pointer to the symbol name string.
+ * registry - The registry the module is on.
+ * handle - The opaque, non-NULL value returned by a previous successful
Review Comment:
why need both registry and handle?
##########
libs/libc/elf/elf_bind.c:
##########
@@ -1040,7 +1042,20 @@ static int libelf_relocatedyn(FAR struct module_s *modp,
return ret;
}
- handle = libelf_insertneeded((FAR const char *)loadinfo->iobuffer);
+ /* The library goes on the registry of the module that needs it */
+
+ if (loadinfo->registry == NULL)
Review Comment:
move the check into libelf_insertneeded or libelf_insert
##########
binfmt/elf.c:
##########
@@ -141,6 +141,12 @@ static int elf_loadbinary(FAR struct binary_s *binp,
if (loadinfo.ehdr.e_type == ET_REL || loadinfo.gotsize != 0)
{
+ /* Its DT_NEEDED libraries go on binp->libraries. The task group
+ * does not exist yet. In a flat build, exec_module() gives them to
+ * the new task group.
+ */
+
+ loadinfo.registry = &binp->libraries;
Review Comment:
how about pass ta_modules directly
##########
libs/libc/elf/elf_symbols.c:
##########
@@ -470,7 +470,9 @@ int libelf_symvalue(FAR struct module_s *modp,
exportinfo.modp = modp;
exportinfo.symbol = NULL;
- ret = libelf_registry_foreach(libelf_symcallback,
+ ret = loadinfo->registry == NULL ? OK :
Review Comment:
move all registry check into libef_reigstry_xxx
##########
include/nuttx/tls.h:
##########
@@ -173,6 +173,10 @@ struct task_info_s
size_t ta_envpc; /* Maximum entries of environment array */
size_t ta_envc; /* Number of environment strings */
#endif
+#ifdef CONFIG_LIBC_ELF
Review Comment:
check binfmt?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#discussion_r4304201390
##########
include/nuttx/tls.h:
##########
@@ -43,10 +43,15 @@
#include <nuttx/mutex.h>
#include <nuttx/queue.h>
-#ifdef CONFIG_PTHREAD_ATFORK
+#if defined(CONFIG_PTHREAD_ATFORK) || defined(CONFIG_LIBC_ELF) || \
+ defined(CONFIG_BINFMT_LOADABLE)
# include <nuttx/list.h>
#endif
+#if defined(CONFIG_LIBC_ELF) || defined(CONFIG_BINFMT_LOADABLE)
+# include <errno.h>
Review Comment:
ditto
##########
libs/libc/elf/elf_getsymbol.c:
##########
@@ -48,9 +48,10 @@
* called.
*
* Input Parameters:
- * handle - The opaque, non-NULL value returned by a previous successful
- * call to libelf_insert().
- * name - A pointer to the symbol name string.
+ * registry - The registry the module is on.
Review Comment:
revert
##########
include/nuttx/tls.h:
##########
@@ -173,6 +178,10 @@ struct task_info_s
size_t ta_envpc; /* Maximum entries of environment array */
size_t ta_envc; /* Number of environment strings */
#endif
+#if defined(CONFIG_LIBC_ELF) || defined(CONFIG_BINFMT_LOADABLE)
Review Comment:
is it enough to check CONFIG_BINFMT_LOADABLE
##########
include/nuttx/tls.h:
##########
@@ -43,10 +43,15 @@
#include <nuttx/mutex.h>
#include <nuttx/queue.h>
-#ifdef CONFIG_PTHREAD_ATFORK
+#if defined(CONFIG_PTHREAD_ATFORK) || defined(CONFIG_LIBC_ELF) || \
+ defined(CONFIG_BINFMT_LOADABLE)
# include <nuttx/list.h>
Review Comment:
let's include always
##########
libs/libc/elf/elf_getsymbol.c:
##########
@@ -62,41 +63,30 @@
* NOTE: This means that the address zero can never be a valid return
* value.
*
+ * Assumptions:
+ * The caller holds the lock of the registry.
Review Comment:
why need
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "casaroli (via GitHub)" <gi...@apache.org>.
casaroli commented on code in PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#discussion_r4263659811
##########
Documentation/os/binfmt/kernel_modules_vs_shared_libraries.rst:
##########
@@ -139,40 +139,52 @@ FLAT build.
Opening a Library More Than Once
================================
-In the FLAT and PROTECTED builds, ``dlopen()`` of a library that is already
+In the FLAT and PROTECTED builds, each task group has its own list of
+loaded libraries. ``dlopen()`` of a library that the task group already
loaded does not load a second copy and does not fail. It returns a handle to
the library that is already loaded and takes an additional reference on it.
Each successful ``dlopen()`` must be matched by a ``dlclose()``; the library
is unloaded only when the last handle is closed. Up to 255 handles may be
outstanding on one library; beyond that ``dlopen()`` fails with ``EMFILE``.
+The libraries that a task group did not close are closed when it exits.
+In the PROTECTED build only ``exit()`` closes them, so a task group that
+ends in another way leaves them loaded.
Some consequences worth keeping in mind:
* A library is identified by the *basename* of the path passed to
``dlopen()``. Two files with the same basename in different directories
are treated as the same library, and the second ``dlopen()`` will return
the first one.
-* There is only one instance of the library's ``.data`` and ``.bss``. Global
- and static data are shared by every user of the library, and by every task
- group in the system.
-* Constructors in ``.init_array`` run once, when the library is first loaded,
- and destructors in ``.fini_array`` run once, when the last handle is
- closed. They do not run per ``dlopen()``/``dlclose()`` pair.
+* Each task group that opens a library loads its own copy. Global and static
+ data are shared by the threads of one task group, not between task groups.
+ Only an FDPIC library executed in place from xipfs shares its ``.text``
+ between task groups.
+* Constructors in ``.init_array`` run once per task group, when the library
+ is first loaded in it, and destructors in ``.fini_array`` run once, when
+ its last handle in the task group is closed. They do not run per
+ ``dlopen()``/``dlclose()`` pair.
+* The libraries that a program loaded by ``exec()`` names in ``DT_NEEDED``
+ belong to its task group in the FLAT build, so ``dlopen()`` of one of them
Review Comment:
In the PROTECTED build, yes: the kernel loads the libraries of a program and keeps them on a list in its struct binary_s.
In the KERNEL build, no: with CONFIG_ARCH_ADDRENV, libelf_bind() fails a module that has DT_NEEDED (ENOSYS), because a library that libelf_insert() loads cannot reach the address space of the program.
##########
include/nuttx/lib/elf.h:
##########
@@ -175,6 +175,10 @@ struct fdpic_desc_s;
struct module_s
{
FAR struct module_s *flink; /* Supports a singly linked list */
+ FAR struct module_s **registry; /* The registry this module is on */
+ FAR struct module_s *libraries; /* exec(): its own registry, for
Review Comment:
Done: struct module_s has only a list node now, and the head is ta_modules in the task info, with its lock ta_modlock.
One field stays in mod_loadinfo_s: the registry that libelf_bind() puts the DT_NEEDED libraries on. A program that exec() loads has no task group yet, so its libraries go first on a list in struct binary_s, and exec_module() moves them to the new task group.
##########
libs/libc/elf/elf_registry.c:
##########
@@ -46,13 +47,35 @@
* Private Data
****************************************************************************/
+/* One lock for every registry. g_mod_registry holds the kernel modules of
+ * insmod(); what dlopen() loads is on the registry of the task group.
+ */
+
static rmutex_t g_modlock = NXRMUTEX_INITIALIZER;
static FAR struct module_s *g_mod_registry;
/****************************************************************************
* Public Functions
****************************************************************************/
+/****************************************************************************
+ * Name: libelf_registry
+ ****************************************************************************/
+
+FAR struct module_s **libelf_registry(void)
+{
+ return &task_get_info()->ta_modules;
+}
+
+/****************************************************************************
+ * Name: libelf_registry_kernel
+ ****************************************************************************/
+
+FAR struct module_s **libelf_registry_kernel(void)
Review Comment:
Done
##########
libs/libc/elf/elf_registry.c:
##########
@@ -46,13 +47,35 @@
* Private Data
****************************************************************************/
+/* One lock for every registry. g_mod_registry holds the kernel modules of
+ * insmod(); what dlopen() loads is on the registry of the task group.
+ */
+
static rmutex_t g_modlock = NXRMUTEX_INITIALIZER;
static FAR struct module_s *g_mod_registry;
/****************************************************************************
* Public Functions
****************************************************************************/
+/****************************************************************************
+ * Name: libelf_registry
+ ****************************************************************************/
+
+FAR struct module_s **libelf_registry(void)
Review Comment:
Done
##########
libs/libc/elf/elf_bind.c:
##########
@@ -1040,7 +984,8 @@ static int libelf_relocatedyn(FAR struct module_s *modp,
return ret;
}
- handle = libelf_insertneeded((FAR const char *)loadinfo->iobuffer);
+ handle = libelf_insertneeded(modp->registry,
Review Comment:
Done
##########
libs/libc/elf/elf_remove.c:
##########
@@ -289,3 +290,42 @@ int libelf_remove(FAR void *handle)
set_errno(-ret);
return ERROR;
}
+
+/****************************************************************************
+ * Name: libelf_closeall
+ *
+ * Description:
+ * Give back every reference that dlopen() took in a registry. A module
+ * that another one still depends on goes with its last dependent.
+ *
+ ****************************************************************************/
+
+void libelf_closeall(FAR struct module_s **registry)
+{
+ FAR struct module_s *modp;
+
+ libelf_registry_lock();
Review Comment:
Done
##########
libs/libc/elf/elf_remove.c:
##########
@@ -289,3 +290,42 @@ int libelf_remove(FAR void *handle)
set_errno(-ret);
return ERROR;
}
+
+/****************************************************************************
+ * Name: libelf_closeall
+ *
+ * Description:
+ * Give back every reference that dlopen() took in a registry. A module
+ * that another one still depends on goes with its last dependent.
+ *
+ ****************************************************************************/
+
+void libelf_closeall(FAR struct module_s **registry)
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "casaroli (via GitHub)" <gi...@apache.org>.
casaroli commented on code in PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#discussion_r4292141563
##########
Documentation/os/binfmt/kernel_modules_vs_shared_libraries.rst:
##########
@@ -139,40 +139,52 @@ FLAT build.
Opening a Library More Than Once
================================
-In the FLAT and PROTECTED builds, ``dlopen()`` of a library that is already
+In the FLAT and PROTECTED builds, each task group has its own list of
+loaded libraries. ``dlopen()`` of a library that the task group already
loaded does not load a second copy and does not fail. It returns a handle to
the library that is already loaded and takes an additional reference on it.
Each successful ``dlopen()`` must be matched by a ``dlclose()``; the library
is unloaded only when the last handle is closed. Up to 255 handles may be
outstanding on one library; beyond that ``dlopen()`` fails with ``EMFILE``.
+The libraries that a task group did not close are closed when it exits.
+In the PROTECTED build only ``exit()`` closes them, so a task group that
+ends in another way leaves them loaded.
Some consequences worth keeping in mind:
* A library is identified by the *basename* of the path passed to
``dlopen()``. Two files with the same basename in different directories
are treated as the same library, and the second ``dlopen()`` will return
the first one.
-* There is only one instance of the library's ``.data`` and ``.bss``. Global
- and static data are shared by every user of the library, and by every task
- group in the system.
-* Constructors in ``.init_array`` run once, when the library is first loaded,
- and destructors in ``.fini_array`` run once, when the last handle is
- closed. They do not run per ``dlopen()``/``dlclose()`` pair.
+* Each task group that opens a library loads its own copy. Global and static
+ data are shared by the threads of one task group, not between task groups.
+ Only an FDPIC library executed in place from xipfs shares its ``.text``
+ between task groups.
+* Constructors in ``.init_array`` run once per task group, when the library
+ is first loaded in it, and destructors in ``.fini_array`` run once, when
+ its last handle in the task group is closed. They do not run per
+ ``dlopen()``/``dlclose()`` pair.
+* The libraries that a program loaded by ``exec()`` names in ``DT_NEEDED``
+ belong to its task group in the FLAT build, so ``dlopen()`` of one of them
Review Comment:
@xiaoxiang781216 I think we can make it work in kernel mode as well, and I would like to do it. However, the kernel work is relatively small compared to the userspace. So I suggest we merge this one as it is, as it wraps up the fdpic (XIP) shared object support. And since my next batch of work is focused on improving kernel mode, I will add shared object support in next PRs.
What do you think? Can we merge this one?
Thank you for the reviews!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "casaroli (via GitHub)" <gi...@apache.org>.
casaroli commented on PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#issuecomment-6188294482
@xiaoxiang781216 can you please take a look at this one? it is blocking my next wave of PRs. thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "casaroli (via GitHub)" <gi...@apache.org>.
casaroli commented on code in PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#discussion_r4282231396
##########
sched/module/mod_registry.c:
##########
@@ -0,0 +1,91 @@
+/****************************************************************************
+ * sched/module/mod_registry.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 <nuttx/config.h>
+
+#include <errno.h>
+
+#include <nuttx/list.h>
+#include <nuttx/mutex.h>
+
+#include "module/module.h"
+
+#ifdef CONFIG_MODULE
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "casaroli (via GitHub)" <gi...@apache.org>.
casaroli closed pull request #20492: libs/libc/elf: Keep a module registry per task group.
URL: https://github.com/apache/nuttx/pull/20492
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "casaroli (via GitHub)" <gi...@apache.org>.
casaroli commented on code in PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#discussion_r4303709761
##########
binfmt/binfmt_execmodule.c:
##########
@@ -45,6 +45,7 @@
#include <nuttx/spawn.h>
#include <nuttx/binfmt/binfmt.h>
#include <nuttx/environ.h>
+#include <nuttx/lib/elf.h>
Review Comment:
Done
##########
binfmt/binfmt_execmodule.c:
##########
@@ -190,6 +191,10 @@ int exec_module(FAR struct binary_s *binp,
char name[PATH_MAX];
#endif
FAR void *stackaddr = NULL;
+#if defined(CONFIG_BINFMT_LOADABLE) && defined(CONFIG_LIBC_ELF) && \
Review Comment:
Done, removed.
##########
binfmt/binfmt_execmodule.c:
##########
@@ -190,6 +191,10 @@ int exec_module(FAR struct binary_s *binp,
char name[PATH_MAX];
#endif
FAR void *stackaddr = NULL;
+#if defined(CONFIG_BINFMT_LOADABLE) && defined(CONFIG_LIBC_ELF) && \
+ defined(CONFIG_BUILD_FLAT)
Review Comment:
In a protected build the kernel loads the program and allocates these entries in kernel memory, but ta_modules belongs to the program's libc, which frees with the user heap. So there they stay on binary_s, as in the DT_NEEDED thread. In a kernel build DT_NEEDED is not supported yet; I will make all modes use the same path when I add it, as agreed there.
##########
binfmt/binfmt_loadmodule.c:
##########
@@ -184,6 +184,12 @@ int load_module(FAR struct binary_s *bin, FAR const char *filename,
if (bin && filename)
#endif
{
+#ifdef CONFIG_LIBC_ELF
+ /* The DT_NEEDED libraries of the program go here */
+
+ list_initialize(&bin->libraries);
Review Comment:
Done
##########
binfmt/elf.c:
##########
@@ -141,6 +141,12 @@ static int elf_loadbinary(FAR struct binary_s *binp,
if (loadinfo.ehdr.e_type == ET_REL || loadinfo.gotsize != 0)
{
+ /* Its DT_NEEDED libraries go on binp->libraries. The task group
+ * does not exist yet. In a flat build, exec_module() gives them to
+ * the new task group.
+ */
+
+ loadinfo.registry = &binp->libraries;
Review Comment:
The new task group does not exist yet when exec() loads the program, so its ta_modules does not exist either. The libraries wait on binp->libraries, and exec_module() moves them once the group is created.
##########
include/nuttx/lib/elf.h:
##########
@@ -776,9 +764,10 @@ FAR void *libelf_insert(FAR const char *filename, FAR const char *modname);
* called.
*
* Input Parameters:
- * handle - The opaque, non-NULL value returned by a previous successful
- * call to libelf_insert().
- * name - A pointer to the symbol name string.
+ * registry - The registry the module is on.
+ * handle - The opaque, non-NULL value returned by a previous successful
Review Comment:
Done: libelf_getsymbol() takes only the handle.
##########
include/nuttx/tls.h:
##########
@@ -173,6 +173,10 @@ struct task_info_s
size_t ta_envpc; /* Maximum entries of environment array */
size_t ta_envc; /* Number of environment strings */
#endif
+#ifdef CONFIG_LIBC_ELF
Review Comment:
Done: the fields exist with CONFIG_LIBC_ELF or CONFIG_BINFMT_LOADABLE.
##########
libs/libc/dlfcn/lib_dlfcn.h:
##########
@@ -0,0 +1,84 @@
+/****************************************************************************
+ * libs/libc/dlfcn/lib_dlfcn.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __LIBS_LIBC_DLFCN_LIB_DLFCN_H
+#define __LIBS_LIBC_DLFCN_LIB_DLFCN_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <errno.h>
+
+#include <nuttx/list.h>
+#include <nuttx/mutex.h>
+#include <nuttx/tls.h>
+
+#ifdef CONFIG_LIBC_ELF
+
+/****************************************************************************
+ * Inline Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: dl_registry_lock
+ *
+ * Description:
+ * Lock the registry of the calling task group, which holds what dlopen()
+ * loads, and return it. The lock is recursive: the constructors of a
+ * library run with it held, and they can call dlopen().
+ *
+ ****************************************************************************/
+
+static inline FAR struct list_node *dl_registry_lock(void)
Review Comment:
Done: task_modules_lock() and task_modules_unlock() are in tls.h now, and lib_dlfcn.h is gone.
##########
libs/libc/elf/elf_bind.c:
##########
@@ -1040,7 +1042,20 @@ static int libelf_relocatedyn(FAR struct module_s *modp,
return ret;
}
- handle = libelf_insertneeded((FAR const char *)loadinfo->iobuffer);
+ /* The library goes on the registry of the module that needs it */
+
+ if (loadinfo->registry == NULL)
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "casaroli (via GitHub)" <gi...@apache.org>.
casaroli commented on code in PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#discussion_r4303710760
##########
libs/libc/elf/elf_getsymbol.c:
##########
@@ -73,13 +78,12 @@ FAR const void *libelf_getsymbol(FAR void *handle, FAR const char *name)
/* Verify that the module is in the registry */
- libelf_registry_lock();
- ret = libelf_registry_verify(modp);
+ ret = libelf_registry_verify(registry, modp);
Review Comment:
Done
##########
libs/libc/elf/elf_symbols.c:
##########
@@ -470,7 +470,9 @@ int libelf_symvalue(FAR struct module_s *modp,
exportinfo.modp = modp;
exportinfo.symbol = NULL;
- ret = libelf_registry_foreach(libelf_symcallback,
+ ret = loadinfo->registry == NULL ? OK :
Review Comment:
Done: libelf_registry_foreach() accepts a NULL registry.
##########
sched/group/group_leave.c:
##########
@@ -80,6 +81,30 @@ static inline void group_release(FAR struct task_group_s *group)
nxrmutex_destroy(&group->tg_mutex);
+#if defined(CONFIG_LIBC_ELF) && defined(CONFIG_BUILD_FLAT)
+ /* Close what dlopen() left open. No thread of the group runs now, so
+ * the lock is not needed.
+ */
+
+ libelf_removeall(&group->tg_info->ta_modules);
+
+# ifdef CONFIG_BINFMT_LOADABLE
+ /* The rest is used by the program. Give it back to the program before
+ * the list goes with the task information: binfmt_exit() unloads it.
+ */
+
+ if (group->tg_bininfo != NULL)
+ {
+ FAR struct list_node *node;
+
+ while ((node = list_remove_head(&group->tg_info->ta_modules)) != NULL)
+ {
+ list_add_tail(&group->tg_bininfo->libraries, node);
Review Comment:
ta_modules is in the task information, which task_uninit_info() frees right after this. The program's own libraries are still on it and the program still uses them, so they go back to tg_bininfo->libraries, and binfmt_exit() unloads them together with the program.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "casaroli (via GitHub)" <gi...@apache.org>.
casaroli commented on code in PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#discussion_r4304393802
##########
include/nuttx/tls.h:
##########
@@ -43,10 +43,15 @@
#include <nuttx/mutex.h>
#include <nuttx/queue.h>
-#ifdef CONFIG_PTHREAD_ATFORK
+#if defined(CONFIG_PTHREAD_ATFORK) || defined(CONFIG_LIBC_ELF) || \
+ defined(CONFIG_BINFMT_LOADABLE)
# include <nuttx/list.h>
Review Comment:
Done.
##########
include/nuttx/tls.h:
##########
@@ -43,10 +43,15 @@
#include <nuttx/mutex.h>
#include <nuttx/queue.h>
-#ifdef CONFIG_PTHREAD_ATFORK
+#if defined(CONFIG_PTHREAD_ATFORK) || defined(CONFIG_LIBC_ELF) || \
+ defined(CONFIG_BINFMT_LOADABLE)
# include <nuttx/list.h>
#endif
+#if defined(CONFIG_LIBC_ELF) || defined(CONFIG_BINFMT_LOADABLE)
+# include <errno.h>
Review Comment:
Done.
##########
include/nuttx/tls.h:
##########
@@ -173,6 +178,10 @@ struct task_info_s
size_t ta_envpc; /* Maximum entries of environment array */
size_t ta_envc; /* Number of environment strings */
#endif
+#if defined(CONFIG_LIBC_ELF) || defined(CONFIG_BINFMT_LOADABLE)
Review Comment:
No. `LIBC_ELF` does not select `BINFMT_LOADABLE`: `mps3-an547:bl` has `CONFIG_LIBC_ELF=y` and no binary loader, and its `dlopen()` uses `ta_modules`.
##########
libs/libc/elf/elf_getsymbol.c:
##########
@@ -48,9 +48,10 @@
* called.
*
* Input Parameters:
- * handle - The opaque, non-NULL value returned by a previous successful
- * call to libelf_insert().
- * name - A pointer to the symbol name string.
+ * registry - The registry the module is on.
Review Comment:
Done.
##########
libs/libc/elf/elf_getsymbol.c:
##########
@@ -62,41 +63,30 @@
* NOTE: This means that the address zero can never be a valid return
* value.
*
+ * Assumptions:
+ * The caller holds the lock of the registry.
Review Comment:
It is not needed. `libelf_getsymbol()` no longer checks the registry, so the lock protected nothing. I removed the note and the lock in `dlsym()`; `dlsym()` is now the same as on master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#discussion_r4266485588
##########
sched/module/mod_registry.c:
##########
@@ -0,0 +1,91 @@
+/****************************************************************************
+ * sched/module/mod_registry.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 <nuttx/config.h>
+
+#include <errno.h>
+
+#include <nuttx/list.h>
+#include <nuttx/mutex.h>
+
+#include "module/module.h"
+
+#ifdef CONFIG_MODULE
Review Comment:
remove
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#discussion_r4292301530
##########
Documentation/os/binfmt/kernel_modules_vs_shared_libraries.rst:
##########
@@ -139,40 +139,52 @@ FLAT build.
Opening a Library More Than Once
================================
-In the FLAT and PROTECTED builds, ``dlopen()`` of a library that is already
+In the FLAT and PROTECTED builds, each task group has its own list of
+loaded libraries. ``dlopen()`` of a library that the task group already
loaded does not load a second copy and does not fail. It returns a handle to
the library that is already loaded and takes an additional reference on it.
Each successful ``dlopen()`` must be matched by a ``dlclose()``; the library
is unloaded only when the last handle is closed. Up to 255 handles may be
outstanding on one library; beyond that ``dlopen()`` fails with ``EMFILE``.
+The libraries that a task group did not close are closed when it exits.
+In the PROTECTED build only ``exit()`` closes them, so a task group that
+ends in another way leaves them loaded.
Some consequences worth keeping in mind:
* A library is identified by the *basename* of the path passed to
``dlopen()``. Two files with the same basename in different directories
are treated as the same library, and the second ``dlopen()`` will return
the first one.
-* There is only one instance of the library's ``.data`` and ``.bss``. Global
- and static data are shared by every user of the library, and by every task
- group in the system.
-* Constructors in ``.init_array`` run once, when the library is first loaded,
- and destructors in ``.fini_array`` run once, when the last handle is
- closed. They do not run per ``dlopen()``/``dlclose()`` pair.
+* Each task group that opens a library loads its own copy. Global and static
+ data are shared by the threads of one task group, not between task groups.
+ Only an FDPIC library executed in place from xipfs shares its ``.text``
+ between task groups.
+* Constructors in ``.init_array`` run once per task group, when the library
+ is first loaded in it, and destructors in ``.fini_array`` run once, when
+ its last handle in the task group is closed. They do not run per
+ ``dlopen()``/``dlclose()`` pair.
+* The libraries that a program loaded by ``exec()`` names in ``DT_NEEDED``
+ belong to its task group in the FLAT build, so ``dlopen()`` of one of them
Review Comment:
Sure, I am busy these days, I will review your change in the weekend. Sorry the delay.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "casaroli (via GitHub)" <gi...@apache.org>.
casaroli closed pull request #20492: libs/libc/elf: Keep a module registry per task group.
URL: https://github.com/apache/nuttx/pull/20492
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#issuecomment-6043526960
<!-- membrowse-pr-comment -->
## <img src="https://membrowse.com/membrowse-logo.svg" height="24" align="top"> MemBrowse Memory Report
### [s698pm-dkit](https://membrowse.com/public/apache/nuttx/target/s698pm-dkit/compare?head=69e3fe60433726e975836906ffee6e5bafa732d0&base=c7cbee7a3378333e686a06eeacf44805887aa8d5)
- **Code**: .text +800 B (+0.2%, 373,680 B)
*No memory changes detected for:*
- [arduino-mega2560](https://membrowse.com/public/apache/nuttx/target/arduino-mega2560/compare?head=69e3fe60433726e975836906ffee6e5bafa732d0&base=c7cbee7a3378333e686a06eeacf44805887aa8d5)
- [esp32-devkitc](https://membrowse.com/public/apache/nuttx/target/esp32-devkitc/compare?head=69e3fe60433726e975836906ffee6e5bafa732d0&base=c7cbee7a3378333e686a06eeacf44805887aa8d5)
- [hifive1-revb](https://membrowse.com/public/apache/nuttx/target/hifive1-revb/compare?head=69e3fe60433726e975836906ffee6e5bafa732d0&base=c7cbee7a3378333e686a06eeacf44805887aa8d5)
- [mirtoo](https://membrowse.com/public/apache/nuttx/target/mirtoo/compare?head=69e3fe60433726e975836906ffee6e5bafa732d0&base=c7cbee7a3378333e686a06eeacf44805887aa8d5)
- [qemu-armv8a](https://membrowse.com/public/apache/nuttx/target/qemu-armv8a/compare?head=69e3fe60433726e975836906ffee6e5bafa732d0&base=c7cbee7a3378333e686a06eeacf44805887aa8d5)
- [qemu-intel64](https://membrowse.com/public/apache/nuttx/target/qemu-intel64/compare?head=69e3fe60433726e975836906ffee6e5bafa732d0&base=c7cbee7a3378333e686a06eeacf44805887aa8d5)
- [rx65n-rsk2mb](https://membrowse.com/public/apache/nuttx/target/rx65n-rsk2mb/compare?head=69e3fe60433726e975836906ffee6e5bafa732d0&base=c7cbee7a3378333e686a06eeacf44805887aa8d5)
- [stm32-nucleo-f103rb](https://membrowse.com/public/apache/nuttx/target/stm32-nucleo-f103rb/compare?head=69e3fe60433726e975836906ffee6e5bafa732d0&base=c7cbee7a3378333e686a06eeacf44805887aa8d5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "casaroli (via GitHub)" <gi...@apache.org>.
casaroli closed pull request #20492: libs/libc/elf: Keep a module registry per task group.
URL: https://github.com/apache/nuttx/pull/20492
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "casaroli (via GitHub)" <gi...@apache.org>.
casaroli commented on PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#issuecomment-6054266923
ok, i agree
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] libs/libc/elf: Keep a module registry per task group. [nuttx]
Posted by "casaroli (via GitHub)" <gi...@apache.org>.
casaroli commented on code in PR #20492:
URL: https://github.com/apache/nuttx/pull/20492#discussion_r4307019508
##########
sched/module/mod_modhandle.c:
##########
@@ -29,6 +29,8 @@
#include <nuttx/module.h>
#include <nuttx/lib/elf.h>
+#include "module/module.h"
+
#ifdef CONFIG_MODULE
Review Comment:
Done.
##########
sched/module/mod_procfs.c:
##########
@@ -42,6 +42,8 @@
#include <nuttx/fs/fs.h>
#include <nuttx/fs/procfs.h>
+#include "module/module.h"
+
#if !defined(CONFIG_DISABLE_MOUNTPOINT) && defined(CONFIG_FS_PROCFS) && \
!defined(CONFIG_FS_PROCFS_EXCLUDE_MODULE)
Review Comment:
Done: the build files check CONFIG_DISABLE_MOUNTPOINT now too, so mod_procfs.c has no check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org