[PATCH] D23505: [ELF] - Linkrscript: change the way of creating PT_LOADs segments when LS is used.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/Writer.cpp:968
@@ +967,3 @@
+    uintX_t NewFlags = Sec->getPhdrFlags();
+    if (((Load->H.p_flags & PF_W) == 0) && ((NewFlags & PF_W) != 0))
+      Load = addHdr(Dest, PT_LOAD, NewFlags);
----------------
grimar wrote:
> ruiu wrote:
> > grimar wrote:
> > > ruiu wrote:
> > > > What if the current section is writable and NewFlags are not writable? This code seems to merge them, but is it the right thing to do?
> > > That is what ld do. I think it is fine because writable sections go after all other ones. 
> > > At least it is assumed they should.
> > I do not understand. Why it is guaranteed that writable sections are written after all other sections? If you are using linker scripts, you have a full control over the section layout, so you can put read-only sections before writable sections.
> > 
> > In addition to that, probably needless to say, but doing the same thing as ld does is not the ultimate goal. It's an important goal, but the ultimate goal should be to implement a logic that we understand and can justify. The last part is important. (So, we don't want to say "we don't know why, but this is what ld does, so do we.")
> >I do not understand. Why it is guaranteed that writable sections are written after all other sections? If you are using linker scripts, you have a full >control over the section layout, so you can put read-only sections before writable sections.
> 
> It is not quaranteed. Using linkerscript you can do whatever you want, even useless things. At fact ld looks always create 2 loads which are text and data. At least that means that:
> 1) This should met everybodies needs as programs in the wild that use ld works with this behavior.
> 2) It is probably makes no sence to mix or reorder sections in the way when writable are not after readable, just because it was impossible with ld, what I guess means nobody wants and use that.
> 3) This approach of 2 loads instead N simplifies things.
> 4) If you want full control over headers you always can use PHDRS command.
> 
> >In addition to that, probably needless to say, but doing the same thing as ld does is not the ultimate goal. It's an important goal, but the ultimate >goal should be to implement a logic that we understand and can justify. The last part is important. (So, we don't want to say "we don't know why, >but this is what ld does, so do we.")
> 
> Sure. My point is ld behavior is just simpler and looks is enough for users, so see no reasons why not to use it. Spec says "The linker will create reasonable program headers by default". From what we see when linking such programs like FreeBSD kernel, lld creates 4 PT_LOADS when ld 2. Question is what is more reasonable and why we create something that looks not really needed ?
I don't think none of your arguments justify the behavior you are implementing here. How would you describe to the user why we merge section A and B if A is read-only and B is writable but don't merge if A is writable and B is read-only?

If we don't want to merge read-only sections with writable sections, we always put them in separate segments. If not, we should merge them. The proposed behavior seems inconsistent and hard to justify.


https://reviews.llvm.org/D23505