[ 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
