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

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

I have no more comments here.



================
Comment at: ELF/InputSection.cpp:887
+    if (!D)
+      continue;
+
----------------
saugustine wrote:
> grimar wrote:
> > saugustine wrote:
> > > grimar wrote:
> > > > This 'continue' is untested by a test case it seems.
> > > I've tried to write test-cases to execute this continue, but have been unable to do so. They protect against an erroneous or strange input.
> > > 
> > > I've just deleted these two lines, if someone manages to trigger this case, lld will just crash.
> > The lines are still there.
> > 
> > I was able to trigger this case after adding the `call foo at plt` and `-shared`
> > to your `x86-64-split-stack-prologue-adjust-success.s` test:
> > 
> > 
> > ```
> > # RUN: ld.lld --defsym __morestack=0x100 --defsym __morestack_non_split=0x200 %t1.o %t2.o -shared -o %t
> > ...
> > 
> > .size	split,. - split
> > 
> > call foo at plt
> > 	
> > .section	.note.GNU-stack,"", at progbits
> > .section	.note.GNU-split-stack,"", at progbits
> > ```
> Using "-shared" creates problems because llvm-objdump doesn't support .plt sections very well.
> 
> https://bugs.llvm.org/show_bug.cgi?id=38186
> 
> Any check using both -shared and llvm-objdump will likely be effectively a bitwise comparison.
> 
> I would like not to block this change on getting that bug fixed, just so that we can have a single extra line of coverage, in near trivial code.
> 
> Can we just not have coverage for this single line?
Go ahead, I'll handle this after your commit.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D46653