[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:
> 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?






Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D45788