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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to