[PATCH] D46653: Start support for linking object files with split stacks

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/Arch/X86_64.cpp:474
+    &X86_64<ELF32LE>::getPrologueAdjustments() const {
+  static llvm::SmallVector<SplitStackPrologueAdjustment, 4> PAs = {
+      SplitStackPrologueAdjustment(
----------------
We generally don't use the table-driven coding pattern that much in lld, especially when compared to other LLVM subprojects. I prefer less abstracted code. I'd write this as a (boring but simple) sequence of "if" and "else if" that calls memcmp() and memcpy() to replace a matched pattern.


================
Comment at: ELF/InputFiles.cpp:649
+  // the no_split_stack attribute, then the object file will also have
+  // a .note.GNU-no-split-stack section."
   if (Name == ".note.GNU-split-stack") {
----------------
saugustine wrote:
> grimar wrote:
> > I'm not an expert in licenses. I do not know if we can quote the text like that. It probably belongs to a different code license. I think we usually write own comments and just add a link if we need. Can you do the same here?
> I've just kept the cross reference and deleted everything else.
I believe quoting that amount of text should be fine, but if you are cautious, please write a comment with your own words instead of deleting the comment entirely. That was a helpful comment.


================
Comment at: ELF/InputSection.cpp:793
 
+template <class ELFT>
+void InputSectionBase::adjustCrossSplitFunctionPrologues(uint8_t *Buf,
----------------
What does this function do? Please write a function comment.

This function seems a bit too long. Please consider splitting.


================
Comment at: ELF/InputSection.cpp:820
+      // fall through to the reporting below.
+      if (Defined *F = getEnclosingFunction<ELFT>(Rel.Offset)) {
+        // Nothing to be done if this prologue was already adjusted.
----------------
This function seems too slow to call for each relocation. The number of relocation can be an order of tens of millions, so per-relocation processing needs to be really fast.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D46653