[PATCH] D66007: [ELF] Move (copy relocation/canonical PLT) before error checking

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

Not got a strong opinion here, happy to go with the consensus.



================
Comment at: ELF/Relocations.cpp:1004
+      !sym.isUndefined() && (sym.isFunc() || sym.isObject())) {
+    if (!canDefineSymbolInExecutable(sym)) {
+      error("cannot preempt symbol: " + toString(sym) +
----------------
Could this trigger if !(sym.isFunc() || sym.isObject())? as this code path could exist in the previous version, but not with this one. This could be fixed by removing the (sym.isFunc() || sym.isObject()) from the initial if and reinstating if (sym.isFunc()) later.


================
Comment at: ELF/Relocations.cpp:1076
+
+  if (config->isPic) {
+    errorOrWarn(
----------------
Maybe able to simplify as Config->isPic is true in both error messages.

```
if (config->isPic) {
  if (!canWrite && !isRel(expr)) {
    error("can't create dynamic relocation");
  } else {
    errorOrWarn("relocation" + toString(type) + " cannot be used against " + ...);
  }
  return;
}

```


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66007/new/

https://reviews.llvm.org/D66007