llext: fix llext_load() optimization

The optimization in llext_load() to avoid using the generic path for sections that are cached in memory was broken for two reasons: - it was comparing an ELF section index to LLEXT_MEM_BSS, which is a llext_mem enum, and - it was using the wrong section address for the cached sections since the "merged sections" feature was introduced in 709b2e4. This patch fixes both issues using the new llext_loaded_sect_ptr() helper function. Signed-off-by: Luca Burelli <[email protected]>
mjaun · Jun 26, 2024 · a1550c4 · a1550c4
1 parent ee6074a
commit a1550c4
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 20 deletions.
diff --git a/subsys/llext/llext_link.c b/subsys/llext/llext_link.c
@@ -288,9 +288,9 @@ int llext_link(struct llext_loader *ldr, struct llext *ext, bool do_local)
 				 * adding st_value to the start address of the section
 				 * in which the target symbol resides.
 				 */
-				enum llext_mem mem_idx = ldr->sect_map[sym.st_shndx].mem_idx;
-
-				link_addr = (uintptr_t)ext->mem[mem_idx] + sym.st_value;
+				link_addr = (uintptr_t)llext_loaded_sect_ptr(ldr, ext,
+									     sym.st_shndx)
+					    + sym.st_value;
 			} else {
 				LOG_ERR("rela section %d, entry %d: cannot apply relocation: "
 					"target symbol has unexpected section index %d (0x%X)",

diff --git a/subsys/llext/llext_load.c b/subsys/llext/llext_load.c
@@ -520,27 +520,23 @@ static int llext_copy_symbols(struct llext_loader *ldr, struct llext *ext,
 
 			sym_tab->syms[j].name = name;
 
-			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
+			elf_shdr_t *shdr = ldr->sect_hdrs + sect;
+			uintptr_t section_addr = shdr->sh_addr;
+			const void *base;
+
+			base = llext_loaded_sect_ptr(ldr, ext, sect);
+			if (!base) {
+				/* If the section is not mapped, try to peek.
+				 * Be noisy about it, since this is addressing
+				 * data that was missed by llext_map_sections.
 				 */
-				base = ext->mem[ldr->sect_map[sect].mem_idx];
-				section_addr = ldr->sects[ldr->sect_map[sect].mem_idx].sh_addr;
-			} else {
-				elf_shdr_t *shdr = ldr->sect_hdrs + sect;
-
 				base = llext_peek(ldr, shdr->sh_offset);
-				if (!base) {
-					LOG_ERR("cannot handle arbitrary sections without .peek\n");
+				if (base) {
+					LOG_DBG("section %d peeked at %p", sect, base);
+				} else {
+					LOG_ERR("No data for section %d", sect);
 					return -EOPNOTSUPP;
 				}
-
-				section_addr = shdr->sh_addr;
 			}
 
 			if (pre_located) {

diff --git a/subsys/llext/llext_priv.h b/subsys/llext/llext_priv.h
@@ -59,6 +59,18 @@ static inline const char *llext_string(struct llext_loader *ldr, struct llext *e
 	return (char *)ext->mem[mem_idx] + idx;
 }
 
+static inline const void *llext_loaded_sect_ptr(struct llext_loader *ldr, struct llext *ext,
+						unsigned int sh_ndx)
+{
+	enum llext_mem mem_idx = ldr->sect_map[sh_ndx].mem_idx;
+
+	if (mem_idx == LLEXT_MEM_COUNT) {
+		return NULL;
+	}
+
+	return (const uint8_t *)ext->mem[mem_idx] + ldr->sect_map[sh_ndx].offset;
+}
+
 /*
  * Relocation (llext_link.c)
  */