From 1fe1cd4eaa97e5ae54e25ff2a8683b5704fe56cd Mon Sep 17 00:00:00 2001 From: Marco Casaroli Date: Fri, 9 Oct 2026 07:16:13 +0200 Subject: [PATCH] libs/libc/elf: Read the symbol table at once for RELA relocations. libelf_relocateadd() keeps a cache of CONFIG_LIBC_ELF_SYMBOL_CACHECOUNT symbols in a list and reads each missing symbol from the file. A large module has many more relocations than symbols (one with 1.1 million relocations against 82000 symbols took more than 30 minutes to load from a 9P share), and most of that time is spent in single reads. If there is memory for it, read the whole symbol table in one read and resolve each symbol only once. Otherwise use the cache as before. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Marco Casaroli --- libs/libc/elf/elf_bind.c | 204 +++++++++++++++++++++++++-------------- 1 file changed, 131 insertions(+), 73 deletions(-) diff --git a/libs/libc/elf/elf_bind.c b/libs/libc/elf/elf_bind.c index 6d2dccead67..f5ba11fff68 100644 --- a/libs/libc/elf/elf_bind.c +++ b/libs/libc/elf/elf_bind.c @@ -529,13 +529,17 @@ static int libelf_relocateadd(FAR struct module_s *modp, { FAR Elf_Shdr *relsec = &loadinfo->shdr[relidx]; FAR Elf_Shdr *dstsec = &loadinfo->shdr[relsec->sh_info]; + FAR Elf_Shdr *symsec = &loadinfo->shdr[loadinfo->symtabidx]; FAR Elf_Rela *relas; FAR Elf_Rela *rela; FAR Elf_SymCache *cache; + FAR Elf_Sym *syms; FAR Elf_Sym *sym; + FAR uint8_t *done; FAR dq_entry_t *e; dq_queue_t q; uintptr_t addr; + size_t nsyms; int symidx; int ret = OK; int i; @@ -555,6 +559,27 @@ static int libelf_relocateadd(FAR struct module_s *modp, dq_init(&q); + /* A large module has many more relocations than symbols, and reading + * the symbols one at a time through the cache below is then very slow. + * If there is memory for it, read the whole symbol table at once and + * resolve each symbol only once. Otherwise use the cache. + */ + + nsyms = symsec->sh_size / sizeof(Elf_Sym); + syms = lib_malloc(nsyms * sizeof(Elf_Sym) + nsyms); + done = NULL; + if (syms != NULL) + { + done = (FAR uint8_t *)&syms[nsyms]; + memset(done, 0, nsyms); + if (libelf_read(loadinfo, (FAR uint8_t *)syms, + nsyms * sizeof(Elf_Sym), symsec->sh_offset) < 0) + { + lib_free(syms); + syms = NULL; + } + } + /* Examine each relocation in the section. 'relsec' is the section * containing the relations. 'dstsec' is the section containing the data * to be relocated. @@ -585,94 +610,126 @@ static int libelf_relocateadd(FAR struct module_s *modp, symidx = ELF_R_SYM(rela->r_info); - /* First try the cache */ - - sym = NULL; - for (e = dq_peek(&q); e; e = dq_next(e)) + if (syms != NULL) { - cache = (FAR Elf_SymCache *)e; - if (cache->idx == symidx) + if (symidx < 0 || (size_t)symidx >= nsyms) { - dq_rem(&cache->entry, &q); - dq_addfirst(&cache->entry, &q); - sym = &cache->sym; - break; - } - } - - /* If the symbol was not found in the cache, we will need to read the - * symbol from the file. - */ - - if (sym == NULL) - { - if (j < CONFIG_LIBC_ELF_SYMBOL_CACHECOUNT) - { - cache = lib_malloc(sizeof(Elf_SymCache)); - if (!cache) - { - berr("Failed to allocate memory for elf symbols\n"); - ret = -ENOMEM; - break; - } - - j++; - } - else - { - cache = (FAR Elf_SymCache *)dq_remlast(&q); - } - - sym = &cache->sym; - - /* Read the symbol table entry into memory */ - - ret = libelf_readsym(loadinfo, symidx, sym, - &loadinfo->shdr[loadinfo->symtabidx]); - if (ret < 0) - { - berr("ERROR: Section %d reloc %d: " - "Failed to read symbol[%d]: %d\n", - relidx, i, symidx, ret); - lib_free(cache); + berr("ERROR: Section %d reloc %d: bad symbol index %d\n", + relidx, i, symidx); + ret = -EINVAL; break; } - /* Get the value of the symbol (in sym.st_value) */ - - ret = libelf_symvalue(modp, loadinfo, sym, - loadinfo->shdr[loadinfo->strtabidx].sh_offset, - exports, nexports); - if (ret < 0) + sym = &syms[symidx]; + if (!done[symidx]) { - /* The special error -ESRCH is returned only in one condition: - * The symbol has no name. - * - * There are a few relocations for a few architectures that do - * no depend upon a named symbol. We don't know if that is the - * case here, but we will use a NULL symbol pointer to indicate - * that case to up_relocate(). That function can then do what - * is best. - */ + /* Get the value of the symbol (in sym.st_value) */ - if (ret == -ESRCH) - { - berr("ERROR: Section %d reloc %d: " - "Undefined symbol[%d] has no name: %d\n", - relidx, i, symidx, ret); - } - else + ret = libelf_symvalue(modp, loadinfo, sym, + loadinfo->shdr[loadinfo->strtabidx].sh_offset, + exports, nexports); + if (ret < 0 && ret != -ESRCH) { berr("ERROR: Section %d reloc %d: " "Failed to get value of symbol[%d]: %d\n", relidx, i, symidx, ret); - lib_free(cache); + break; + } + + done[symidx] = 1; + } + } + else + { + /* First try the cache */ + + sym = NULL; + for (e = dq_peek(&q); e; e = dq_next(e)) + { + cache = (FAR Elf_SymCache *)e; + if (cache->idx == symidx) + { + dq_rem(&cache->entry, &q); + dq_addfirst(&cache->entry, &q); + sym = &cache->sym; break; } } - cache->idx = symidx; - dq_addfirst(&cache->entry, &q); + /* If the symbol was not found in the cache, we will need to read + * the symbol from the file. + */ + + if (sym == NULL) + { + if (j < CONFIG_LIBC_ELF_SYMBOL_CACHECOUNT) + { + cache = lib_malloc(sizeof(Elf_SymCache)); + if (!cache) + { + berr("Failed to allocate memory for elf symbols\n"); + ret = -ENOMEM; + break; + } + + j++; + } + else + { + cache = (FAR Elf_SymCache *)dq_remlast(&q); + } + + sym = &cache->sym; + + /* Read the symbol table entry into memory */ + + ret = libelf_readsym(loadinfo, symidx, sym, + &loadinfo->shdr[loadinfo->symtabidx]); + if (ret < 0) + { + berr("ERROR: Section %d reloc %d: " + "Failed to read symbol[%d]: %d\n", + relidx, i, symidx, ret); + lib_free(cache); + break; + } + + /* Get the value of the symbol (in sym.st_value) */ + + ret = libelf_symvalue(modp, loadinfo, sym, + loadinfo->shdr[loadinfo->strtabidx].sh_offset, + exports, nexports); + if (ret < 0) + { + /* The special error -ESRCH is returned only in one + * condition: The symbol has no name. + * + * There are a few relocations for a few architectures that + * do no depend upon a named symbol. We don't know if that + * is the case here, but we will use a NULL symbol pointer + * to indicate that case to up_relocate(). That function + * can then do what is best. + */ + + if (ret == -ESRCH) + { + berr("ERROR: Section %d reloc %d: " + "Undefined symbol[%d] has no name: %d\n", + relidx, i, symidx, ret); + } + else + { + berr("ERROR: Section %d reloc %d: " + "Failed to get value of symbol[%d]: %d\n", + relidx, i, symidx, ret); + lib_free(cache); + break; + } + } + + cache->idx = symidx; + dq_addfirst(&cache->entry, &q); + } } if (sym->st_shndx == SHN_UNDEF && sym->st_name == 0) @@ -708,6 +765,7 @@ static int libelf_relocateadd(FAR struct module_s *modp, } lib_free(relas); + lib_free(syms); while ((e = dq_peek(&q)) != NULL) { dq_rem(e, &q);