[PATCH] D32399: [LLD] Order writable executable sections before writable ones

[Rafael Espíndola via llvm-commits]

What about the Config->SingleRoRx case? That is the bfd default on
x86_64. Is that different on sparc?

Please update the comment to say why you want rxw sections before rw sections.

I actually like the structure of the function. Writing it as a series of

if (AFoo != BFoo)
  return Afoo;

Gives a clear hierarchy and a place to put comments about why "Foo"
should impact the order. After that return we have the post condition
that all sections are Foo or !Foo.

Cheers,
Rafael



On 24 April 2017 at 18:05, Rui Ueyama via Phabricator
<reviews at reviews.llvm.org> wrote:
> ruiu added inline comments.
>
>
> ================
> Comment at: lld/ELF/Writer.cpp:698
>      if (AIsExec != BIsExec)
> -      return BIsExec;
> +      return (A->Flags & SHF_WRITE) ? AIsExec : BIsExec;
>    }
> ----------------
> kettenis wrote:
>> ruiu wrote:
>> > Is there any problem if you just return `AIsExec`?
>> Yes.  Then read-only executable sections would be sorted before read-only sections, which is not what we want.  Basically the current ordering is:
>>
>> R
>> RX
>> RW <- contains .bss
>> RWX
>>
>> I propose we change this to
>>
>> R
>> RX
>> RWX
>> RW <- contains .bss
>>
> I believe this code is correct, but this function seems a bit puzzling to me. Each decision-making code block is well-commented and clear about what it does, but as a whole it's not easy to predict what this function returns for some two sections. I want to make this as simple as your example
>
> R
> RX
> RWX
> RW.
>
> Do you think you can improve this function? If you don't have a bandwidth to handle it, I'll try to simplify.
>
>
> https://reviews.llvm.org/D32399
>
>
>