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

[George Rimar via Phabricator via llvm-commits]

grimar 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
----------------
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.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D45788