[PATCH] D35724: [ELF] - Fix missing relocation when linking executable with --unresolved-symbols=ignore-all

[Rafael Avila de Espindola via llvm-commits]

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

>    // Only symbols that appear in dynsym can be preempted.
>    if (!symbol()->includeInDynsym())
>      return false;
>  
>    // Only default visibility symbols can be preempted.
>    if (symbol()->Visibility != STV_DEFAULT)
>      return false;
>  
> +  // Undefined symbols in non-DSOs are usually just an error, so it
> +  // doesn't matter whether we return true or false here. However, if
> +  // -unresolved-symbols=ignore-all is specified, undefined symbols in
> +  // executables are automatically exported so that the runtime linker
> +  // can try to resolve them. In that case, they is preemptible. So, we
> +  // return true for an undefined symbol in case the option is specified.
> +  if (!Config->Shared)
> +    return isUndefined() && !symbol()->isWeak();

The change in this patch is to add "!symbol()->isWeak();"
above. Shouldn't this case have been handled by includeInDynsym two ifs above?

>  bool Symbol::includeInDynsym() const {
>    if (computeBinding() == STB_LOCAL)
>      return false;
> -  return ExportDynamic || body()->isShared() ||
> -         (body()->isUndefined() && Config->Shared);
> +  if (ExportDynamic || body()->isShared())
> +    return true;
> +  if (!body()->isUndefined())
> +    return false;
> +  return Config->Shared || !body()->symbol()->isWeak();

Can you change the last line to just "return Config->Shared"?

Cheers,
Rafael