From 024f43b1f0960fe247f4a7589182cded67fbe412 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 13 Dec 2024 11:31:33 +0100 Subject: [PATCH] llext: move a calculation to a more logical location Currently llext_link_plt() calculates offsets to relocation addresses relative to the .text section and passes them to arch_elf_relocate_global() and arch_elf_relocate_local(), where then .text memory address is added to them. Instead it's more logical to concentrate the entire calculation in the caller, which then also removes the assumption, that all sections have the same VMA - LMA offset from those functions. Signed-off-by: Guennadi Liakhovetski --- arch/xtensa/core/elf.c | 23 +++++++++++------------ include/zephyr/llext/llext.h | 8 ++++---- subsys/llext/llext_link.c | 23 ++++++++++++----------- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/arch/xtensa/core/elf.c b/arch/xtensa/core/elf.c index 04dac478215..84d80127411 100644 --- a/arch/xtensa/core/elf.c +++ b/arch/xtensa/core/elf.c @@ -33,15 +33,18 @@ LOG_MODULE_DECLARE(llext, CONFIG_LLEXT_LOG_LEVEL); #define R_XTENSA_SLOT0_OP 20 static void xtensa_elf_relocate(struct llext_loader *ldr, struct llext *ext, - const elf_rela_t *rel, uint8_t *text, uintptr_t addr, + const elf_rela_t *rel, uintptr_t addr, uint8_t *loc, int type, uint32_t stb) { elf_word *got_entry = (elf_word *)loc; switch (type) { case R_XTENSA_RELATIVE: + ; /* Relocate a local symbol: Xtensa specific. Seems to only be used with PIC */ - *got_entry += (uintptr_t)text - addr; + uintptr_t text = (uintptr_t)ext->mem[LLEXT_MEM_TEXT]; + + *got_entry += text - addr; break; case R_XTENSA_GLOB_DAT: case R_XTENSA_JMP_SLOT: @@ -113,11 +116,9 @@ static void xtensa_elf_relocate(struct llext_loader *ldr, struct llext *ext, * @brief Architecture specific function for STB_LOCAL ELF relocations */ void arch_elf_relocate_local(struct llext_loader *ldr, struct llext *ext, const elf_rela_t *rel, - const elf_sym_t *sym, size_t got_offset, + const elf_sym_t *sym, uint8_t *rel_addr, const struct llext_load_param *ldr_parm) { - uint8_t *text = ext->mem[LLEXT_MEM_TEXT]; - uint8_t *loc = text + got_offset; int type = ELF32_R_TYPE(rel->r_info); uintptr_t sh_addr; @@ -133,24 +134,22 @@ void arch_elf_relocate_local(struct llext_loader *ldr, struct llext *ext, const sh_addr = ldr->sects[LLEXT_MEM_TEXT].sh_addr; } - xtensa_elf_relocate(ldr, ext, rel, text, sh_addr, loc, type, ELF_ST_BIND(sym->st_info)); + xtensa_elf_relocate(ldr, ext, rel, sh_addr, rel_addr, type, ELF_ST_BIND(sym->st_info)); } /** * @brief Architecture specific function for STB_GLOBAL ELF relocations */ void arch_elf_relocate_global(struct llext_loader *ldr, struct llext *ext, const elf_rela_t *rel, - const elf_sym_t *sym, size_t got_offset, const void *link_addr) + const elf_sym_t *sym, uint8_t *rel_addr, const void *link_addr) { - uint8_t *text = ext->mem[LLEXT_MEM_TEXT]; - elf_word *got_entry = (elf_word *)(text + got_offset); int type = ELF32_R_TYPE(rel->r_info); /* For global relocations we expect the initial value for R_XTENSA_RELATIVE to be zero */ - if (type == R_XTENSA_RELATIVE && *got_entry) { - LOG_WRN("global: non-zero relative value %#x", *got_entry); + if (type == R_XTENSA_RELATIVE && *(elf_word *)rel_addr) { + LOG_WRN("global: non-zero relative value %#x", *(elf_word *)rel_addr); } - xtensa_elf_relocate(ldr, ext, rel, text, (uintptr_t)link_addr, (uint8_t *)got_entry, type, + xtensa_elf_relocate(ldr, ext, rel, (uintptr_t)link_addr, rel_addr, type, ELF_ST_BIND(sym->st_info)); } diff --git a/include/zephyr/llext/llext.h b/include/zephyr/llext/llext.h index 5bd16ea3356..fc9654c408a 100644 --- a/include/zephyr/llext/llext.h +++ b/include/zephyr/llext/llext.h @@ -377,11 +377,11 @@ int llext_get_section_header(struct llext_loader *loader, struct llext *ext, * @param[in] ext Extension to call function in * @param[in] rel Relocation data provided by elf * @param[in] sym Corresponding symbol table entry - * @param[in] got_offset Offset within a relocation table or in the code + * @param[in] rel_addr Address where relocation should be performed * @param[in] ldr_parm Loader parameters */ void arch_elf_relocate_local(struct llext_loader *loader, struct llext *ext, const elf_rela_t *rel, - const elf_sym_t *sym, size_t got_offset, + const elf_sym_t *sym, uint8_t *rel_addr, const struct llext_load_param *ldr_parm); /** @@ -391,11 +391,11 @@ void arch_elf_relocate_local(struct llext_loader *loader, struct llext *ext, con * @param[in] ext Extension to call function in * @param[in] rel Relocation data provided by elf * @param[in] sym Corresponding symbol table entry - * @param[in] got_offset Offset within a relocation table or in the code + * @param[in] rel_addr Address where relocation should be performed * @param[in] link_addr target address for table-based relocations */ void arch_elf_relocate_global(struct llext_loader *loader, struct llext *ext, const elf_rela_t *rel, - const elf_sym_t *sym, size_t got_offset, const void *link_addr); + const elf_sym_t *sym, uint8_t *rel_addr, const void *link_addr); /** * @} diff --git a/subsys/llext/llext_link.c b/subsys/llext/llext_link.c index 4ad168f4f47..2cd038dfb11 100644 --- a/subsys/llext/llext_link.c +++ b/subsys/llext/llext_link.c @@ -33,13 +33,13 @@ __weak int arch_elf_relocate(elf_rela_t *rel, uintptr_t loc, } __weak void arch_elf_relocate_local(struct llext_loader *ldr, struct llext *ext, - const elf_rela_t *rel, const elf_sym_t *sym, size_t got_offset, + const elf_rela_t *rel, const elf_sym_t *sym, uint8_t *rel_addr, const struct llext_load_param *ldr_parm) { } __weak void arch_elf_relocate_global(struct llext_loader *ldr, struct llext *ext, - const elf_rela_t *rel, const elf_sym_t *sym, size_t got_offset, + const elf_rela_t *rel, const elf_sym_t *sym, uint8_t *rel_addr, const void *link_addr) { } @@ -216,14 +216,15 @@ static void llext_link_plt(struct llext_loader *ldr, struct llext *ext, elf_shdr * This is valid only when CONFIG_LLEXT_STORAGE_WRITABLE=y * and peek() is usable on the source ELF file. */ - size_t got_offset; + uint8_t *rel_addr = (uint8_t *)ext->mem[LLEXT_MEM_TEXT] - + ldr->sects[LLEXT_MEM_TEXT].sh_offset; if (tgt) { - got_offset = rela.r_offset + tgt->sh_offset - - ldr->sects[LLEXT_MEM_TEXT].sh_offset; + /* Relocatable / partially linked ELF. */ + rel_addr += rela.r_offset + tgt->sh_offset; } else { - got_offset = llext_file_offset(ldr, rela.r_offset) - - ldr->sects[LLEXT_MEM_TEXT].sh_offset; + /* Shared / dynamically linked ELF */ + rel_addr += llext_file_offset(ldr, rela.r_offset); } uint32_t stb = ELF_ST_BIND(sym.st_info); @@ -256,14 +257,14 @@ static void llext_link_plt(struct llext_loader *ldr, struct llext *ext, elf_shdr } /* Resolve the symbol */ - arch_elf_relocate_global(ldr, ext, &rela, &sym, got_offset, link_addr); + arch_elf_relocate_global(ldr, ext, &rela, &sym, rel_addr, link_addr); break; case STB_LOCAL: - arch_elf_relocate_local(ldr, ext, &rela, &sym, got_offset, ldr_parm); + arch_elf_relocate_local(ldr, ext, &rela, &sym, rel_addr, ldr_parm); } - LOG_DBG("symbol %s offset %#zx r-offset %#zx .text offset %#zx stb %u", - name, got_offset, + LOG_DBG("symbol %s relocation @%p r-offset %#zx .text offset %#zx stb %u", + name, (void *)rel_addr, (size_t)rela.r_offset, (size_t)ldr->sects[LLEXT_MEM_TEXT].sh_offset, stb); } }