[PATCH] D45788: Mitigate relocation overflow [part 1 of 2]

[Han Shen via Phabricator via llvm-commits]

shenhan added inline comments.


================
Comment at: test/ELF/linkerscript/no-space.s:4
 
-# RUN: echo "SECTIONS {foo 0 : {*(foo*)} }" > %t.script
+# RUN: echo "SECTIONS {foo 30 : {*(foo*)} }" > %t.script
 # RUN: ld.lld --hash-style=sysv -o %t --script %t.script %t.o -shared
----------------
grimar wrote:
> shenhan wrote:
> > grimar wrote:
> > > I do not think this is correct, sorry. This change broke the intention of the test case I believe.
> > > 
> > > You did that because LLD starts triggering an error:
> > > 
> > > ```
> > > error: section .dynsym file range overlaps with foo
> > > >>> .dynsym range is [0x1000, 0x1017]
> > > >>> foo range is [0x1000, 0x1007]
> > > ```
> > > 
> > > What happens because orphan section `.dynsym` is placed before `foo`. And then during
> > > assigning of VA, `Dot` is explicitly set to 0 for `foo`, making it intersect with .dynsym.
> > > 
> > > I think the expected behavior is to not place these orphan sections before `foo`, since
> > > the position of `foo` is explicitly requested by linker script. And sure I do not think
> > > we should emit an error in that case.
> > > 
> > > This case needs some investigation.
> > > I'll be happy to hear other people opinion on that too.
> > Thanks for pointing this out, yes, I agree we need to have consensus on this (and the "out-of-order.s" test case below).
> > 
> > Here is my thoughts:
> > 1. The test writer assume the fact that "foo" must be the first section in its segment (which is a group of output sections), otherwise, its address intersects with the first output section in the segment, because DOT is set to 0 for "foo" by linkerscript.
> > 
> > 2. There is no fact in the test case supports this assumption. 
> > 
> > 3. So this test case might cause linker error whenever output sections change relative order in the segment.
> > 
> > So my suggestion:
> > Honor the assumption, so instead of change the DOT for foo, change it for .dynsym (the first output section of segment):
> > - # RUN: echo "SECTIONS {foo 0 : {*(foo*)} }" > %t.script
> > + # RUN: echo "SECTIONS {.dynsym 0 : {*(.dynsym)} }" > %t.script 
> > This reserves the origin test intention.
> > 
> > .OR.
> > Always layout output sections with explicit address assignment first, then layout those without. So for this particular case, the origin output section order is:
> >   .dynsym .dynstr foo
> > We layout it like below:
> >   foo .dynsym .dynstr
> > 
> > .OR.
> > Report error on rewinding "." to a conflict address.
> > 
> > [This also applies to out-of-order.s test case below.]
> > 
> > I'm happy to go any direction.
> > 
> > Comments?
> > 
> > 
> > 
> > 
> > .OR.
> > Always layout output sections with explicit address assignment first
> 
> I believe it is the correct way. A script should specify the layout, any orphans
> should be placed after the explicitly specified sections. That seems logically correct
> and consistent with gnu linkers behavior (I think).
> 
> I would really want to hear from other people before proceeding this place probably.
Thanks, George.

@ruiu Can you comment?


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D45788