From f86dd0e18a623ab332446fad66e88ebc4a7f0670 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Wed, 8 May 2024 15:18:46 +0200 Subject: [PATCH 1/3] llext: fix handling of non-standard sections in relocatable case When building partially linked / relocatable objects no ELF segments are created and it becomes more difficult to predict which sections the compiler will build and use. In this case a .data.rel.local section is created by the compiler and it is needed to link .rodata strings in a twister test. We can handle arbitrary sections at run- time if .peek() is supported. If it isn't we need to allocate and copy the section. For now we simply error out in such cases. Fixing that would represent a larger change and can be done incrementally. This also fixes the relocation calculation to point to the correct symbol address instead of the memory location, where it's currently residing, because that can be a temporary buffer as is the case with SOF. Signed-off-by: Guennadi Liakhovetski --- include/zephyr/llext/llext.h | 5 ++++ subsys/llext/llext.c | 57 +++++++++++++++++++++++++++++++----- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/include/zephyr/llext/llext.h b/include/zephyr/llext/llext.h index 59a136ab5d4ef..acdca225721a5 100644 --- a/include/zephyr/llext/llext.h +++ b/include/zephyr/llext/llext.h @@ -124,6 +124,11 @@ int llext_iterate(int (*fn)(struct llext *ext, void *arg), void *arg); struct llext_load_param { /** Should local relocation be performed */ bool relocate_local; + /** + * Use the virtual symbol addresses from the ELF, not addresses within + * the memory buffer, where the object is loaded + */ + bool pre_located; }; #define LLEXT_LOAD_PARAM_DEFAULT {.relocate_local = true,} diff --git a/subsys/llext/llext.c b/subsys/llext/llext.c index d0a04ff7d8c68..35afa99077c08 100644 --- a/subsys/llext/llext.c +++ b/subsys/llext/llext.c @@ -530,7 +530,8 @@ static int llext_export_symbols(struct llext_loader *ldr, struct llext *ext) return 0; } -static int llext_copy_symbols(struct llext_loader *ldr, struct llext *ext) +static int llext_copy_symbols(struct llext_loader *ldr, struct llext *ext, + bool pre_located) { size_t ent_size = ldr->sects[LLEXT_MEM_SYMTAB].sh_entsize; size_t syms_size = ldr->sects[LLEXT_MEM_SYMTAB].sh_size; @@ -564,16 +565,58 @@ static int llext_copy_symbols(struct llext_loader *ldr, struct llext *ext) if ((stt == STT_FUNC || stt == STT_OBJECT) && stb == STB_GLOBAL && sect != SHN_UNDEF) { - enum llext_mem mem_idx = ldr->sect_map[sect]; const char *name = llext_string(ldr, ext, LLEXT_MEM_STRTAB, sym.st_name); __ASSERT(j <= sym_tab->sym_cnt, "Miscalculated symbol number %u\n", j); sym_tab->syms[j].name = name; - sym_tab->syms[j].addr = (void *)((uintptr_t)ext->mem[mem_idx] + - sym.st_value - - (ldr->hdr.e_type == ET_REL ? 0 : - ldr->sects[mem_idx].sh_addr)); + + uintptr_t section_addr; + void *base; + + if (sect < LLEXT_MEM_BSS) { + /* + * This is just a slight optimisation for cached + * sections, we could use the generic path below + * for all of them + */ + base = ext->mem[ldr->sect_map[sect]]; + section_addr = ldr->sects[ldr->sect_map[sect]].sh_addr; + } else { + /* Section header isn't stored, have to read it */ + size_t shdr_pos = ldr->hdr.e_shoff + sect * ldr->hdr.e_shentsize; + elf_shdr_t shdr; + + ret = llext_seek(ldr, shdr_pos); + if (ret != 0) { + LOG_ERR("failed seeking to position %zu\n", shdr_pos); + return ret; + } + + ret = llext_read(ldr, &shdr, sizeof(elf_shdr_t)); + if (ret != 0) { + LOG_ERR("failed reading section header at position %zu\n", + shdr_pos); + return ret; + } + + base = llext_peek(ldr, shdr.sh_offset); + if (!base) { + LOG_ERR("cannot handle arbitrary sections without .peek\n"); + return -EOPNOTSUPP; + } + + section_addr = shdr.sh_addr; + } + + if (pre_located) { + sym_tab->syms[j].addr = (uint8_t *)sym.st_value + + (ldr->hdr.e_type == ET_REL ? section_addr : 0); + } else { + sym_tab->syms[j].addr = (uint8_t *)base + sym.st_value - + (ldr->hdr.e_type == ET_REL ? 0 : section_addr); + } + LOG_DBG("function symbol %d name %s addr %p", j, name, sym_tab->syms[j].addr); j++; @@ -954,7 +997,7 @@ static int do_llext_load(struct llext_loader *ldr, struct llext *ext, } LOG_DBG("Copying symbols..."); - ret = llext_copy_symbols(ldr, ext); + ret = llext_copy_symbols(ldr, ext, ldr_parm ? ldr_parm->pre_located : false); if (ret != 0) { LOG_ERR("Failed to copy symbols, ret %d", ret); goto out; From dff2b6c8164e13c8f4c7ee1d6a34d8ab87edef4c Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Wed, 8 May 2024 15:34:46 +0200 Subject: [PATCH 2/3] llext: (cosmetic) fix a misplaced space and re-use a variable Use an existing variable instead of re-calculating and fix swapped space and a paranthesis. Signed-off-by: Guennadi Liakhovetski --- subsys/llext/Kconfig | 2 +- subsys/llext/llext.c | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/subsys/llext/Kconfig b/subsys/llext/Kconfig index c96017c79c9f1..3034b0259ff29 100644 --- a/subsys/llext/Kconfig +++ b/subsys/llext/Kconfig @@ -26,7 +26,7 @@ config LLEXT_TYPE_ELF_OBJECT config LLEXT_TYPE_ELF_RELOCATABLE bool "Relocatable ELF file" help - Build and expect relocatable (partially linked )files as the + Build and expect relocatable (partially linked) files as the binary object type for the llext subsystem. These object files are generated by the linker by combining multiple object files into a single one. diff --git a/subsys/llext/llext.c b/subsys/llext/llext.c index 35afa99077c08..de90670916a6f 100644 --- a/subsys/llext/llext.c +++ b/subsys/llext/llext.c @@ -165,8 +165,7 @@ static int llext_find_tables(struct llext_loader *ldr) } LOG_DBG("section %d at %zx: name %d, type %d, flags %zx, addr %zx, size %zd", - i, - (size_t)ldr->hdr.e_shoff + i * ldr->hdr.e_shentsize, + i, pos, shdr.sh_name, shdr.sh_type, (size_t)shdr.sh_flags, From 21f16ceb5fb408d348c199484325312ec4269772 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Wed, 8 May 2024 15:37:39 +0200 Subject: [PATCH 3/3] llext: zero is a valid relocation offset Zero offset in a relocation entry is valid, shouldn't ignore it. Signed-off-by: Guennadi Liakhovetski --- subsys/llext/llext.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/subsys/llext/llext.c b/subsys/llext/llext.c index de90670916a6f..ec145cdd4df6e 100644 --- a/subsys/llext/llext.c +++ b/subsys/llext/llext.c @@ -738,11 +738,6 @@ static void llext_link_plt(struct llext_loader *ldr, struct llext *ext, continue; } - if (!rela.r_offset) { - LOG_WRN("PLT: zero offset idx %u name %s", j, name); - continue; - } - /* Resolve the symbol */ *(const void **)(text + got_offset) = link_addr; break;