[PATCH] D100490: [ELF] Check the Elf_Rel addends for dynamic relocations

[Alexander Richardson via Phabricator via llvm-commits]

arichardson added inline comments.


================
Comment at: lld/ELF/OutputSections.cpp:548
+    const auto *sec = dyn_cast<RelocationBaseSection>(sections[i]);
+    // When linking with -r we might also call this function for input
+    // .rel[a].data sections. In this case we don't check the addend since we
----------------
arichardson wrote:
> peter.smith wrote:
> > Would it be possible to assert that we are not linking with -r and make sure we don't call checkDynRelAddends() for ld -r?
> Yes, that sounds like a better approach, will update.
Actually, this also happens for `--emit-relocs`, so I think it's probably better to skip them. I've updated the comment.


================
Comment at: lld/ELF/OutputSections.cpp:567
+            getErrorLocation(relocTarget) +
+            "wrote incorrect addend for dynamic relocation: value is 0x" +
+            utohexstr(writtenAddend) + " but expected 0x" + utohexstr(addend) +
----------------
arichardson wrote:
> peter.smith wrote:
> > If I've understood this correctly if we get here then it is an internal LLD error rather than a user error? If I'm right it would be useful to prefix with something along the lines "Internal error, please contact your supplier etc." Otherwise if I'm just using LLD I might wonder if it is something that I've done wrong.
> Yes, this should only ever be triggered if there is either a missing case in `target->getImplicitAddend()` (i.e. using `-z rel` on an architecture where getImplicitAddend() is not implemented) or if we failed to write the implicit addend correctly.
The new warning is
```
ld.lld: error: /Users/alex/cheri/upstream-llvm-project/cmake-build-debug/tools/lld/test/ELF/Output/got32-i386-pie-rw.s.tmp:(.got+0x0): internal linker error: wrote incorrect addend value 0x0 instead of 0x1180 for dynamic R_386_RELATIVE relocation against foo at offset 0x31F0
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100490