[PATCH] D40026: [ELF] - Allow applying SHF_MERGE optimization for relocatable output.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/Writer.cpp:494
+    // which is used for -r.
+    if (isa<SyntheticSection>(IS) && !(IS->Flags & SHF_MERGE))
+      continue;
----------------
ruiu wrote:
> grimar wrote:
> > ruiu wrote:
> > > grimar wrote:
> > > > ruiu wrote:
> > > > > I wonder if you really need to handle this as a special case.
> > > > Do you mean we can create symbols for all synthetic sections instead ? I think that is at least confusing to see in output and excessive.
> > > Well, I mean I actually do not understand what this code is for in the first place.
> > It is for this place:
> > https://github.com/llvm-mirror/lld/blob/master/ELF/SyntheticSections.cpp#L1529
> > 
> > When we rewrite relocations we updating info field and want to set proper section index.
> > Before this patch we had regular SHF_MERGE sections in output, so symbols were created for them.
> > But now them are synthetic and there is no symbol, so we emit wrong zero index and relocation is broken.
> > 
> > BTW, I think that affects regular output + --emit-relocs as well. I am going to check soon and will split that change to different patch if so.
> This is probably okay, but let me think about it, as it still feels somewhat ad-hoc. There might be some simpler way.
Sure. I can confirm that --emit-reloc is affected, I am going to post a patch for that soon, so we can continue discussion of this place there.


https://reviews.llvm.org/D40026