Re: Should undefined symbols in sysv hash table be ignored in do_lookup_x?

[Roland McGrath <roland at hack dot frob dot com>]

> It turns out that values of undefined function symbols in gnu hash
> table:
> 
> [hjl@gnu-6 glibc-32bit-test]$ readelf -D -s /bin/ld| grep UND
>   208   1: 0000000000404280     0 FUNC    GLOBAL DEFAULT UND xmalloc
>   211   3: 0000000000403e40     0 FUNC    GLOBAL DEFAULT UND free
>   213   3: 0000000000404a10     0 FUNC    GLOBAL DEFAULT UND
> _bfd_elf_match_sections_b
>   217   5: 0000000000404070     0 FUNC    GLOBAL DEFAULT UND bfd_hash_newfunc
> [hjl@gnu-6 glibc-32bit-test]$
> 
> are their PLT entries, which are used as function address.

Right.  I should have thought of that and pushed back on your original
claim that .gnu_hash never contains SHN_UNDEF symbols.

Anyway, while the linker putting other SHN_UNDEF symbols in the hash table
would be a linker bug if it happened (since the only purpose of the hash
table is for symbol table entries that might need to be looked up by name),
I don't think we would or should consider it a violation of the format.
So the treatment of symbol table entries should be completely identical
regardless of which hash table format was involved in locating them.

> i386 and x86-64 treat TLS relocations as PLT relocations:
> 
> /* ELF_RTYPE_CLASS_PLT iff TYPE describes relocation of a PLT entry or
>    TLS variable, so undefined references should not be allowed to
>    define the value.
[...]
> Only ppc64, am33 and mips don't treat TLS relocations as PLT relocations.

Do those three have some reason for not yielding ELF_RTYPE_CLASS_PLT for
the TLS relocation types?  If not, then we should just fix them so they do
and make it clear in the sysdeps/generic/dl-machine.h comments (which are
entirely missing for elf_machine_type_class, should be fixed) that this is
a universal requirement on elf_machine_type_class.

>    assert (ELF_RTYPE_CLASS_PLT == 1);
>    if (__builtin_expect ((sym->st_value == 0 /* No value.  */
>                 && stt != STT_TLS)
> -              || (type_class & (sym->st_shndx == SHN_UNDEF)),
> +              || ((type_class || stt == STT_TLS)
> +              & (sym->st_shndx == SHN_UNDEF)),
>                0))
>      return NULL;
> 
> works.  

"type_class ||" is wrong.  It should be "type_class == ELF_RTYPE_CLASS_PLT ||"
and then the assert above can be dropped.  Likewise & should be &&.

But it would be preferable not to abandon the branch-avoidance of the &
trick, if indeed it does produce better code now than the more obvious:
	(type_class == ELF_RTYPE_CLASS_PLT && sym->st_shndx == SHN_UNDEF)

Perhaps there is some reasonably clean way to achieve code that is as close
as possible to what we have now.  This is a very important hot path and the
problem being addressed is a very cold issue.


Thanks,
Roland