Re: objtool "no non-local symbols" error with tip of tree LLVM

Nathan Chancellor
2022-05-17T18:53:14Z

On Tue, May 17, 2022 at 05:42:04PM +0200, Peter Zijlstra wrote:
> On Tue, May 17, 2022 at 05:33:59PM +0200, Peter Zijlstra wrote:
> > On Mon, May 16, 2022 at 11:40:06PM +0200, Peter Zijlstra wrote:
> > > Does something simple like this work? If not, I'll try and reproduce
> > > tomorrow, it shouldn't be too hard to fix.
> >
> > Oh, man, I so shouldn't have said that :/
> >
> > I have something that almost works, except it now mightly upsets
> > modpost.
> >
> > I'm not entirely sure how the old code worked as well as it did. Oh
> > well, I'll get it sorted.
>
> Pff, it's been a *long* day.. here this works.

Thanks a lot for the quick fix! It resolves the error I see on 5.17 and
I don't see any new issues on mainline.

Tested-by: Nathan Chancellor <nathan@kernel.org>

> ---
> tools/objtool/elf.c | 191 ++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 125 insertions(+), 66 deletions(-)
>
> diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> index ebf2ba5755c1..a9c3e27527de 100644
> --- a/tools/objtool/elf.c
> +++ b/tools/objtool/elf.c
> @@ -600,24 +600,24 @@ static void elf_dirty_reloc_sym(struct elf *elf, struct symbol *sym)
> }
>
> /*
> - * Move the first global symbol, as per sh_info, into a new, higher symbol
> - * index. This fees up the shndx for a new local symbol.
> + * The libelf API is terrible; gelf_update_sym*() takes a data block relative
> + * index value. As such, iterate the data blocks and adjust index until it fits.
> + *
> + * If no data block is found, allow adding a new data block provided the index
> + * is only one past the end.
> */
> -static int elf_move_global_symbol(struct elf *elf, struct section *symtab,
> - struct section *symtab_shndx)
> +static int elf_update_symbol(struct elf *elf, struct section *symtab,
> + struct section *symtab_shndx, struct symbol *sym)
> {
> - Elf_Data *data, *shndx_data = NULL;
> - Elf32_Word first_non_local;
> - struct symbol *sym;
> - Elf_Scn *s;
> + Elf_Data *symtab_data = NULL, *shndx_data = NULL;
> + Elf32_Word shndx = sym->sec->idx;
> + Elf_Scn *s, *t = NULL;
> + int size, idx = sym->idx;
>
> - first_non_local = symtab->sh.sh_info;
> -
> - sym = find_symbol_by_index(elf, first_non_local);
> - if (!sym) {
> - WARN("no non-local symbols !?");
> - return first_non_local;
> - }
> + if (elf->ehdr.e_ident[EI_CLASS] == ELFCLASS32)
> + size = sizeof(Elf32_Sym);
> + else
> + size = sizeof(Elf64_Sym);
>
> s = elf_getscn(elf->elf, symtab->idx);
> if (!s) {
> @@ -625,79 +625,120 @@ static int elf_move_global_symbol(struct elf *elf, struct section *symtab,
> return -1;
> }
>
> - data = elf_newdata(s);
> - if (!data) {
> - WARN_ELF("elf_newdata");
> - return -1;
> + if (symtab_shndx) {
> + t = elf_getscn(elf->elf, symtab_shndx->idx);
> + if (!t) {
> + WARN_ELF("elf_getscn");
> + return -1;
> + }
> }
>
> - data->d_buf = &sym->sym;
> - data->d_size = sizeof(sym->sym);
> - data->d_align = 1;
> - data->d_type = ELF_T_SYM;
> + for (;;) {
> + symtab_data = elf_getdata(s, symtab_data);
> + if (t)
> + shndx_data = elf_getdata(t, shndx_data);
>
> - sym->idx = symtab->sh.sh_size / sizeof(sym->sym);
> - elf_dirty_reloc_sym(elf, sym);
> + if (!symtab_data) {
> + if (!idx) {
> + void *buf;
>
> - symtab->sh.sh_info += 1;
> - symtab->sh.sh_size += data->d_size;
> - symtab->changed = true;
> + symtab_data = elf_newdata(s);
> + if (t)
> + shndx_data = elf_newdata(t);
>
> - if (symtab_shndx) {
> - s = elf_getscn(elf->elf, symtab_shndx->idx);
> - if (!s) {
> - WARN_ELF("elf_getscn");
> + buf = calloc(1, size);
> + if (!buf) {
> + WARN("malloc");
> + return -1;
> + }
> +
> + symtab_data->d_buf = buf;
> + symtab_data->d_size = size;
> + symtab_data->d_align = 1;
> + symtab_data->d_type = ELF_T_SYM;
> +
> + symtab->sh.sh_size += size;
> + symtab->changed = true;
> +
> + if (t) {
> + shndx_data->d_buf = &sym->sec->idx;
> + shndx_data->d_size = sizeof(Elf32_Word);
> + shndx_data->d_align = 4;
> + shndx_data->d_type = ELF_T_WORD;
> +
> + symtab_shndx->sh.sh_size += 4;
> + symtab_shndx->changed = true;
> + }
> +
> + break;
> + }
> +
> + WARN("index out of range");
> return -1;
> }
>
> - shndx_data = elf_newdata(s);
> - if (!shndx_data) {
> - WARN_ELF("elf_newshndx_data");
> + if (!symtab_data->d_size) {
> + WARN("zero size data");
> return -1;
> }
>
> - shndx_data->d_buf = &sym->sec->idx;
> - shndx_data->d_size = sizeof(Elf32_Word);
> - shndx_data->d_align = 4;
> - shndx_data->d_type = ELF_T_WORD;
> + if (idx * size < symtab_data->d_size)
> + break;
>
> - symtab_shndx->sh.sh_size += 4;
> - symtab_shndx->changed = true;
> + idx -= symtab_data->d_size / size;
> }
>
> - return first_non_local;
> + if (idx < 0) {
> + WARN("negative index");
> + return -1;
> + }
> +
> + if (shndx >= SHN_UNDEF && shndx < SHN_LORESERVE) {
> + sym->sym.st_shndx = shndx;
> + if (!shndx_data)
> + shndx = 0;
> + } else {
> + sym->sym.st_shndx = SHN_XINDEX;
> + if (!shndx_data) {
> + WARN("no .symtab_shndx");
> + return -1;
> + }
> + }
> +
> + if (!gelf_update_symshndx(symtab_data, shndx_data, idx, &sym->sym, shndx)) {
> + WARN_ELF("gelf_update_symshndx");
> + return -1;
> + }
> +
> + return 0;
> }
>
> static struct symbol *
> elf_create_section_symbol(struct elf *elf, struct section *sec)
> {
> struct section *symtab, *symtab_shndx;
> - Elf_Data *shndx_data = NULL;
> - struct symbol *sym;
> - Elf32_Word shndx;
> + Elf32_Word first_non_local, new;
> + struct symbol *sym, *old;
> + int size;
> +
> + if (elf->ehdr.e_ident[EI_CLASS] == ELFCLASS32)
> + size = sizeof(Elf32_Sym);
> + else
> + size = sizeof(Elf64_Sym);
>
> symtab = find_section_by_name(elf, ".symtab");
> if (symtab) {
> symtab_shndx = find_section_by_name(elf, ".symtab_shndx");
> - if (symtab_shndx)
> - shndx_data = symtab_shndx->data;
> } else {
> WARN("no .symtab");
> return NULL;
> }
>
> - sym = malloc(sizeof(*sym));
> + sym = calloc(1, sizeof(*sym));
> if (!sym) {
> perror("malloc");
> return NULL;
> }
> - memset(sym, 0, sizeof(*sym));
> -
> - sym->idx = elf_move_global_symbol(elf, symtab, symtab_shndx);
> - if (sym->idx < 0) {
> - WARN("elf_move_global_symbol");
> - return NULL;
> - }
>
> sym->name = sec->name;
> sym->sec = sec;
> @@ -707,24 +748,42 @@ elf_create_section_symbol(struct elf *elf, struct section *sec)
> // st_other 0
> // st_value 0
> // st_size 0
> - shndx = sec->idx;
> - if (shndx >= SHN_UNDEF && shndx < SHN_LORESERVE) {
> - sym->sym.st_shndx = shndx;
> - if (!shndx_data)
> - shndx = 0;
> - } else {
> - sym->sym.st_shndx = SHN_XINDEX;
> - if (!shndx_data) {
> - WARN("no .symtab_shndx");
> +
> + new = symtab->sh.sh_size / size;
> +
> + /*
> + * Move the first global symbol, as per sh_info, into a new, higher
> + * symbol index. This fees up a spot for a new local symbol.
> + */
> + first_non_local = symtab->sh.sh_info;
> + old = find_symbol_by_index(elf, first_non_local);
> + if (old) {
> + old->idx = new;
> +
> + hlist_del(&old->hash);
> + elf_hash_add(symbol, &old->hash, old->idx);
> +
> + elf_dirty_reloc_sym(elf, old);
> +
> + if (elf_update_symbol(elf, symtab, symtab_shndx, old)) {
> + WARN("elf_update_symbol move");
> return NULL;
> }
> +
> + new = first_non_local;
> }
>
> - if (!gelf_update_symshndx(symtab->data, shndx_data, sym->idx, &sym->sym, shndx)) {
> - WARN_ELF("gelf_update_symshndx");
> + sym->idx = new;
> + if (elf_update_symbol(elf, symtab, symtab_shndx, sym)) {
> + WARN("elf_update_symbol");
> return NULL;
> }
>
> + /*
> + * Either way, we added a LOCAL symbol.
> + */
> + symtab->sh.sh_info += 1;
> +
> elf_add_symbol(elf, sym);
>
> return sym;