Messages in this thread | | | Date | Wed, 17 Mar 2021 14:52:23 +0100 (CET) | From | Miroslav Benes <> | Subject | Re: [PATCH 6/9] objtool: Add elf_create_undef_symbol() |
| |
[ also correcting my e-mail address ]
On Fri, 12 Mar 2021, Peter Zijlstra wrote:
Just a remark regarding SHN_XINDEX...
> +static bool elf_symbol_add(struct elf *elf, struct symbol *sym, Elf32_Word shndx) > +{ > + struct list_head *entry; > + struct rb_node *pnode; > + > + sym->type = GELF_ST_TYPE(sym->sym.st_info); > + sym->bind = GELF_ST_BIND(sym->sym.st_info); > + > + if ((sym->sym.st_shndx > SHN_UNDEF && > + sym->sym.st_shndx < SHN_LORESERVE) || > + (shndx != SHN_XINDEX && sym->sym.st_shndx == SHN_XINDEX)) { > + if (sym->sym.st_shndx != SHN_XINDEX) > + shndx = sym->sym.st_shndx; > + > + sym->sec = find_section_by_index(elf, shndx); > + if (!sym->sec) { > + WARN("couldn't find section for symbol %s", > + sym->name); > + return false; > + }
...
> @@ -366,47 +414,11 @@ static int read_symbols(struct elf *elf) > goto err; > } > > - sym->type = GELF_ST_TYPE(sym->sym.st_info); > - sym->bind = GELF_ST_BIND(sym->sym.st_info); > + if (!shndx_data) > + shndx = SHN_XINDEX; > > - if ((sym->sym.st_shndx > SHN_UNDEF && > - sym->sym.st_shndx < SHN_LORESERVE) || > - (shndx_data && sym->sym.st_shndx == SHN_XINDEX)) { > - if (sym->sym.st_shndx != SHN_XINDEX) > - shndx = sym->sym.st_shndx; > - > - sym->sec = find_section_by_index(elf, shndx); > - if (!sym->sec) { > - WARN("couldn't find section for symbol %s", > - sym->name); > - goto err;
...
> + if (!elf_symbol_add(elf, sym, SHN_XINDEX)) { > + WARN("elf_symbol_add"); > + return NULL; > + }
SHN_XINDEX means that the extended section index is used. Above you seem to use it in the opposite sense too (assigning to shndx when shndx_data is NULL). While it makes the code easier to handle, it is a bit confusing (and maybe I am just confused now). Could you add a comment about that, please? elf_symbol_add() seems like a good place.
Miroslav
| |