[PATCH] D143136: [ELF] -z notext: avoid dynamic relocations in .eh_frame

[Peter Smith via Phabricator via llvm-commits]

peter.smith accepted this revision.
peter.smith added a comment.
This revision is now accepted and ready to land.

LGTM as avoiding text segment relocations when we can is probably better than trying to add dynamic relocations.

I'm not sure I fully understand why our sectionBase::getOffset isn't working for this particular case, I expect it would become clear if I traced through the code though. However I think the canonical PLT is good solution regardless.

Some suggestions for the language in the comments.



================
Comment at: lld/ELF/Relocations.cpp:1082
 
-  bool canWrite = (sec->flags & SHF_WRITE) || !config->zText;
+  // Use a simple -z notext rule that treats all sections except .eh_frame
+  // writable. GNU ld does not reproduce dynamic relocations in .eh_frame (and
----------------
Use a simple -z notext rule that treats all sections except .eh_frame as writable. GNU ld does not produce dynamic relocations in .ehframe (and if we do, our sectionBase::getOffset will incorrectly adjust the offset).


================
Comment at: lld/test/ELF/eh-frame-znotext.s:4
+## try avoiding that (https://github.com/llvm/llvm-project/issues/60392) and
+## and use a canonical PLT entry instead.
+
----------------
Typo and and.


================
Comment at: lld/test/ELF/eh-frame-znotext.s:4
+## try avoiding that (https://github.com/llvm/llvm-project/issues/60392) and
+## and use a canonical PLT entry instead.
+
----------------
peter.smith wrote:
> Typo and and.
There's a typo (and and), could be useful to restrict the canonical PLT to .ehframe. Probably implicit from the test name though.

suggest  "try avoiding that in .ehframe (https://github.com/llvm/llvm-project/issues/60392) and use a canonical PLT entry instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143136