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

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/Symbols.cpp:152
 
+  // Undefined external symbols in a non-DSO usually reported by linker and link
+  // fails, but together with --unresolved-symbols=ignore-all link succeeds and
----------------
ruiu wrote:
> Why do you have to move this code?
Previously for `!Config->Shared` we always returned `false` and it was early exit,
but if we want to handle undefined symbols case we need to move it and we can not exit
early anymore.

For example we should not return that undefined symbol is preemtible if it is hidden
(in that case includeInDynsym() should filter it out, that is required for hidden-vis-shared.s).

There are many other tests with other cases which would reasonable fail if I do not move this piece.



================
Comment at: ELF/Symbols.cpp:365-373
   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;
+  if (Config->Shared)
----------------
ruiu wrote:
> Does this order matter?
A kind of. What I tried to do here is preserve original logic, but add code that is equal to:
```
if (!Config->Shared && canBeExternal() && !body()->symbol()->isWeak())
  return true;
```

I just separated conditions and tried to exit early when possible. I reduced it by 2 lines in fresh diff.


https://reviews.llvm.org/D35724