From e596d5bd5ea4be9cc0b3e2ab8910c6d90332045c Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Fri, 28 Jan 2022 15:13:41 +0900 Subject: [PATCH] binfmt/libelf: Implement sh_addralign handling Basically, mirror the following two commits from modlib. It's shame we have two copies of elf loaders. ``` commit 51490bad551737ce8210c196ce962fd28121a97d Author: YAMAMOTO Takashi Date: Wed Apr 14 17:07:39 2021 +0900 modlib: Implement sh_addralign handling I've seen a module with 16 bytes .rodata alignment for xmm operations. It was getting SEGV on sim/Linux because of the alignment issue. The same module binary seems working fine after applying this patch. Also, tested on sim/macOS and esp32 on qemu, using a module with an artificially large alignment. (64 bytes) ``` ``` commit 418e11b8b38855eacf55deea8d658c5760b51488 Author: YAMAMOTO Takashi Date: Thu Apr 15 11:33:48 2021 +0900 modlib: Always use separate allocation for text and data Pros: * Reduce code differences * Smaller allocations for !CONFIG_ARCH_USE_MODULE_TEXT Cons: * Likely to use more memory for !CONFIG_ARCH_USE_MODULE_TEXT in total Tested with: * sim:module on macOS * esp32-devkit:nsh + CONFIG_MODULE on qemu * lm3s6965-ek:qemu-protected + CONFIG_EXAMPLES_SOTEST on qemu ``` --- binfmt/elf.c | 2 ++ binfmt/libelf/libelf_addrenv.c | 31 +++++++++++++------------------ binfmt/libelf/libelf_load.c | 16 ++++++++++++++++ include/nuttx/binfmt/elf.h | 5 ++--- 4 files changed, 33 insertions(+), 21 deletions(-) diff --git a/binfmt/elf.c b/binfmt/elf.c index 5b298bfe1b..85fbe300aa 100644 --- a/binfmt/elf.c +++ b/binfmt/elf.c @@ -112,6 +112,8 @@ static void elf_dumploadinfo(FAR struct elf_loadinfo_s *loadinfo) binfo(" dataalloc: %08lx\n", (long)loadinfo->dataalloc); binfo(" textsize: %ld\n", (long)loadinfo->textsize); binfo(" datasize: %ld\n", (long)loadinfo->datasize); + binfo(" textalign: %zu\n", loadinfo->textalign); + binfo(" dataalign: %zu\n", loadinfo->dataalign); binfo(" filelen: %ld\n", (long)loadinfo->filelen); #ifdef CONFIG_BINFMT_CONSTRUCTORS binfo(" ctoralloc: %08lx\n", (long)loadinfo->ctoralloc); diff --git a/binfmt/libelf/libelf_addrenv.c b/binfmt/libelf/libelf_addrenv.c index 1fd5be96e3..9ede25a383 100644 --- a/binfmt/libelf/libelf_addrenv.c +++ b/binfmt/libelf/libelf_addrenv.c @@ -122,7 +122,8 @@ int elf_addrenv_alloc(FAR struct elf_loadinfo_s *loadinfo, size_t textsize, up_textheap_memalign(loadinfo->textalign, textsize); #else - loadinfo->textalloc = (uintptr_t)kumm_malloc(textsize + datasize); + loadinfo->textalloc = (uintptr_t)kumm_memalign(loadinfo->textalign, + textsize); #endif if (!loadinfo->textalloc) @@ -130,16 +131,15 @@ int elf_addrenv_alloc(FAR struct elf_loadinfo_s *loadinfo, size_t textsize, return -ENOMEM; } -#if defined(CONFIG_ARCH_USE_TEXT_HEAP) - loadinfo->dataalloc = (uintptr_t)kumm_malloc(datasize); - - if (0 != datasize && !loadinfo->dataalloc) + if (loadinfo->datasize > 0) { - return -ENOMEM; + loadinfo->dataalloc = (uintptr_t)kumm_memalign(loadinfo->dataalign, + datasize); + if (!loadinfo->dataalloc) + { + return -ENOMEM; + } } -#else - loadinfo->dataalloc = loadinfo->textalloc + textsize; -#endif return OK; #endif @@ -177,24 +177,19 @@ void elf_addrenv_free(FAR struct elf_loadinfo_s *loadinfo) } #else -#if defined(CONFIG_ARCH_USE_TEXT_HEAP) if (loadinfo->textalloc != 0) { +#if defined(CONFIG_ARCH_USE_TEXT_HEAP) up_textheap_free((FAR void *)loadinfo->textalloc); +#else + kumm_free((FAR void *)loadinfo->textalloc); +#endif } if (loadinfo->dataalloc != 0) { kumm_free((FAR void *)loadinfo->dataalloc); } -#else - /* If there is an allocation for the ELF image, free it */ - - if (loadinfo->textalloc != 0) - { - kumm_free((FAR void *)loadinfo->textalloc); - } -#endif #endif diff --git a/binfmt/libelf/libelf_load.c b/binfmt/libelf/libelf_load.c index 943a5f6c2a..ce235f29a3 100644 --- a/binfmt/libelf/libelf_load.c +++ b/binfmt/libelf/libelf_load.c @@ -57,6 +57,10 @@ # define MIN(x,y) ((x) < (y) ? (x) : (y)) #endif +/* _ALIGN_UP: 'a' is assumed to be a power of two */ + +#define _ALIGN_UP(v, a) (((v) + ((a) - 1)) & ~((a) - 1)) + /**************************************************************************** * Private Constant Data ****************************************************************************/ @@ -104,11 +108,21 @@ static void elf_elfsize(struct elf_loadinfo_s *loadinfo) if ((shdr->sh_flags & SHF_WRITE) != 0) { + datasize = _ALIGN_UP(datasize, shdr->sh_addralign); datasize += ELF_ALIGNUP(shdr->sh_size); + if (loadinfo->dataalign < shdr->sh_addralign) + { + loadinfo->dataalign = shdr->sh_addralign; + } } else { + textsize = _ALIGN_UP(textsize, shdr->sh_addralign); textsize += ELF_ALIGNUP(shdr->sh_size); + if (loadinfo->textalign < shdr->sh_addralign) + { + loadinfo->textalign = shdr->sh_addralign; + } } } } @@ -172,6 +186,8 @@ static inline int elf_loadfile(FAR struct elf_loadinfo_s *loadinfo) pptr = &text; } + *pptr = (FAR uint8_t *)_ALIGN_UP((uintptr_t)*pptr, shdr->sh_addralign); + /* SHT_NOBITS indicates that there is no data in the file for the * section. */ diff --git a/include/nuttx/binfmt/elf.h b/include/nuttx/binfmt/elf.h index 46e74bb536..5cd2e29c1c 100644 --- a/include/nuttx/binfmt/elf.h +++ b/include/nuttx/binfmt/elf.h @@ -95,10 +95,9 @@ struct elf_loadinfo_s uintptr_t textalloc; /* .text memory allocated when ELF file was loaded */ uintptr_t dataalloc; /* .bss/.data memory allocated when ELF file was loaded */ size_t textsize; /* Size of the ELF .text memory allocation */ -#ifdef CONFIG_ARCH_USE_TEXT_HEAP - size_t textalign; /* Necessary alignment of .text */ -#endif size_t datasize; /* Size of the ELF .bss/.data memory allocation */ + size_t textalign; /* Necessary alignment of .text */ + size_t dataalign; /* Necessary alignment of .bss/.data */ off_t filelen; /* Length of the entire ELF file */ Elf_Ehdr ehdr; /* Buffered ELF file header */