xiaoxiang781216 commented on code in PR #20089:
URL: https://github.com/apache/nuttx/pull/20089#discussion_r3972223004
##########
libs/libc/elf/elf_bind.c:
##########
@@ -858,6 +903,22 @@ static int libelf_relocatedyn(FAR struct module_s *modp,
}
}
+ /* FDPIC relocations without the OS/ABI byte cannot be run:
+ * nothing would place the segments apart or install the data
+ * base.
+ */
+
+ if (!loadinfo->fdpic &&
Review Comment:
remove? the same check already do in up_realocate.
##########
libs/libc/elf/elf_bind.c:
##########
@@ -815,6 +851,15 @@ static int libelf_relocatedyn(FAR struct module_s *modp,
ret = OK;
lrelent = reldata.relsz[idx_rel] / reldata.relentsz[idx_rel];
+#ifdef HAVE_ARCH_ELF_FDPIC
+ /* Say which table this is. A relocation out of DT_JMPREL overwrites
+ * a word the linker pre-loaded with a lazy binding stub, which is not
+ * an addend and must not be added to.
+ */
+
+ arch_data.pltrel = (idx_rel == I_PLT);
Review Comment:
not good to touch the internal field of arch specific struct in the common
code
##########
libs/libc/machine/arm/armv8-m/arch_elf.c:
##########
@@ -175,6 +176,97 @@ int up_relocate(const Elf32_Rel *rel, const Elf32_Sym
*sym, uintptr_t addr,
}
break;
+ case R_ARM_FUNCDESC_VALUE:
+ {
+ /* The target is a descriptor: entry point and data base. The
+ * addend sits in the word that becomes the entry point and
+ * carries the Thumb bit, so it must be kept. The base written is
+ * this object's own, which is what makes a callback work.
+ */
+
+ FAR struct fdpic_desc_s *desc =
Review Comment:
remove FAR in this file, since it arm specific code and not the far model.
##########
libs/libc/elf/elf_bind.c:
##########
@@ -909,7 +970,37 @@ static int libelf_relocatedyn(FAR struct module_s *modp,
addr += rela->r_addend;
}
- *(FAR uintptr_t *)addr = (uintptr_t)ep;
+ if (loadinfo->fdpic)
+ {
+ /* Under FDPIC an import may be a descriptor,
+ * which is built rather than assigned, so let the
+ * relocation type decide what to write.
+ */
+
+ Elf_Sym extsym =
+ {
+ 0
+ };
+
+ extsym.st_value = (uintptr_t)ep;
+
+ ret = up_relocate(rel, &extsym, addr,
+ ARCH_ELFDATA_PARM);
+ if (ret < 0)
+ {
+ berr("ERROR: Section %d reloc %d: "
+ "Relocation failed: %d\n",
+ relidx, i, ret);
+ lib_free(sym);
+ lib_free(rels);
+ lib_free(dyn);
+ return ret;
+ }
+ }
+ else
+ {
+ *(FAR uintptr_t *)addr = (uintptr_t)ep;
+ }
}
else if (loadinfo->fdpic)
Review Comment:
can we share the same `else if` and `else` block, let up_relocate handle the
difference?
--
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]
