[PATCH] D61197: [LLD][ELF] Fix getRankProximity to "ignore" not live sections

Tue Apr 30 02:45:30 PDT 2019

grimar added a comment.

> @MaskRay is right that the linker script can be simplified to SECTIONS { .pad : { QUAD(0); } .text : { *(.text) } .ro : { *(.ro) } }, I think it looks much simpler.

I am taking these words back :)
Now, after debugging this, I think the original test case was fine. It allowed to catch a case when .orphan was placed in a different segment for me during
the experiments with the code. So I would not simplify it probably.

In D61197#1483670 <https://reviews.llvm.org/D61197#1483670>, @MaskRay wrote:

> It does fix PR41653.
>
>   # fuchsia/zircon/kernel/kernel.ld
>       .text.boot0 : {  ## it has no input section thus Sec->Live is false for it. `int Proximity = getRankProximity(Sec, *I);` is -1 which is not expected.
>

There is a mistake here. `int Proximity = getRankProximity(Sec, *I);` is -1 in a new code. In the old code it would return something different.

To summarize I am OK with this patch I think.
What I am a bit unhappy with is that `Sec->Live` is false for the output section that is actually in the output. It was confusing for me.
That started before this patch though. I tried to switch `Sec->Live` to true for `.pad` section and experimented with a code a bit but was unable to quickly find the solution.
Perhaps what can we do to avoid the confusion is to comment `Live` flag for an output section better.
Now it contains comment from input section perpective.

================
Comment at: test/ELF/linkerscript/orphan-live-only.s:38
+.section .orphan,"a"
+ .byte 0
----------------
What do you think about adding one more .orphan here with "ax" to show it is placed to a different segment?
i.e:

```
.section .orphan1,"a"
 .byte 0

.section .orphan2,"ax"
 .byte 0
```

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61197/new/

https://reviews.llvm.org/D61197