xiaoxiang781216 commented on code in PR #20368:
URL: https://github.com/apache/nuttx/pull/20368#discussion_r4115821300
##########
binfmt/Kconfig:
##########
@@ -66,6 +66,7 @@ config FDPIC
default n
select PIC
depends on ARCH_HAVE_ELF_FDPIC
+ depends on BUILD_FLAT
Review Comment:
why depend on BUILD_FLAT
##########
libs/libc/elf/elf_symbols.c:
##########
@@ -93,8 +93,8 @@ extern int nglobals;
*
****************************************************************************/
-static int libelf_symname(FAR struct mod_loadinfo_s *loadinfo,
- FAR const Elf_Sym *sym, Elf_Off sh_offset)
+int libelf_symname(FAR struct mod_loadinfo_s *loadinfo,
Review Comment:
need move to public section
##########
libs/libc/elf/elf_remove.c:
##########
@@ -101,6 +104,17 @@ int libelf_uninit(FAR struct module_s *modp)
modp->modinfo.exports = NULL;
modp->modinfo.nexports = 0;
+#ifdef CONFIG_LIBC_DLFCN
+ /* Let go of anything opened for DT_NEEDED. After the destructors, which
+ * may call into those libraries.
+ */
+
+ while (modp->nlibs > 0)
+ {
+ dlclose(modp->libs[--modp->nlibs]);
Review Comment:
libc/elf shouldn't call dlxxx directly since this library may used by kernel
module and elf loader
##########
include/nuttx/lib/elf.h:
##########
@@ -214,6 +218,16 @@ struct module_s
* module goes when the last does
*/
+#ifdef CONFIG_LIBC_DLFCN
+ /* Libraries opened with dlopen() for this module's DT_NEEDED entries.
+ * These are references this module holds on others, where nopen above
+ * counts the references others hold on this one.
+ */
+
+ FAR void *libs[CONFIG_LIBC_ELF_MAXNEEDED];
+ uint8_t nlibs;
+#endif
+
#if CONFIG_LIBC_ELF_MAXDEPEND > 0
Review Comment:
why need both CONFIG_LIBC_ELF_MAXNEEDED and CONFIG_LIBC_ELF_MAXDEPEND? only
one is needed to implement this feature.
##########
libs/libc/elf/elf_bind.c:
##########
@@ -812,9 +853,68 @@ static int libelf_relocatedyn(FAR struct module_s *modp,
loadinfo->gotbase);
}
- /* After the loop, because DT_PLTGOT is read there. Both relocation
- * tables are walked under this one arch_data, so the pool cursor
- * survives from one to the next.
+ /* Open whatever the object names in DT_NEEDED. dlopen() is the loader
+ * for a shared library, so hand the work to it.
+ */
+
+#ifdef CONFIG_LIBC_DLFCN
+
+ symhdr = &loadinfo->shdr[loadinfo->dsymtabidx];
+
+ for (i = 0; i < nlibs; i++)
+ {
+ Elf_Sym namesym;
+ FAR void *handle;
+
+ /* The name is a string table offset, which is what st_name is, so
+ * the existing reader can fetch it.
+ */
+
+ memset(&namesym, 0, sizeof(namesym));
+ namesym.st_name = libs[i];
+
+ ret = libelf_symname(loadinfo, &namesym,
+ loadinfo->shdr[symhdr->sh_link].sh_offset);
+ if (ret < 0)
+ {
+ berr("ERROR: DT_NEEDED %d has no name\n", i);
+ lib_free(sym);
+ lib_free(rels);
+ lib_free(dyn);
+ return ret;
+ }
+
+ handle = dlopen((FAR const char *)loadinfo->iobuffer, RTLD_NOW);
+ if (handle == NULL)
+ {
+ berr("ERROR: Cannot open needed library %s\n",
+ (FAR char *)loadinfo->iobuffer);
+ lib_free(sym);
+ lib_free(rels);
+ lib_free(dyn);
+ return -ELIBACC;
+ }
+
+ binfo("Opened needed library %s\n", (FAR char *)loadinfo->iobuffer);
+
+ modp->libs[modp->nlibs++] = handle;
Review Comment:
why not reuse LIBC_ELF_MAXDEPEND
##########
libs/libc/elf/elf.h:
##########
@@ -255,6 +255,17 @@ int libelf_freebuffers(FAR struct mod_loadinfo_s
*loadinfo);
*
****************************************************************************/
+/****************************************************************************
Review Comment:
move before line 241
--
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]
