[PATCH] D34977: [ELF] - Allow moving location counter backward in some cases.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 6 03:45:53 PDT 2017


>> +static void checkSectionsOrder(OutputSection *FirstInPtLoad) {
>> +  std::vector<OutputSection *> V = getSectionsInLoad(FirstInPtLoad);
>> +  OutputSection *Prev = nullptr;
>> +  for (OutputSection *Sec : V) {
>> +    // VA of .tbss is meaningless, it is possible for this sections to
>> +    // have VA that violates ascending ordering. Just ignore them.
>> +    bool IsTbss = (Sec->Flags & SHF_TLS) && Sec->Type == SHT_NOBITS;
>> +    if (IsTbss)
>> +      continue;
>
>What test fails if you remove the tbss special case?

Oops, I am sorry, this check as well as SHF_ALLOC check below  is excessive here,
(because both .tbss and non-alloc are explicitly filtered in needsPtLoad() from load)
I experimented with ways to check proper sections ordering in a simple way and missed
my conditions become unused because I am iterating loads in posted version.

I'll update the patch.

FWIW initially I had to add that because tried to workaround tls-offset.s, it has next output 
(.tbss has VA less then .data.rel.ro)
  [ 2] .tdata            PROGBITS         0000000000202000  00002000
  [ 3] .tbss             NOBITS           0000000000202010  00002004
  [ 4] .data.rel.ro      PROGBITS         0000000000202004  00002004

>> +    if (!(Sec->Flags & SHF_ALLOC))
>> +      continue;
>
>This means we support out of order or overlapping non alloc
>sections. That is, we accept things like
>
>. = 0x2000
>.non_alloc_1 : {...}
>. = 0x1000
>.non_alloc_2 : {...}
>
>Should we reject that too?
>
>What about using overlapping BYTE commands?

So yes, with this patch we will not produce errors on moving location counter
backward here, but I believe these 2 cases are both probably very uncommon.
We may ignore them probably.

>Given that it looks like the current code just works in the saneish
>cases, I am tempted to suggest reducing the patch to just:
> <skipped patch>

I have to admit that I was thinking about the same all time during working on this patch.
May be it is reasonable solution to keep everything simple. 

But I have concerns about broken outputs we will produce. 
With latest version of patch checks are still short, but able to filter most of
broken cases with counter moving.
Particulary I probably would try to avoid to return to the point where we can have huge possible
broken outputs because of overflows in calculating offsets. 
That will happen again I think if we just remove the check.

George.


More information about the llvm-commits mailing list