[PATCH] D35724: [ELF] - Fix missing relocation when linking executable with --unresolved-symbols=ignore-all
George Rimar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 26 04:26:15 PDT 2017
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
> 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
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 ExportDynamic || body()->isShared() ||
- (body()->isUndefined() && Config->Shared);
+ if (ExportDynamic || body()->isShared())
+ return true;
+ if (!body()->isUndefined())
+ return false;
+ if (Config->Shared)
> 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())
I just separated conditions and tried to exit early when possible. I reduced it by 2 lines in fresh diff.
More information about the llvm-commits