Questionable code in dynamic linker find_sym

Questionable code in dynamic linker find_sym

[Rich Felker]

I ran across this segment in find_sym while investigating (and fixing)
the mips dynamic linker regression in 1.1.3:

----------------------------------------------------------------------
		if (!sym) continue;
		if (!sym->st_shndx)
			if (need_def || (sym->st_info&0xf) == STT_TLS
			    || ARCH_SYM_REJECT_UND(sym))
				continue;
		if (!sym->st_value)
			if ((sym->st_info&0xf) != STT_TLS)
				continue;
		if (!(1<<(sym->st_info&0xf) & OK_TYPES)) continue;
		if (!(1<<(sym->st_info>>4) & OK_BINDS)) continue;
----------------------------------------------------------------------

The logic is to keep looking for a symbol definition (and reject the
current result, if any) if the current lookup:

1. Failed to be found at all.

2. Has a section of SHN_UNDEF meaning "undefined symbol" and one of
   the following conditions is met:

   a. The caller needs a real definition, i.e. it's a JMP_SLOT (PLT)
      relocation that can't resolve to itself.

   b. It's a TLS symbol. (Like JMP_SLOT, these have undefined symbols
      in the referencing DSO that can't resolve to themselves.)

   c. An arch-specific rule (like the one just added for mips) says it
      should be rejected.

3. If the symbol's value is 0 (but either not undefined, or undefined
   but not caught by condition 2), and it's not TLS (since the first
   TLS object of any module legitimately has a value of 0).

4. If the type or binding of the symbol is unacceptable (mainly
   throwing out junk that should never have been in the dynamic symbol
   table to begin with).

What looks suspicious is the placement of #3 and the special case in
it. Why is a value of 0 valid for TLS but not for other types of
symbols, e.g. a defined symbol pointing to the beginning of a shared
library's Ehdr, which would have relative address 0?

It seems to me like rule 3 should actually fall under 2 as a condition
for discarding undefined (SHN_UNDEF) symbols rather than as its own
rule applied regardless of the section. Then the special case for TLS
under #3 could be thrown out.

I think the special case for TLS in #2a could also be removed if TLS
lookups simply used need_def==true like PLT lookups do, but that's a
separate issue and should not result in any functional change, so I'm
not as concerned about it. The above proposed change would change
behavior (allowing some symbols with value zero to be used, which are
rejected right now).

Rich